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);
}
}
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:
Dialog is just a component for Flow the same like Div .Div on the page then it's expected (see above) that both of them will be triggered .Div and Dialog .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:
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.
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:
Notification, Dialog) to determine that the shortcut should be listened on there instead of UI when necessary. 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.
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.
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.
@rolfsmeds here could be a11y added as well.
@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:
@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).
@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"
Right, that sentence doesn't make sense as router links can be inside the modal dialogs too and should thus work. I'll slap the author with a large 馃悷 and fix it
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.
@pleku I don't mind the 14.5 and 19.x release! Thanks for your work!