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

md-dialog: Escape key press does not close the dialog completely + navigation back on mobile not working #5313

Closed
kkachniarz220 opened this issue Dec 18, 2023 · 6 comments · Fixed by #5295 · May be fixed by X-oss-byte/pigweed#61
Assignees

Comments

@kkachniarz220
Copy link
Contributor

kkachniarz220 commented Dec 18, 2023

What is affected?

Component

Description

On escape key press the 'cancel' event is not emitting + backdrop layer is still visible
screen-capture

There is problem also with browser history back - backdrop layer is still visible (as on native browser )

Reproduction

simple md-dialog use

Workaround

.

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

1.1.1

Browser/OS/Node environment

Browser: Chrome 120.0.6099.110
Windows 11
Node version: 18.17.1
npm version: 9.6.7

@kkachniarz220 kkachniarz220 changed the title md-dialog: Escape key press does not close the dialog completely md-dialog: Escape key press does not close the dialog completely + navigation back on mobile not working Dec 18, 2023
@asyncLiz asyncLiz self-assigned this Dec 18, 2023
@asyncLiz
Copy link
Collaborator

Underlying issue is https://bugs.chromium.org/p/chromium/issues/detail?id=1512224

Think I can add a patch though

@josepharhar
Copy link

I think that dialog/internal/dialog.ts should be looking at both the close and cancel events. close will always fire, but in this case where the closing of the dialog is not preventable, the cancel event will not be fired.

It looks like the close event is not being listened to:

@cancel=${this.handleCancel}

@asyncLiz
Copy link
Collaborator

The Chromium bug still breaks the ability to detect if a dialog was canceled by pressing the escape key vs calling dialog.close().

Was there a breaking spec change where pressing the escape key closes a dialog without cancelation?

@josepharhar
Copy link

Yeah we changed the spec here: whatwg/html#9462

copybara-service bot pushed a commit that referenced this issue Dec 20, 2023
@asyncLiz
Copy link
Collaborator

Got a patch in #5329 that'll fix it for now, but gonna leave this open to track addressing non-cancelable CloseWatcher anti-abuse gestures, such as the Android back button or pressing escape twice.

@asyncLiz
Copy link
Collaborator

Actually going to track in #5330 to avoid scope creep with this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment