formaat-design / reshaped

Community repository for storing examples, reporting issues and tracking roadmap
https://reshaped.so
97 stars 3 forks source link

Tooltips are displayed even when text is undefined #255

Closed noob2 closed 1 month ago

noob2 commented 1 month ago

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:

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

blvdmitry commented 1 month ago

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 commented 1 month ago

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

https://github.com/formaat-design/reshaped/assets/887379/31e2924f-7d61-4202-844d-9eb88a11302b

noob2 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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

https://github.com/formaat-design/reshaped/assets/887379/a35bab83-5a05-4c32-9296-f8aa09e5e93e

blvdmitry commented 1 month ago

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.

https://github.com/formaat-design/reshaped/assets/887379/cb875f06-c587-454f-86fd-3fb386ba2a76

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