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.

Sign up for GitHub

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

Jump to bottom

[A11Y] Triggering click on drawer button can cause layered backdrops #2663

Closed
Tracked by #3360
davwheat opened this issue Mar 7, 2021 · 2 comments · Fixed by #3018
Closed
Tracked by #3360

[A11Y] Triggering click on drawer button can cause layered backdrops #2663

davwheat opened this issue Mar 7, 2021 · 2 comments · Fixed by #3018
Assignees
@davwheat
Labels
type/accessibility Issues relating to accessibility (keyboard navigation, screenreaders, text contrast, etc.)
Milestone
1.2

Comments

@davwheat
Copy link
Member

davwheat commented Mar 7, 2021

Bug Report

Current Behavior
On mobile viewports, focusing the nav drawer toggle button and triggering a click on it multiple times (e.g. via space) causes multiple backdrops to be shown, but only one of these closes when closing the drawer.

1l96mO

Steps to Reproduce

  1. Shrink viewport to mobile size
  2. Use TAB to focus the drawer toggle button
  3. Hit SPACE twice
  4. Close the drawer
  5. Backdrop remains

Expected Behavior
Only one backdrop should be created.

Possible solution(s)

I have two ideas:

  1. We prevent this situation happening by checking if the drawer is currently open on button click. This fixes this situation.
  2. We put a check in the Drawer class to prevent it being opened if App has drawerOpen. This fixes it in case any exts open the drawer or if something else can cause this. We may also wish to consider removing all .drawer-backdrop as a precautionary measure when closing, too.
@askvortsov1
Copy link
Member

I'm assuming this is primarily problematic for accessibility devices? That's probably why we hadn't caught it yet. If so, this might be good to add to the mega-issue.

@davwheat
Copy link
Member Author

davwheat commented Mar 7, 2021

Hehe I discovered it while working on a fix for #2565.

I'll add it now.

@davwheat davwheat changed the title Triggering click on drawer button can cause layered backdrops [A11Y] Triggering click on drawer button can cause layered backdrops Mar 7, 2021
@davwheat davwheat added the type/accessibility Issues relating to accessibility (keyboard navigation, screenreaders, text contrast, etc.) label Mar 15, 2021
@davwheat davwheat added this to the 0.1 milestone Mar 16, 2021
@askvortsov1 askvortsov1 modified the milestones: 1.0, 1.1 Apr 16, 2021
davwheat added a commit that referenced this issue Aug 12, 2021
@davwheat
Add focus trap to Modals
33105cd 
Fixes #2663
@davwheat davwheat mentioned this issue Aug 12, 2021
Merged
1 task
davwheat added a commit that referenced this issue Sep 3, 2021
@davwheat
Add focus trap to Modals
e55a38f 
Fixes #2663
@askvortsov1 askvortsov1 modified the milestones: 1.1, 1.2 Sep 28, 2021
@SychO9 SychO9 added this to Roadmap Oct 1, 2021
davwheat added a commit that referenced this issue Nov 19, 2021
@davwheat
Add focus trap to Modals
08f4bdb 
Fixes #2663
Repository owner moved this from Todo to Done in Roadmap Nov 21, 2021
davwheat added a commit that referenced this issue Nov 21, 2021
@davwheat @dsevillamartin
fix(a11y): add focus traps to modals and nav drawer (#3018)
eaf1b86 
* Add focus trap util

* Add focus trap to Modals

Fixes #2663

* Split tab press into `onTab` handler

* Remove deprecated code

* Use requestAnimationFrame instead of setTimeout

* Reduce code duplication

* Implement focus trap in nav drawer

Fixes #2665

* Hide drawer when window is resized to be bigger

Fixes issue where focus trap would remain on the drawer when it is
just the app header, if the drawer was opened then the window was
made larger.

* Simplify conditional function calls

* Fix modal focus trap

* Remove debug code

* Simplify resize handler conditional statements

* Add info about reasoning of resize handler

* Prefer native JS methods over jQuery

* Update conditional function call to handle `undefined`

* Expose screen sizes as CSS custom properties

* Use `window.matchMedia` rather than resize handler

* Fix spelling error

Co-authored-by: David Sevilla Martin <me@datitisev.me>

* Remove breaking change

Co-authored-by: David Sevilla Martin <me@datitisev.me>
@SychO9 SychO9 removed this from Roadmap Dec 30, 2021
@davwheat davwheat mentioned this issue Mar 28, 2022
Open
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@davwheat davwheat

Labels
type/accessibility Issues relating to accessibility (keyboard navigation, screenreaders, text contrast, etc.)
Projects
None yet
Milestone
1.2
Development

Successfully merging a pull request may close this issue.

[A11Y] Add focus traps to modals and nav drawer flarum/framework
2 participants
@davwheat @askvortsov1