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

Prevent focusing hidden/disabled elements #263

Merged
merged 6 commits into from
May 8, 2024

Conversation

TylerJDev
Copy link
Contributor

@TylerJDev TylerJDev commented Dec 19, 2023

Closes #257

This PR attempts to address the issue where if an element is disabled or hidden after the focusZone function has ran, the focus trap no longer works. I've adjusted the mutation observer that watches for added or removed nodes within the focusZone container to now track the hidden and disabled attributes.

If either attribute is applied or removed, they will added/removed from the focus trap respectively.

Copy link

changeset-bot bot commented Dec 19, 2023

🦋 Changeset detected

Latest commit: a54cafc

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

@TylerJDev TylerJDev marked this pull request as ready for review December 21, 2023 15:46
@TylerJDev TylerJDev requested review from a team and langermank December 21, 2023 15:46
}
})

observer.observe(container, {
subtree: true,
childList: true,
attributeFilter: ['hidden', 'disabled'],
attributeOldValue: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this narrow down the observed set of elements in the first place? Seems like the filter was broader and the attributeFilter narrows it down. Just trying to understand how the observer works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

attributeFilter allows us to watch for any changes of the attributes in the array, in addition to the rules that we already have. So it won't narrow our existing ruleset down, as it will still only run the callback if either a node is added/removed (childList), or if an element receives/removes the hidden, or disabled attributes within the container. This essentially just slightly widens what we watch for.

Copy link
Contributor

github-actions bot commented Mar 3, 2024

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 3, 2024
@TylerJDev TylerJDev removed the Stale label Mar 4, 2024
Copy link
Contributor

github-actions bot commented May 5, 2024

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 5, 2024
@TylerJDev TylerJDev removed the Stale label May 6, 2024
@TylerJDev
Copy link
Contributor Author

Hey team! I was wondering if I could get a review on this PR? For context it's making sure that disabled or hidden elements are removed from the focus zone if they are disabled/hidden after it has been initiated. It'll also add the elements back if they are no longer disabled/hidden. This is primarily useful when using focus-zone outside of React.

@primer/engineer-reviewers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants