Material: dialog(multiple): when 3+ dialogs open, ESC key closes more than one dialog at a time

[jakobadam]

Bug, enhancement request, or proposal:

Bug

CodePen and steps to reproduce the issue:

CodePen Demo which demonstrates the issue:

https://codepen.io/jakobadam/pen/zaxVBO?editors=1010

Detailed Reproduction Steps:

1. Open two dialogs with escapeToClose: true
2. Give the body focus
3. Hit ESC

What is the expected behavior?

The front most dialog should close

What is the current behavior?

All dialogs closes

What is the use-case or motivation for changing an existing behavior?

N/A

Which versions of AngularJS, Material, OS, and browsers are affected?

• AngularJS: 1.1.9
• AngularJS Material: 1.6.7
• OS: All
• Browsers: All

Is there anything else we should know? Stack Traces, Screenshots, etc.

Consider replacing ev.stopPropagation with ev.stopImmediatePropagation so only one event listener is triggered:
https://github.com/angular/material/blob/master/src/components/dialog/dialog.js#L967

[Splaktar]

I cannot reproduce this with the CodePen provided. You mentioned All browsers, but I tried with Chrome Stable (Version 67.0.3396.62 (Official Build) (64-bit)). You also mentioned All OS, I tried with macOS High Sierra.

I did the following:

1. Open the first dialog
2. Open the second dialog from the button on the first dialog
3. Click on the backdrop
4. Press the ESC key
   The front/second dialog dismissed.
5. Press the ESC key
   The back/first dialog dismissed.

Did I miss something or is this specific to a certain browser?

[rhandersen]

Splaktar: I also can't reproduce the issue with only two dialogs, but if > 2 dialogs are opened, hitting escape will close all but one dialog. This is not the expected behavior. Expected behavior is that it closes the top dialog only.

[jakobadam]

Two variants:
1a. First ESC exits the input.
1b. First ESC triggers close of the dialog.

1. ESC closes all remaining dialogs.

Note: After 1a and 1b focus is on body.

I can reproduce variant 1a on:

• Chromium Version 66.0.3359.181 (Official Build) Built on Ubuntu , running on Ubuntu 16.04 (64-bit)
• Some recent version of Chrome. Can't remember exact version (I was at a work machine, when I submitted the bug).

And variant 1b on:

• Google Chrome, Version 66.0.3359.170 (Official Build) (64-bit)
• Firefox 60.0.1

[Splaktar]

OK, yeah if I open 3+ dialogs, I can see that sometimes the ESC closes multiple dialogs instead of just one per keypress.

I can't reproduce ESC removing focus on an input. Perhaps that's an Ubuntu-specific behavior.

[TemitaTom]

The z-index of the md-backdrop when open 1+ dialogs not is correct

[Splaktar]

@TemitaTom please open a separate issue for that unless you can verify that is what is causing this issue.

As for investigating this issue, the code that handles hiding dialogs (and other interimElements) can be found here:
https://github.com/angular/material/blob/v1.1.10/src/core/services/interimElement/interimElement.js#L338-L377

It does this to remove the last dialog added:

        // Hide the latest showing interim element.
        return closeElement(showingInterims[showingInterims.length - 1]);

[hayzey]

Can confirm this is an issue in Chrome 69.0.3497.92 on macOS, using Angular Material 1.1.10 and Angular 1.7.4

[codymikol]

I did some manual tests and @jakobadam is 100% correct, we should be using stopImmediatePropagation. I have done manual testing and made a PR. 🐔

[codymikol]

@Splaktar if you can add him to my PR somehow so he gets credit that would be great, not sure if that's possible though.

[Splaktar]

@codymikol just mentioning him and linking to his comment with the suggested fix in your PR is the way to do that.

[codymikol]

@Splaktar I added a test. It's the best I could to to reproduce the issue in a test, but it doesn't fail in master :/.

[jakobadam]

Thanks for getting this in:) Our users can now close dialogs with ESC again...

[codymikol]

@jakobadam glad this helped you out :D