Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tooltips are displayed even when text is undefined #255

Closed
noob2 opened this issue May 17, 2024 · 6 comments
Closed

Tooltips are displayed even when text is undefined #255

noob2 opened this issue May 17, 2024 · 6 comments

Comments

@noob2
Copy link

noob2 commented May 17, 2024

Describe the bug
Tooltips are displayed even when text is undefined. Additionally, when I open a modal, the first tooltip is activated automatically.

To Reproduce
Create an isolated reproduction on the CodeSandox:
sandbox

Expected behavior
When text is undefined, empty string, or null, the tooltip should not be present on the screen.
When I open a modal, tooltips should not be visible until hovered over.

Screenshots
image
image

Environment:

  • OS: Ubuntu
  • Browser: Google Chrome

P.S. I also noticed that <Tooltip text="hello" active={reactiveValue}> is not reactive.

@blvdmitry
Copy link
Contributor

Regarding tooltips in the modal – this happens due to focus trapping since it the first focusable element inside the modal gets focused when it opens. What would be your expected behavior in that case? Would you expect the whole modal to get focused first as an "additional" element in the list of focusable elements or something else?

@blvdmitry
Copy link
Contributor

Updated the text property implementation to support undefined values too. I think this became a side-effect of the latest react releases where ReactNode started supporting undefined values, while originally Tooltip wasn't supporting undefined

Also do you have a reproduction case for the reactivity part? When testing it locally based on the state – it seems to work as expected

Screen.Recording.2024-05-19.at.21.51.16.mov

@noob2
Copy link
Author

noob2 commented May 21, 2024

@blvdmitry I woud expect that the modal is focused as additional element.
Also I updated the sandbox example to include the 3-rd case when using useResponsiveClientValue with 'active' property of the tooltip

@blvdmitry
Copy link
Contributor

To keep you updated – this issue happens because of the implementation internals, where we rely on a multi-step state changes based on a reducer which trigger one after another in the react lifecycle. However media query changes happen based on the native event loop and we trigger some of the related reducer actions before React state gets updated. I'm planning to look into the positioning utility next and try to move as much logic as possible to sync vanilla js computations instead of waiting for useEffects to resolve in an attempt to address this.

@blvdmitry
Copy link
Contributor

Actually found a simpler way to fix this even though it took some code refactoring just to understand the problem better. Going to resolve the modal bit tomorrow or the day after too and ship a new patch with this update

Screen.Recording.2024-05-29.at.01.19.36.mov

@blvdmitry
Copy link
Contributor

Tried multiple approaches and added a new autoFocus={false} prop for disabling the auto focus on the first element and move it to the modal root initially. Additionally added an explicit ariaLabel property to add a custom label inn case Modal.Title and Modal.Subtitle are not used for in the modal.

Screen.Recording.2024-05-29.at.23.09.12.mov

You can try it out in 2.11.11 and feel free to reply here or in a new issue in case there are any other suggestions regarding this thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Waiting for the release
Development

No branches or pull requests

2 participants