Dont set aria hidden on large viewports

[koddsson]

Hello 👋🏻

We're hitting this snag in our app where the sidebar is set with [aria-hidden="true"] when initialized. Apart from being a pretty serious accessibility issue it's also erroring in some cases for us in Chrome with the following in error:

It looks like the sidebar is always initialized as hidden which does make sense in the context of a smaller viewport as it should render hidden and then toggled on and off. It makes less sense on larger viewports where we expect the sidebar to be rendered initially and then optionally users should be able to hide it.

This PR checks to see if we are rendering initially on a small viewport (640px or less) and sets the visibility state accordingly. I also made sure to read the CSS variable --small-viewport so that this size can be changed. I'm not sure if that's the way we'd like to do this. I thought there might be a specific Tailwind variable but didn't find one. We could also read this value from a data attribute if that makes more sense? It would mean that we'd be less likely to clash with a CSS variable set by apps.

Let me know what you think :)

[Lexachoc]

Here is the related issue with reproducible code and videos showing the exact issue.

#943 (comment)
#943 (comment)
#943 (comment)

[zoltanszogyenyi]

Hey @koddsson,

Thanks for the PR! Why do we set the visible variable to something else other than false?

That sets the visibility to false by default, regardless of smaller or larger devices because the visibility functionality should be the same. From what I understand we have the issue with the aria hidden attribute, so why not add the mobile/desktop device logic only to that?

This could very well break a lot of the base functionality of the drawer, I'll also check it locally in a few days.

Thanks,
Zoltan

[zoltanszogyenyi]

Also, I think the main issue here is not the usage of aria hidden on larger devices, but rather the possibility to focus or not focus elements within it, so what we should actually do is remove the focus ability for the inside elements when the drawer is actually hidden, both on desktop and mobile devices :)

[koddsson]

This could very well break a lot of the base functionality of the drawer, I'll also check it locally in a few days.

Any news on this? We've been running this patch in production since I opened this PR and haven't had any problems.

Thanks for the PR! Why do we set the visible variable to something else other than false?

In this PR? I'm setting _visible to match the functionality of the component. When the component is initialized on a small viewport it's initially hidden and can be then toggled on and off.

Large viewport	Small viewport

You can see this issue in action on https://flowbite.com/docs/components/sidebar/#default-sidebar.

Screen.Recording.2024-09-30.at.08.59.34.mov

If you look at the HTML of that demo you can see where the sidebar is marked with aria-hidden=true while it's clearly not hidden at all.