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

Set "display: none" when height is 0 #16

Closed
apers opened this issue Aug 8, 2017 · 7 comments
Closed

Set "display: none" when height is 0 #16

apers opened this issue Aug 8, 2017 · 7 comments

Comments

@apers
Copy link

apers commented Aug 8, 2017

Would it be possible to set "display: none" when height is 0 so the content doesn't take tab focus when hidden?

@Stanko
Copy link
Owner

Stanko commented Aug 8, 2017

Hello @apers,
Recently I stumbled upon the same problem, I'll release new version today or tomorrow.
Cheers!

@Stanko
Copy link
Owner

Stanko commented Aug 8, 2017

@apers I just released version 0.9.10
Can you please test and close the issue if it works, thanks!

Commit: e52a3b9

@apers
Copy link
Author

apers commented Aug 8, 2017

That was quick!
Tested and working, thanks!

@apers apers closed this as completed Aug 8, 2017
@nizioleque
Copy link

nizioleque commented Oct 11, 2023

@Stanko I think this functionality is breaking something else - it is causing animating from 0 to another value (using ResizeObserver) to not work.

This is what's happening, if I understand correctly:

  • I render AnimateHeight with height={x} where x is the contentHeight from a div observed with the ResizeObserver API - initially the observed div is empty so x is 0
  • the content div gets display: none (because of this feature, I guess)
  • then I add contents to the observed div, but because it's inside a container with display: none the browser doesn't calculate the size - the contentHeight of my observed div is still 0
  • the animation is not working because x is still 0

Could you please let us disable this feature or find another way to fix the tab focus issue without display: none?

Here's a workaround I came up with:

  • I set contentClassName='rah-content' on the AnimateHeight component
  • I add the following CSS: .rah-content { display: block !important; }

@tuff
Copy link

tuff commented Nov 9, 2023

@Stanko I'd also like to request that the display: none behaviour be opened to configuration.

I would like elements within zero-height containers to be able to receive focus, so that the user can tab to them and have the container be automatically expanded. I can also see where and why a developer would not want hidden elements to be focusable - both options are useful.

Currently I am using a similar !important workaround as @nizioleque, which seems brittle.

@Stanko
Copy link
Owner

Stanko commented Nov 10, 2023

hey, it is a valid point. I'll try to add it today or during the weekend.

@Stanko
Copy link
Owner

Stanko commented Nov 10, 2023

I just published v3.2.3 with a new disableDisplayNone prop.

I added an additional check - when height is zero and there is no children elements, display: none won't be applied.

Hope this helps, cheers!

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

No branches or pull requests

4 participants