Fix #5019: Nested dialog escape listeners prioritization

[salihozdemir]

Fixed an issue when a dialog is nested, the escape listener for the child dialog should be prioritized over the escape listener for the parent dialog.

fix #5019

[vercel]

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

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
primereact	⬜️ Ignored (Inspect)	Visit Preview		Oct 20, 2023 1:18pm

[github-actions]

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

[melloware]

@avasuro can you review this? I am not sure this is correct either?

[avasuro]

I didn't test the "live" example, but based on the code, the current solution may not work as expected for two reasons:

• It still relies on the order in which dialogs are rendered. According to the code, it may not work for a situation like this:

  <Dialog visible={true} closable closeOnEscape>visible</Dialog>
  <Dialog visible={false} closable closeOnEscape>hidden</Dialog>
  

  In this case, the hidden dialog will always handle the escape key, so the visible dialog will never be closed (because of stopImmediatePropagation).
• Similarly, if a dialog is rendered alongside other elements that also listen for the "esc" key, there's no guarantee that another component won't handle the escape key first, and the modal may never be closed again (for the same reason - stopImmediatePropagation)

[salihozdemir]

I didn't test the "live" example, but based on the code, the current solution may not work as expected for two reasons:

• It still relies on the order in which dialogs are rendered. According to the code, it may not work for a situation like this:

  <Dialog visible={true} closable closeOnEscape>visible</Dialog>
  <Dialog visible={false} closable closeOnEscape>hidden</Dialog>
  

  In this case, the hidden dialog will always handle the escape key, so the visible dialog will never be closed (because of stopImmediatePropagation).

• Similarly, if a dialog is rendered alongside other elements that also listen for the "esc" key, there's no guarantee that another component won't handle the escape key first, and the modal may never be closed again (for the same reason - stopImmediatePropagation)

Hey @avasuro,

Thanks for the review! You're right, the current solution only works limited scenarios. I just saw your PR(#5021) and it looks like you've got a better solution. I'll watch your PR and see if I can help out.

I'm closing this.