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

Fix #6095 'dialog focus display none' #6434

Closed

Conversation

KirilCycle
Copy link
Contributor

@KirilCycle KirilCycle commented Apr 21, 2024

Defect Fixes

Fix #6095

Focus Loop

PR implements a loop that iterates through an array of focusable elements. It keeps trying to focus on the next element until:

  • Focus successful: The loop successfully focuses on an element.
  • Array ends: There are no more elements in the array.

Note

Works fine with deep nested focusable elements in display none parent
Without 'onKeyDown' method tab pressing functions well

Video description:

Screen.Recording.2024-04-21.at.14.03.53.mov

Copy link

vercel bot commented Apr 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Apr 21, 2024 1:10pm
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Apr 21, 2024 1:10pm

@KirilCycle KirilCycle changed the title Fix/dialog focus display none Fix #6095 'dialog focus display none' Apr 21, 2024
@melloware
Copy link
Member

@KirilCycle i think I know what is going on here let me do some testing!

@KirilCycle
Copy link
Contributor Author

KirilCycle commented Apr 21, 2024

@melloware I found a new easier solution, we can use focusableElement.offsetParent !== null at 'getFocusableElements' method, this will prevent adding invisible elements to result array

@melloware
Copy link
Member

@KirilCycle see my PR. I fixed it how PrimeVue is doing it using the FocusTrap component.

#6435

@nitrogenous nitrogenous self-requested a review April 22, 2024 09:51
@melloware
Copy link
Member

I am going to close this one in favor of my FocusTrap fix PR.

@melloware melloware closed this Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dialog: (accessibility) "display: none" inside dialog breaks focus tabbing
2 participants