mantinedev / mantine

A fully featured React components library
https://mantine.dev
MIT License
26.87k stars 1.9k forks source link

Spoiler component, not showing up correctly, disappears after page refresh #1158

Closed FranciscoMessina closed 2 years ago

FranciscoMessina commented 2 years ago

What package has an issue

@mantine/core

Describe the bug

I added a Spoiler to the Mantine UI comment component, when I first added it it worked fine, but then I went to check it again and could not see it any more, the text was hidden when being taller than the set heigh but the "Show more" text wasn't there. When I changed the height and saved the changed it worked again, and after refreshing the page it stopped working. This is the repository .

I am not sure if maybe I made a mistake somewhere and that is why it stopped working, so if anybody else experienced this and fixed and can tell how to do it, it would be highly appreciated.

In which browser did the problem occur

Brave, Firefox

If possible, please include a link to a codesandbox with the reproduced problem

No response

Do you know how to fix the issue

No

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

No response

rtivital commented 2 years ago

Please send an example in codesandbox that reproduces the issue

FranciscoMessina commented 2 years ago

Of course, here https://codesandbox.io/s/modern-frost-rm8216?file=/src/components/Comment.tsx

I am also leaving an image of the comment, the overflow text is hidden but the Show More is not visible. image

ercgrat commented 2 years ago

The root cause of the issue that @FranciscoMessina is reporting is that the comments are initially rendering with a clientHeight of 0 inside a Collapse component that is collapsed. The child component of the Spoiler is not rerendering when the Collapse is expanded, so there is no recalculation for the spoiler link to show (it relies on an effect with children prop as a dependency).

I wasn't able to figure out how to fix this with resize observing on the content div inside Spoiler, though I assume that it is still possible via a resize listener or MutationObserver of some sort.

A more straightforward way to fix this would be to change the way Collapse behaves. The application of display: none is what causes the children clientHeight to be calculated as 0. Collapse is also unconditionally rendering its children, but could do it conditionally so they rerender when the collapse is toggled (e.g. {opened ? children : null}). However, I tested that and conditional rendering doesn't fix it fully, needs the display: none to be removed.

Workarounds would be manipulating the comments to rerender on some timeout after collapse is expanded, or to render them expanded by default.

rtivital commented 2 years ago

@ercgrat Spoiler component does not use Collapse in any way

ercgrat commented 2 years ago

I know that Spoiler does not use Collapse. I was explaining why the Spoiler in @FranciscoMessina's code sandbox has an issue. It's an interaction between Collapse and Spoiler, so needs some judgment on deciding which component to adjust, if any.

Updated the original comment!

FranciscoMessina commented 2 years ago

@ercgrat That makes a lot of sense, well I guess for now I will use it without the collapse. Thanks for taking the time to look at it!

I don't think the combined behavior of those two components is optimal, but I can't think about something that could be done to change it.

auronsan commented 2 years ago

I think this one can be fixed by listen onTransitionEnd @FranciscoMessina

this sandbox fixed already https://codesandbox.io/s/clever-hawking-fr0ph1?file=/src/pages/Detail.tsx

FranciscoMessina commented 2 years ago

Thanks! That is a great way to fix it!

rtivital commented 2 years ago

Fixed in 5.1.1