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

Add IterateFocusableElements options to focusZone #235

Merged
merged 5 commits into from
Oct 23, 2023

Conversation

JelloBagel
Copy link
Contributor

@JelloBagel JelloBagel commented Oct 20, 2023

Background:

We ran into a bug when using useFocusZone in our component where when the css visibility is set to "hidden" the user is unable to tab past the element. I noticed iterateFocusableElements has a strict option that checks for elements with hidden visibility but useFocusZone does not support the strict argument to be passed through to iterateFocusableElements. We would like to enable strict mode in useFocusZone to remove visibility: hidden elements as focusable elements.

More details on the bug: https://github.com/github/issues/issues/7911

What are you trying to accomplish?

Add ability in focus zone to pass through settings options to iterateFocusableElements. Also, I added an additional check to not focus on elements that have "display: none" on the element and its parents

@changeset-bot
Copy link

changeset-bot bot commented Oct 20, 2023

🦋 Changeset detected

Latest commit: 00478de

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/behaviors Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@JelloBagel JelloBagel marked this pull request as ready for review October 20, 2023 01:47
@JelloBagel JelloBagel requested review from a team and broccolinisoup October 20, 2023 01:47
<button>Peach</button>
</div>
</div>
<button tabIndex={-1}>Cantaloupe</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm missing something. Doesn't this make it non focusable with keyboard? How does this button have a place in the focusable array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The iterateFocusableElements will return "any element with tabindex explicitly set can be focusable, even if it's set to "-1"" if onlyTabbable option is false

@JelloBagel JelloBagel merged commit 3463e25 into main Oct 23, 2023
9 checks passed
@JelloBagel JelloBagel deleted the jellobagel-focus-zone-options branch October 23, 2023 17:30
@primer-css primer-css mentioned this pull request Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants