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

feat(is-in-tab-order): add isInTabOrder to commons #3619

Merged
merged 13 commits into from
Aug 30, 2022
Merged

feat(is-in-tab-order): add isInTabOrder to commons #3619

merged 13 commits into from
Aug 30, 2022

Conversation

dbowling
Copy link
Contributor

@dbowling dbowling commented Aug 24, 2022 •
edited
Loading

  • add axe.commons.isInTabOrder()
  • replace uses of isFocusable with isInTabOrder when isFocusable also checked for negative tabindex

references: #3500

@dbowling dbowling marked this pull request as ready for review August 24, 2022 23:20
@dbowling dbowling requested a review from a team as a code owner August 24, 2022 23:20
WilcoFiers
WilcoFiers previously requested changes Aug 25, 2022
View reviewed changes
Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Where appropriate, can you replace uses of isFocusable with isInTabOrder?

lib/commons/dom/is-in-tab-order.js Show resolved Hide resolved
lib/commons/dom/is-in-tab-order.js Outdated Show resolved Hide resolved
test/commons/dom/is-in-tab-order.js Show resolved Hide resolved
straker
straker previously requested changes Aug 26, 2022
View reviewed changes
test/commons/dom/is-in-tab-order.js Show resolved Hide resolved
test/commons/dom/is-in-tab-order.js Outdated Show resolved Hide resolved
test/commons/dom/is-in-tab-order.js Outdated Show resolved Hide resolved
dbowling and others added 2 commits August 26, 2022 11:40
@dbowling @straker
Apply suggestions from code review
096c6b5 
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
@dbowling
assert against text node
270cd7c
@dbowling dbowling dismissed WilcoFiers’s stale review August 26, 2022 20:38

All the instances of isFocusable that I could find which were accounting for negative tabindex have been updated

WilcoFiers
WilcoFiers previously requested changes Aug 29, 2022
View reviewed changes
lib/checks/keyboard/no-focusable-content-evaluate.js Outdated Show resolved Hide resolved
lib/commons/dom/get-tabbable-elements.js Outdated Show resolved Hide resolved
lib/commons/dom/inserted-into-focus-order.js Outdated Show resolved Hide resolved
@dbowling dbowling dismissed WilcoFiers’s stale review August 29, 2022 21:02

Resolved changes requested.

@dbowling dbowling merged commit 77afe90 into develop Aug 30, 2022
@dbowling dbowling deleted the 3500-isInTabOrder branch August 30, 2022 17:20
@Shubhabrata08 Shubhabrata08 mentioned this pull request Jan 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WilcoFiers WilcoFiers WilcoFiers approved these changes

@straker straker straker left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dbowling @WilcoFiers @straker