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

events / buttons are not properly registered on nested mdc-dialog #1506

Closed
iocast opened this issue Oct 30, 2017 · 5 comments
Closed

events / buttons are not properly registered on nested mdc-dialog #1506

iocast opened this issue Oct 30, 2017 · 5 comments

Comments

@iocast
Copy link

iocast commented Oct 30, 2017

Bug

What MDC-Web Version are you using?

Version: 0.23.0

What browser(s) is this bug affecting?

Tested under Chrome version 61.0.3163.100 and Safari version 11.0 (13604.1.38.1.6)

What are the steps to reproduce the bug?

Here is a codepen.io pen. Just do the following to reproduce the bug:

  1. open the first dialog (button: SHOW DIALOG)
  2. open the second dialog (button: SHOW)
  3. cancel or accept the second dialog.

What is the expected behavior?

Only closing the second dialog without closing the first (underlying) dialog.

What is the actual behavior?

It closes all dialogs.

Any other information you believe would be useful?

All dialog footer buttons are registered on the dialog as footerBtnRipples_. Thus the first dialog includes the footer buttons from the second dialog and listens to the events of the second dialog.

As you see in the provided example, the console log shows that the first dialog listens on / includes 4 buttons. The variable footerBtnRipples_ includes 4 buttons for the first dialog and 2 buttons for the second dialog.

Findings

I think the issue is at line 47 of mdc-dialog/index.js. The query selection does take all .mdc-dialog__footer__button without respecting the current hierarchy / nesting.

@lynnmercier
Copy link
Contributor

We don't support nesting a dialog within another dialog. Sorry.

@iocast
Copy link
Author

iocast commented Nov 2, 2017

I still have a question. In the spec it is written that the picker component uses a dialog. In addition in the usage section it says that

On mobile, pickers are best suited for display in a confirmation dialog.

What I understand is, that when I open a confirmation dialog and I want to add a picker, the picker opens another dialog inside the confirmation dialog. Ergo we have something like a nested dialog.
Or do I missed something?

@Garbee
Copy link
Contributor

Garbee commented Nov 6, 2017

Nested dialogs is one reason the dialog element should be used and polyfilled in where not available. It handles nesting by default in the browsers so these things "just work" as it were.

@iocast
Copy link
Author

iocast commented Nov 8, 2017

@Garbee thanks. when I setup a pure html example then it works. When I use the mdc-dialog example with dialog instead of aside it still not works. I still have the same behaviour.

Doesn't matter. I will do something else.

@Garbee
Copy link
Contributor

Garbee commented Nov 8, 2017

When I use the mdc-dialog example with dialog instead of aside it still not works

This is because the dialog component here isn't using any of the native browser dialog functions. So, it isn't going to handle nested/stacking dialogs. The component internally should use the native stuff when available and otherwise trigger a polyfill script of the native behavior IMO. But, this is not something the team wished to do in the initial development of the component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants