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

Cat-dropdown: focus-trap library conflicts with FocusMonitor(@angular/cdk/a11y) #496

Open
glushkova91 opened this issue Apr 15, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@glushkova91
Copy link
Collaborator

When user uses Tab for navigation inside cat-dropdown the first element is not reachable. It's reproducible only when FocusMonitor service is imported and provided into any component in the tree.

The reason underhood is that the focus-trap library uses preventDefault inside Tab keydown event handler in order to not let browser natively navigate the the next focusable element after programmatically setup the focus on the first element:
https://github.com/focus-trap/focus-trap/blob/master/index.js#L762
In the same time InputModalityDetector class, which is used by FocusMonitor, add event listener to keydown event with configuration passive: true
https://github.com/angular/components/blob/main/src/cdk/a11y/input-modality/input-modality-detector.ts#L196
which prevent preventDefault (ironically).

But to be honest I'm not sure why keydown event handler from focus-trap library which was defined with passive: false is impacted by options of event listener from InputModalityDetector.

How we can possible resolve it? Maybe we can start from creating the issue ticket in focus trap repo, seems like they are responded quickly. In the case it's gonna be resolved the only thing we need to do is to update the library version.

@glushkova91
Copy link
Collaborator Author

focus-trap/focus-trap#1165 - added a ticket to focus-trap repo

@glushkova91 glushkova91 added the bug Something isn't working label Apr 24, 2024
@glushkova91
Copy link
Collaborator Author

This is a Zone.js issue:
There is an open ticket for that in Angular repo and draft PR.
I'd suggest to get rid of focus-trap library and implement our own focus trap service with approach different from focus-trap library: either we can keep keydown listener but somehow get rid of preventDefault inside of handler, or instead of keydown we can listen for focus event

@stefcameron
Copy link

At the risk of butting my nose in where it may not belong,

I'd suggest to get rid of focus-trap library and implement our own focus trap service with approach different from focus-trap library: either we can keep keydown listener but somehow get rid of preventDefault inside of handler, or instead of keydown we can listen for focus event

That's one way to go. On the other hand, the zone.js issue has been open for over 2 years now, and the PR has been in draft mode for over a year. And building a "focus trap" library is non-trivial, unless you plan on implementing something very targeted with very limited functionality to suit your specific needs.

Perhaps another way to address this would be to fork zone.js, merge the PR in your fork, and use your version of zone.js (assuming you have a direct dependency on the package; if it's a downstream dependency, this may not work).

And I guess another solution would be to fork focus-trap and "fix" it to work with zone.js in its current state the way you think it should. You may then at least still be able to take in future updates to focus-trap this way while still maintaining your modifications, and saving you the hassle of building your own "focus trap" solution.

@glushkova91
Copy link
Collaborator Author

Hello @stefcameron! Thanks for stopping by,

  1. The problem with zone.js patching is that we have multiple micro-services which use Angular and consume catalyst library. I believe it's gonna be too aggressive solution in this case to patch zone.js in every micro-service to fix the problem of UI components library.
  2. I like the idea with focus-trap fork. One of the possible ways which might work is adding focusable anchors in the start and the end of focus trap area (example), and potentially it can be done on top of focus-trap library.

But since there is a workaround with isKeyForward and isKeyBackward which allows user to reach the first element, this bug has low priority and I'm not sure what happens first: Angular will merge PR with zone.js fix and we install the update which let us continue using your library or we will have to fix it on our side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants