Dialog doesn't prevent the execution of clickshortcuts behind it

[knoobie]

When I create a view with a Shortcut Listener (Enter) on a button and the view opens a "Add XYZ Dialog" for some additional content - I would expect that I can create a second Shortcut Listener within the Dialog and the Dialog prevents that both Enter Shortcuts are executed.

Only the Shortcut Listener within the Dialog should be executed. Currently both Shortcuts are executed.

This creates weird side effects. Additionally any Dialog should be modal (WCAG) - that mean's that interacting with the content behind it SHOULD BE impossible.

Vaadin: 14.1.19
Browser: FF 74

How to test:

1. Click Open Dialog
2. Press Enter
3. 2 Notification are displayed. It should only be one visible

@Route("shortcut")
public class ShortcutBug extends VerticalLayout {

  public ShortcutBug() {
    add(new Button("Open Dialog", e -> {
      Button myDialogButton = new Button("Hit Enter", event -> {
        Notification.show("Enter was called", 2000, Position.TOP_START).addThemeVariants(NotificationVariant.LUMO_SUCCESS);
      });
      myDialogButton.addClickShortcut(Key.ENTER);
      Dialog dialog = new Dialog(myDialogButton);
      dialog.open();
    }));

    Button myNonDialogButton = new Button("Should not be called by the other Other Enter Shortcut", e -> {
      Notification.show("ERROR: Enter was called", 5000, Position.TOP_START).addThemeVariants(NotificationVariant.LUMO_ERROR);
    });
    myNonDialogButton.addClickShortcut(Key.ENTER);

    add(new Hr(), myNonDialogButton);
  }
}

[denis-anisimov]

I don't think this is a bug.
This works as designed even though I agree that the behavior could be better.
Here is the explanation:

• Flow allows to add the same shortcut to several components : that how it works (this can be an issue itself though).
• So they are global.
• Dialog is just a component for Flow the same like Div .
• If I have two buttons : one is on the page, another is inside Div on the page then it's expected (see above) that both of them will be triggered .
• As I said: for Flow there is no any difference between Div and Dialog .
• So it works as expected.

There can be a discussion whether we should trigger actions for every component with a shortcut (then cases when some components has assigned shortcuts but not attached should be considered).
But this discussion doesn't help here anyway since in this case both buttons presents on the page (the only difference that the one is in the dialog).

So : this is not a bug but works by design.

On the other hand the issue makes sense: this is intuitively expected that there is an "input context" which is only active at the moment.
The "input context" is the abstraction layer which is just absent in Flow completely .
So we need to add a new abstraction unit into Flow which represents an "input context" , then Dialog should use this abstraction (e.g. implementing an interface) and only components which are active within the "input context" will be activated.
That requires:

• design
• discussion
• implementation in Flow
• implementation in Dialog, Notification.

It's a very long process and I would mark this issue as an enhancement.
The workaround here is quite simple though: just deactivate shortcuts when a dialog is opened .

At the moment the issue is technically in the Dialog component but it can't be implemented there (I believe) without changing design and API in Flow: components won't be able to change the Flow shortcuts logic.

[pleku]

The "input context" is the abstraction layer which is just absent in Flow completely .

I agree with this, it is what we would need, but yes it would be an enhancement. The "fix" for this would be to document the current behavior in javadocs & documentation.

This creates weird side effects. Additionally any Dialog should be modal (WCAG) - that mean's that interacting with the content behind it SHOULD BE impossible.

The WCAG 2.x doesn't have anything about a "modal dialog" so I understand it the same way - there is no strict requirement for preventing the interaction with the content outside it. So it would be up to the application developer to ensure that the everything works as expected.

Also with after discussing about vaadin-dialog as a component, while I do agree that everyone would expect the same behavior regardless if they use Flow or not, when working with client driven views one has to similarly handle the situation between the keyboard shortcuts on the background and with the dialog.

But so now to handle this situation:

• I'll add notes to javadocs & documentatio
• Create an enhancement issue about adding a "input context" (name pending) abstraction layer to Flow which can be used in components (Notification, Dialog) to determine that the shortcut should be listened on there instead of UI when necessary.

[knoobie]

Thanks for your additional feedback!

The WCAG 2.x doesn't have anything about a "modal dialog"

WCAG 2.x has no direct reference to dialog, but WAI-ARIA has:

A dialog is a window overlaid on either the primary window or another dialog window. Windows under a modal dialog are inert. That is, users cannot interact with content outside an active dialog window. Inert content outside an active dialog is typically visually obscured or dimmed so it is difficult to discern, and in some implementations, attempts to interact with the inert content cause the dialog to close.

Source: https://www.w3.org/TR/wai-aria-practices-1.1/#dialog_modal

That is, users cannot interact with content outside an active dialog window.

Especially this sentence.

[pleku]

Source: https://www.w3.org/TR/wai-aria-practices-1.1/#dialog_modal

I saw that via Google too but I discarded it as I though it was outdated information, because (I have to admit) I've misread your first comment here - I first read as "interacting should be possible" even though you clearly have written "interacting should be impossible". My bad.

So based on that I agree - a true modal dialog should IMO out-of-the-box prevent background interaction such as keyboard shortcuts. I'll have to discuss again with the component developers / designers about this.

[daniel-pss-prosoft]

We also reported this bug, with warranty priority, and they gave the same answer: "won't fix".
Eventually we implemented an in-house fix... with about 30 minutes of effort.

Great premium support.

[knoobie]

@rolfsmeds here could be a11y added as well.

[knoobie]

@pleku I've looked at your branch and it looks really promising! One idea that I had while checking the code - a real life IT test:

Scenario: "User navigates away from a form without saving. Modal dialog appears to confirm that he really wants to leave."

Tests:

• clicking the inert router link behind the dialog won't work
• interacting with all components within the dialog works (does interacting with router links within the dialog work? I'm personally not sure what's best)
• clicking confirm within the dialog closes the dialog, removes inert and navigates away.

[pleku]

@knoobie thanks for the feedback. More complex tests are missing, but planned https://github.com/vaadin/flow/compare/fix/10172#diff-18b269c7881365739d04976845a325b99e67db49f573b5de3341ed1bf076eb79R6 and I'll add those too.

interacting with all components within the dialog works (does interacting with router links within the dialog work? I'm personally not sure what's best)

I've understood that anything inside the modal part should work - so if an application developer puts a router link there, it should work. For confirmation dialogs there probably are no router links used though, but there might be other cases (like access denied or access changed dialog giving alternative routes to go to).

[knoobie]

@pleku I personally would expect router link to work within the dialog but the following comment made me question myself :)

        // When the UI is made inert, all router link navigation is
        // ignored. For the part of the UI that is not inert, navigation
        // could be allowed by using something else than router links (with
        // server side listeners).

Because the second sentence said "not inert = navigation could be allowed by using something else than router links"

[pleku]

The PRs are open (flow & component integrations) that would make this land to 14.5+ and 19+.

The fix could be landed to 14.4, but with 14.5 around the corner (RC 17th, GA 24th of March) I won't do it unless explicitly asked to.

[knoobie]

@pleku I don't mind the 14.5 and 19.x release! Thanks for your work!