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

[A11Y] Add focus traps to modals and nav drawer #3018

Merged
merged 19 commits into from
Nov 21, 2021

Conversation

davwheat
Copy link
Member

@davwheat davwheat commented Aug 12, 2021

Fixes #2663
Fixes #2665

Please merge #3007 before this PR!

Changes proposed in this pull request:

  • Add focus trap to modals
  • Add focus trap to drawer on phone viewports
  • Auto close drawer when leaving phone viewport
  • Replace deprecated JQuer ($.click() -> $.on('click'))
  • Use requestAnimationFrame instead of setTimeout

Reviewers should focus on:

Screenshot

Confirmed

  • Frontend changes: tested on a local Flarum installation.

@davwheat davwheat added the type/accessibility Issues relating to accessibility (keyboard navigation, screenreaders, text contrast, etc.) label Aug 12, 2021
@davwheat davwheat self-assigned this Aug 12, 2021
@davwheat davwheat added this to the 1.1 milestone Aug 12, 2021
js/package.json Outdated Show resolved Hide resolved
js/src/common/components/ModalManager.js Outdated Show resolved Hide resolved
js/src/common/components/ModalManager.js Outdated Show resolved Hide resolved
js/src/common/components/ModalManager.js Outdated Show resolved Hide resolved
js/src/common/components/ModalManager.js Outdated Show resolved Hide resolved
js/src/common/utils/Drawer.js Outdated Show resolved Hide resolved
js/src/common/utils/Drawer.js Outdated Show resolved Hide resolved
js/src/common/utils/Drawer.js Outdated Show resolved Hide resolved
js/src/common/utils/Drawer.js Outdated Show resolved Hide resolved
js/src/common/utils/Drawer.js Outdated Show resolved Hide resolved
@davwheat davwheat force-pushed the dw/2663-2665-add-focus-trap branch from 42eb187 to b1d68e7 Compare September 3, 2021 15:53
Copy link
Member

@dsevillamartin dsevillamartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested locally, but code looks good aside from a few comments.

js/src/common/utils/Drawer.js Outdated Show resolved Hide resolved
js/src/common/components/ModalManager.js Outdated Show resolved Hide resolved
js/src/forum/utils/KeyboardNavigatable.ts Show resolved Hide resolved
Copy link
Member

@dsevillamartin dsevillamartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean to approve.

@davwheat davwheat force-pushed the dw/2663-2665-add-focus-trap branch 3 times, most recently from 2790899 to 4f19eab Compare September 10, 2021 22:05
@davwheat
Copy link
Member Author

YOU WILL GET CO-AUTHORSHIP OF THAT COMMIT IF IT IS THE LAST THING I DO

@askvortsov1 askvortsov1 modified the milestones: 1.1, 1.2 Sep 28, 2021
@davwheat
Copy link
Member Author

Rebase in progress...

@datitisev you still have "requested changes" marked on this PR -- I think I addressed your comment about a breaking change, but I'll re-request your review and you can either approve, dismiss the request, or request other changes.

@davwheat davwheat requested review from dsevillamartin and removed request for dsevillamartin November 19, 2021 13:19
@davwheat davwheat force-pushed the dw/2663-2665-add-focus-trap branch from 4f19eab to 23a03eb Compare November 19, 2021 13:56
@dsevillamartin dsevillamartin removed their request for review November 21, 2021 19:34
@davwheat davwheat merged commit eaf1b86 into master Nov 21, 2021
@davwheat davwheat deleted the dw/2663-2665-add-focus-trap branch November 21, 2021 19:44
@SychO9 SychO9 removed this from the 1.2 milestone Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/accessibility Issues relating to accessibility (keyboard navigation, screenreaders, text contrast, etc.)
Projects
None yet
4 participants