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

feat: focus utils #1556

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: focus utils #1556

wants to merge 1 commit into from

Conversation

renrizzolo
Copy link
Contributor

@renrizzolo renrizzolo commented Dec 21, 2022

Description

NB This is part 1 of an effort to refactor popover and add a new dropdown menu component.

  • Add some focus utils, namely a tree walker that allows us to get all focussable/tabbable nodes within an element

  • Add DismissableFocusTrap component. As this is intended for use with portaled UI elements like modals, menus and popovers, it combines focus trapping and escape/click outside dismiss behaviour. This could be separated into 2 components (FocusTrap / Dismissable) if preferred.

    • focusses the first focusable element on mount
    • focusses the trigger element on unmount
    • keeps focus within the component
    • optionally loop focus
    • callbacks for escape, click outside, tab after last element, tab before first element
  • Implement the focus trap in the ActionPanel component

    • Add prop to allow overriding the close on escape behaviour
    • Add accessible aria attributes/role to modal

Reference: https://www.w3.org/WAI/ARIA/apg/patterns/dialogmodal/

Does this PR introduce a breaking change?

  • Yes
  • No

Manual testing step?

Screenshots (if appropriate):

Kapture 2022-12-21 at 14 23 32

@codecov
Copy link

codecov bot commented Dec 21, 2022

Codecov Report

Merging #1556 (4e19741) into master (8078b67) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 4e19741 differs from pull request most recent head 07a5dc7. Consider uploading reports for the commit 07a5dc7 to get more accurate results

@@            Coverage Diff            @@
##            master     #1556   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           96       100    +4     
  Lines         1703      1737   +34     
  Branches       477       501   +24     
=========================================
+ Hits          1703      1737   +34     
Impacted Files Coverage Δ
src/components/ActionPanel/index.jsx 100.00% <100.00%> (ø)
src/components/DismissibleFocusTrap/index.jsx 100.00% <100.00%> (ø)
src/hooks/useArrowFocus.js 100.00% <100.00%> (ø)
src/hooks/useClickOutside.js 100.00% <100.00%> (ø)
src/lib/focus.js 100.00% <100.00%> (ø)

... and 13 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@sonarcloud
Copy link

sonarcloud bot commented Jan 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Mar 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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

Successfully merging this pull request may close these issues.

None yet

1 participant