[menu-panel] Menu key handler for "Escape" does not work when inside modal dialog

[justvanrossum]

I have a modal dialog that contains a popup menu ("Add source" dialog in https://github.com/googlefonts/fontra/pull/2102, eg. the plust button below the glyph sources list). When using the popup menu, typing escape dismisses the dialog instead of the just the menu.

Possible cause:

• menu-panel uses a key event handler attached to the window, and dispatches a custom event
• the dialog has its own key even handler, which takes precedence over the menu-panel handler

If this is correct, the custom event dispatching mechanism of menu-panel is flawed: it cannot cooperate correctly in nested contexts such as these.

@giovanniTramonto, do you remember why you used a custom event in the first place?

https://github.com/googlefonts/fontra/blob/dd37f6752eb9da5854494b835b9d332d0e155813/src-js/fontra-webcomponents/src/menu-panel.js#L468-L472

[giovanniTramonto]

Am I clicking the right "plus" here? (main branch)

[justvanrossum]

Yes, but then in the branch for #2102, which has a popup in that dialog.

[giovanniTramonto]

I remember problems when navigating between the panels when listener were attached to their root. But this pattern is not great anyways. Let me have a look how this can be improved.

@giovanniTramonto, do you remember why you used a custom event in the first place?

fontra/src-js/fontra-webcomponents/src/menu-panel.js

Lines 468 to 472 in dd37f67

window.addEventListener("keydown", (event) => {
for (const element of MenuPanel.openMenuPanels) {
dispatchCustomEvent(element, "menu-panel:keydown", event);
}
});

[justvanrossum]

(The #2102 branch should get merged in the next few days, but the fix for this menu issue should be independent of that.)

[giovanniTramonto]

Is it still in time if I have a look into it this evening?

[justvanrossum]

There's no rush at all. It may be easier to fix this once 2102 is merged, since that's the place where it can be tested. But if you can test it with 2102, and yet create an independent PR, that's good, too. (I just don't want to burden 2102 with more stuff as it's a pretty complex one already.)

[giovanniTramonto]

OK Great

[giovanniTramonto]

The actual problem was the propagation. In some cases menu-bar needs to receive keydown events at the same time as menu-panel while outside of the menu-bar context stopping propagation is mandatory. Actually, it would make things much easier if keydown is handled only in menu-panel (and triggers navigateMenuBar i.e. through custom events). Maybe for later revision. For now this should work.