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

Dialog: Escape listener closes everything always #16955

Open
LoaderB0T opened this issue Dec 5, 2024 · 1 comment · May be fixed by #17052
Open

Dialog: Escape listener closes everything always #16955

LoaderB0T opened this issue Dec 5, 2024 · 1 comment · May be fixed by #17052
Labels
Resolution: Help Wanted Issue or pull request requires extra help and feedback Status: Pending Review Issue or pull request is being reviewed by Core Team
Milestone

Comments

@LoaderB0T
Copy link
Contributor

Describe the bug

When opening a panel of sorts (dropdown, multi-select, calendar, etc.) and pressing escape to close it (accessibility), not only does the panel close but also the dialog.

As far as I can tell, primeng just globally listens for escape in some locations and closes everything that's opened. It would be great if only the latest / top level would react to the escape key. This seems to have been an issue for a long time.

I would be very happy to create a PR with a proposal for a fix, but I would require some guidance beforehand on how I should go about fixing the issue.


Idea 1: Using the current focus (check if the panel contains document.activeElement) or el.matches(':focus') to check if the panel should be closed. issues: The element does not necessarily have the focus

Idea 2: Make the global escape listener smarter and only close the latest still-existing element (Some sort of global escape listener registry)

Idea 3: Again making the global escape listener a bit smarter, but by using the computed z Index of a panel to determine which one is the top level. This might not work when multiple dialogs are opened in non-modal mode. There the focus would make more sense.

Idea 4: Escape listeners have a priority-based execution order. Panels like calendar, dropdown etc have a higher prior and are executed first. The listener gives feedback on whether something closed. If something has been closed no further listeners are executed. Basically like event.stopPropagation(). This would at least solve the issue for controls within dialog, but does not change anything for multiple dialogs (which would be fine for us at the moment as multiple dialogs opened are not ideal anyway and it could be changed in the future with an additional issue).


My stomach tells me option 4 might work and not be too complicated and reliable.

Open for more ideas and discussion. I might also provide more varieties of use-cases or edge cases for the problem in the stackblitz to have more test cases.

Feedback & ideas are appreciated. For our company accessibility is becoming more & more important and small seeming issues like this one are a real pain for people who rely on keyboard usage.

Thanks!

Environment

https://stackblitz.com/edit/github-bwbrvj?file=src%2Fapp%2Fapp.component.html

Reproducer

https://stackblitz.com/edit/github-bwbrvj?file=src%2Fapp%2Fapp.component.html

Angular version

18.0.1

PrimeNG version

17.18.12 & 18.0.0-rc.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

20.18.0

Browser(s)

All

Steps to reproduce the behavior

See stackblitz!

Expected behavior

Only the top level panel closes

@LoaderB0T LoaderB0T added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Dec 5, 2024
@LoaderB0T LoaderB0T changed the title Dialog: Title Dialog: Escape listener closes everything always Dec 5, 2024
@mertsincan mertsincan added this to the 19.x milestone Dec 11, 2024
@github-project-automation github-project-automation bot moved this to Review in PrimeNG Dec 11, 2024
@mertsincan mertsincan added Status: Pending Review Issue or pull request is being reviewed by Core Team Resolution: Help Wanted Issue or pull request requires extra help and feedback and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Dec 11, 2024
Copy link

Due to PrimeNG team's busy roadmap, this issue is available for anyone to work on. Make sure to reference this issue in your pull request. ✨ Thank you for your contribution! ✨

@LoaderB0T LoaderB0T linked a pull request Dec 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Help Wanted Issue or pull request requires extra help and feedback Status: Pending Review Issue or pull request is being reviewed by Core Team
Projects
Status: Review
Development

Successfully merging a pull request may close this issue.

2 participants