feat: 🎸 add option to only enable close for last opened dialog

milo526 commented 1 year ago

Closes #96

PR Checklist

Please check if your PR fulfills the following requirements:

[x] The commit message follows our guidelines: CONTRIBUTING.md#commit
[x] Tests for the changes have been added (for bug fixes / features)
[x] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

In situations where there are multiple opened modals that are stacked over each other; closing with Escape results in all of the modals being closed. Clicking the backdrop only dismisses the top modal.

Issue Number: #96

What is the new behavior?

With the enableClose config value set to onlyForLastOpenedDialog, pressing escape will only dismiss the last opened dialog instead of all open dialogs.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

stackblitz[bot] commented 1 year ago

Run & review this pull request in StackBlitz Codeflow.

milo526 commented 1 year ago

The changes as suggested work for our use case.

The injection of the dialog service into the dialog component feels a bit hacky.

I was doubting if these tests are in the right place; I'd rather have some more integrated tests opening multiple dialogs through the service and seeing if the correct dialogs close. However, this did not seem to be the style of the tests you are targeting.

I would love some feedback on this PR.

milo526 commented 1 year ago

Thank you for the feedback. The PR has been updated accordingly.

ngneat / dialog