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

Offcanvas navbar expand remaining aria-hidden #35577

Closed
3 tasks done
florianlacreuse opened this issue Dec 20, 2021 · 9 comments · Fixed by #35589
Closed
3 tasks done

Offcanvas navbar expand remaining aria-hidden #35577

florianlacreuse opened this issue Dec 20, 2021 · 9 comments · Fixed by #35589

Comments

@florianlacreuse
Copy link
Contributor

Prerequisites

Describe the issue

Following PR #34273.

An offcanvas navbar with .navbar-expand-${breakpoint} can lead to a remaining aria-hidden="true", if the offcanvas is closed and the navbar is expanded again. This edge case happen when resizing the window on desktop or rotating an ipad (for instance).

Reduced test cases

https://codepen.io/florianlacreuse/pen/wvreVxN

What operating system(s) are you seeing the problem on?

Linux

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

v5.1.3

@florianlacreuse
Copy link
Contributor Author

florianlacreuse commented Dec 22, 2021

May be aria-hidden="true" attribute is not required for an offcanvas element since it already has the css property visibility: hidden;?

For instance, collapsed elements don't have the aria-hidden attribute, just a basic display: none;: https://getbootstrap.com/docs/5.1/components/collapse/#example

@GeoSot
Copy link
Member

GeoSot commented Dec 22, 2021

@patrickhlauke can you help us on this?

@patrickhlauke
Copy link
Member

I'll have a look...

@ffoodd
Copy link
Member

ffoodd commented Dec 22, 2021

I think @florianlacreuse is right regarding visibility. AFAIK it effectively removes the element from the AOM. 🤔

@patrickhlauke
Copy link
Member

yes, visibility:hidden, like display:none, already hides things from the accessibility tree, so this could potentially be used - though i believe it's done on purpose currently because it just reuses the regular scripting for modal https://getbootstrap.com/docs/5.1/components/modal/ (since off-canvas is just a variant of a modal).

more fundamentally, the offcanvas component doesn't currently deal with (as in clean up after itself) changes in viewport size. a more dramatic problem: open the offcanvas and resize the page while it's open ... you'll end up with the semi-transparent overlay over the page.

I'd say for both of these the more robust solution would be to check for when the offcanvas disappears/isn't used anymore, and do a proper graceful handoff (run the code to "close" the offcanvas, to remove the overlay; and then remove the aria-hiddens inside what was the offcanvas)

@patrickhlauke
Copy link
Member

ah, looking even at the modal code itself, i see it uses display:none so its addition or aria-hidden="true" is also redundant. might need a nice little simplification/refactor of the modal and offcanvas code then to just rely on display:none/visibility:hidden and forego the aria-hidden

(though the other issue of the overlay remaining there when resizing an open offcanvas page remains...probably worth filing a separate issue for that though)

@patrickhlauke
Copy link
Member

did some refactoring... #35589

@florianlacreuse
Copy link
Contributor Author

@patrickhlauke Thank you for your feedback and the PR!

I agree about the need of offcanvas navbar to deal with viewport size changes, having this behavior built-in would be perfect.

@patrickhlauke
Copy link
Member

The separate issue I filed for the overlay behaviour #35594

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

Successfully merging a pull request may close this issue.

6 participants
@XhmikosR @patrickhlauke @ffoodd @florianlacreuse @GeoSot and others