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

DOM: Limit single tabbable radio input by name #14128

Merged
merged 2 commits into from
Mar 15, 2019

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 26, 2019

Closes #6988

This pull request seeks to enhance the behavior of tabbables.find to include only a single radio input for a given name, preferring the checked input, falling back to the first in the tabindex-sorted set.

Testing Instructions:

Repeat both sets of Steps to Reproduce from #6988 (comment) , verifying the expected behavior.

@aduth aduth added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] DOM /packages/dom labels Feb 26, 2019
@aduth aduth requested a review from afercia February 26, 2019 21:07
Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

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

From an accessibility perspective, works nicely. Tested on the Visibility panel: tabbing is now constrained within the panel. Wish there was a simpler way to solve it 😬 It's self container. Has test. Brilliant, thanks.
(will leave code review to the author himself 😉)

@aduth aduth force-pushed the fix/6988-tabbable-radio-by-name branch from 4949c79 to 956cf39 Compare February 27, 2019 14:54
@gziolo gziolo added this to the 5.3 (Gutenberg) milestone Mar 11, 2019
@aduth aduth force-pushed the fix/6988-tabbable-radio-by-name branch from 956cf39 to 2809765 Compare March 11, 2019 19:37
@@ -30,6 +35,46 @@ function getTabIndex( element ) {
export function isTabbableIndex( element ) {
return getTabIndex( element ) !== -1;
}
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: An empty line above maybe

@aduth aduth force-pushed the fix/6988-tabbable-radio-by-name branch from 2246544 to 7f9bbda Compare March 15, 2019 18:06
@aduth aduth merged commit 1019c17 into master Mar 15, 2019
@aduth aduth deleted the fix/6988-tabbable-radio-by-name branch March 15, 2019 19:48
youknowriad pushed a commit that referenced this pull request Mar 20, 2019
* DOM: Limit single tabbable radio input by name

* DOM: Avoid consolidating unnamed radio inputs
youknowriad pushed a commit that referenced this pull request Mar 20, 2019
* DOM: Limit single tabbable radio input by name

* DOM: Avoid consolidating unnamed radio inputs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] DOM /packages/dom
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants