search

Stanko / react-animate-height

Lightweight React component for animating height using CSS transitions. Slide up/down the element, and animate it to any specific height.
https://muffinman.io/react-animate-height
MIT License
758 stars 53 forks source link

Provided `style` prop values overridden internally #105

Closed bmaclean closed 2 years ago

bmaclean commented 3 years ago

The AnimateHeight internally overrides overflow styles provided explicitly to the component. As a result, AnimateHeight instances with a numeric or percentage height. I understand this is the default behaviour to prevent overflow (and compute height?) when setting an explicit height, but it restricts developers from modifying this behaviour and having to resort to !important or other means to add rules with higher specificity. In my case, the child's box-shadow is clipped even when providing a style props value of {overflow: 'visible'}.

Code example

<AnimateHeight
    height={condition ? 'auto' : 120}
    style={{overflow: 'visible'}}
    >
    <MyComponentWithBoxShadow />
</AnimateHeight>

The above example retains inline styling of overflow: hidden. See codesandbox.

Expected behavior The user should be able to override the default overflow behaviour.

Possible Solution I'd have to check out the project locally, but looks like the conditional overflow value could be reversed to default to the style provided by the parent, and not the overflow value determined internally.

Your Environment

Just as a side note, this library has been overall great to use for animating height changes! The API is really simple and intuitive, so nice work!

Stanko commented 3 years ago

Hello @bmaclean, I've made it like that on purpose. But if you have a legit use case, let's think about solving it. I would love to see a real world example, it would be great if you can provide me with one.

I need to revisit how overflow attribute is set in percentage scenario, it may need some refactoring to allow the override. Maybe I can add a separate prop, something like __dangerouslyOverrideStyle which would allow overriding, and people wouldn't use it by mistake.

Let me know what you think. Cheers!

P.S. I'm also thinking about writing a new version with hooks, but I can't find the time atm.

EDIT: Thank you for the detailed issue description 🙌

eostrom commented 2 years ago

I'm not sure this is the same issue, but we ran into a problem with overflow cutting off the edges of our <ol> counters during the transition:

2021-11-17 at 5 43 PM

Code Sandbox.

There are workarounds, and we'll implement one, but I thought I'd chime in here as a kind of data point.

Stanko commented 2 years ago

Hey @eostrom, Unfortunately, I don't think there is a way to avoid that. The content is wrapped in a div, which then changes height using transitions. In order for it to reveal content it has to have overflow hidden. If it had overflow visible content would just show up. I would suggest adding padding, and maybe even negative margins to compensate for padding.

Stanko commented 2 years ago

I omitted height and overflow from style prop type definition and added a note in the readme.

https://github.com/Stanko/react-animate-height/commit/762058a6153f59f18a6a1e8b64135ec62b024993

Closing it.