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 #14010, #13162, #13768, #13082, #10472 #15263

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

BGBRWR
Copy link
Contributor

@BGBRWR BGBRWR commented Apr 11, 2024

fix #14010, fix #13162, fix #13768, fix #13082, fix #10472

Copy link

vercel bot commented Apr 11, 2024

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primeng ⬜️ Ignored (Inspect) Visit Preview Apr 11, 2024 8:13pm

@BGBRWR BGBRWR changed the title Issue 14010 Fix #14010, #13162, #13768, #13082, #10472 Apr 11, 2024
@cetincakiroglu cetincakiroglu added the Status: Pending Review Issue or pull request is being reviewed by Core Team label Apr 17, 2024
@cetincakiroglu
Copy link
Contributor

Hi @BGBRWR

Thanks for fixing all these issues, we appreciate the effort and support!

@cetincakiroglu cetincakiroglu merged commit c4163b7 into primefaces:master Apr 18, 2024
2 checks passed
@kevinley
Copy link
Contributor

kevinley commented May 8, 2024

@cetincakiroglu @BGBRWR this PR introduced some unexpected behaviour. Now anything focusable inside the dialog gets focus, for instance I have a form inside a modal, where the first focusable element is a p-calendar, this opens the calendar overlay immediately, but we don't want that, because it covers other form elements.

Wether to focus the first focusable element within a dialog should be configurable with an option.

@dodesheide
Copy link

dodesheide commented May 8, 2024

After this fix i run into errors with p-dialog and headless mode:

#15496

@BGBRWR
Copy link
Contributor Author

BGBRWR commented May 13, 2024

@cetincakiroglu @BGBRWR this PR introduced some unexpected behaviour. Now anything focusable inside the dialog gets focus, for instance I have a form inside a modal, where the first focusable element is a p-calendar, this opens the calendar overlay immediately, but we don't want that, because it covers other form elements.

Wether to focus the first focusable element within a dialog should be configurable with an option.

You can set focusOnShow on the dialog to false.

@kevinley
Copy link
Contributor

@cetincakiroglu @BGBRWR this PR introduced some unexpected behaviour. Now anything focusable inside the dialog gets focus, for instance I have a form inside a modal, where the first focusable element is a p-calendar, this opens the calendar overlay immediately, but we don't want that, because it covers other form elements.
Wether to focus the first focusable element within a dialog should be configurable with an option.

You can set focusOnShow on the dialog to false.

Tried that, doesn't work. Your code seems to ignore the flag

@rbbp23
Copy link

rbbp23 commented May 23, 2024

Another issue introduced by this PR.
On a dynamic dialog with a custom header and a footer template, I get a kind of infinite focus with the following error:

core.mjs:6531 ERROR RangeError: Maximum call stack size exceeded
at DomHandler.getFocusableElement (primeng-dom.mjs:553:31)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:592:36)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)
at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)

@TheWrightDev
Copy link
Contributor

@rbbp23 Have you found any solution to this? This error completely breaks our ui ordering when it occurs, something weird happens with the z-index and things start appearing underneath random components on the page.

@BGBRWR
Copy link
Contributor Author

BGBRWR commented Jun 5, 2024

Tried that, doesn't work. Your code seems to ignore the flag

You should create an issue with a stackblitz reproducing the bug because I'm not able to reproduce it myself.

@BGBRWR
Copy link
Contributor Author

BGBRWR commented Jun 5, 2024

Another issue introduced by this PR. On a dynamic dialog with a custom header and a footer template, I get a kind of infinite focus with the following error:

core.mjs:6531 ERROR RangeError: Maximum call stack size exceeded at DomHandler.getFocusableElement (primeng-dom.mjs:553:31) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:592:36) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)

Another issue introduced by this PR. On a dynamic dialog with a custom header and a footer template, I get a kind of infinite focus with the following error:

core.mjs:6531 ERROR RangeError: Maximum call stack size exceeded at DomHandler.getFocusableElement (primeng-dom.mjs:553:31) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:592:36) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18) at DynamicDialogComponent.focus (primeng-dynamicdialog.mjs:607:18)

You should create an issue with a stackblitz reproducing the bug because I'm not able to reproduce it myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team
Projects
None yet
6 participants