vaadin / web-components

A set of high-quality standards based web components for enterprise web applications. Part of Vaadin 20+
https://vaadin.com/docs/latest/components
445 stars 83 forks source link

MultiSelectComboBox takes focus after value submit #7103

Open probert94 opened 7 months ago

probert94 commented 7 months ago

Description

I have a MultiSelectCombobox to enter email addresses. As not all email addresses are known to the system, it also accepts custom values but they have to be validated. If the user enters an invalid email address a error dialog is openend and the confirm button of it should be focused. Unfortunately this deos not seem to work with MultiSelectComboBox as it takes back the focus.

Expected outcome

The confirm button should keep the focus

Minimal reproducible example

public class MainView extends HorizontalLayout {

    public MainView() {
        setPadding(true);
        setJustifyContentMode(JustifyContentMode.CENTER);
        setAlignItems(Alignment.CENTER);
        setSizeFull();
        MultiSelectComboBox<String> mailTo = new MultiSelectComboBox<>("Mail to");
        mailTo.setItems(query -> mailTo.getValue().stream().skip(query.getOffset()).limit(query.getLimit()));
        mailTo.setSelectedItemsOnTop(true);
        mailTo.addCustomValueSetListener(e -> {
           Dialog dlg = new Dialog("Invalid mail adress" );
           Button ok = new Button("OK", ignore -> dlg.close());
           dlg.getFooter().add(ok);
           dlg.addAttachListener(ae -> ok.focus());
           dlg.open();
        });
        mailTo.setRenderer(new ComponentRenderer<>(i -> new Span(i)));

        TextField textField = new TextField("Test");
        textField.addValueChangeListener(e -> {
            Dialog dlg = new Dialog("Invalid mail adress" );
            Button ok = new Button("OK", ignore -> dlg.close());
            dlg.getFooter().add(ok);
            dlg.addAttachListener(ae -> ok.focus());
            dlg.open();
        });

        add(new VerticalLayout(mailTo, textField));
    }
}

Steps to reproduce

  1. Download a new starter and paste the above code into the MainView
  2. Enter some value into the MultiSelectComboBox with label "Mail to". When you confirm the value with enter a dialog opens.
  3. The "OK"-button should be focused now but the MultiSelectComboBox is still focused
  4. Try the same with the TextField with label "Test". Again, when you confirm the value with enter, a dialog opens
  5. Now the "OK" button has the focus

https://github.com/vaadin/web-components/assets/18258317/9a046478-5c2a-4b8e-802f-1382e0065c90

Environment

Flow: 24.3.3 Vaadin: 24.3.3 Java: Eclipse Adoptium 17.0.6 OS: amd64 Windows 11 10.0 Browser: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36

Browsers

Issue is not browser related

TatuLund commented 7 months ago

dlg.addAttachListener(ae -> ok.focus());

Instead of attach listener you should probably try dlg.addOpenedChangedListener(openEvent -> ... )

probert94 commented 7 months ago

The outcome is still the same, it works with the TextField but does not work with he MultiSelectComboBox

TatuLund commented 7 months ago

The working version is this:

@Route(value = "multi-dialog", layout = MainLayout.class)
public class MultiComboDialogView extends HorizontalLayout {

    public MultiComboDialogView() {
        setPadding(true);
        setJustifyContentMode(JustifyContentMode.CENTER);
        setAlignItems(Alignment.CENTER);
        setSizeFull();
        MultiSelectComboBox<String> mailTo = new MultiSelectComboBox<>(
                "Mail to");
        mailTo.setItems("hello@vaadin.com", "world@vaadin.com");
        mailTo.setSelectedItemsOnTop(true);
        mailTo.addCustomValueSetListener(e -> {
            Dialog dlg = new Dialog("Invalid mail adress");
            Button ok = new Button("OK", ignore -> dlg.close());
            dlg.getFooter().add(ok);
            dlg.addOpenedChangeListener(ae -> {
                if (ae.isOpened()) {
                    ok.getElement().executeJs("setTimeout(() => this.focus(), 100);");
                }
            });
            dlg.open();
        });
        mailTo.setRenderer(new ComponentRenderer<>(i -> new Span(i)));

        TextField textField = new TextField("Test");
        textField.addValueChangeListener(e -> {
            Dialog dlg = new Dialog("Invalid mail adress");
            Button ok = new Button("OK", ignore -> dlg.close());
            dlg.getFooter().add(ok);
            dlg.addOpenedChangeListener(ae -> {
                if (ae.isOpened()) {
                    ok.focus();
                }
            });
            dlg.open();
        });

        add(new VerticalLayout(mailTo, textField));
    }
}

I think this is not a bug in MultiSelectComboBox. When MultiSelectComboBox drop down is closing, it should return the focus from dropdown to the field. Otherwise we would have accessibility issues. Same to other direction, i.e. the focus should move from field to dropdown when it opens, otherwise keyboard navigation would break.

Now in your case Dialog disrupts this flow. The solution is to defer call of the focus after dialog opening is complete, thus I am using JavaScript call with timeout function.

Note, Dialog remembers the last focus and returns it when closes, this is intended feature for the same accessibility purposes.

This is possibly a bug in Dialog, i.e. opened change event is fired too early, thus timeout function needed here.

probert94 commented 7 months ago

Would it be possible that the MultiSelectCombobox checks if the overlay had the focus when closing? If not, it should probably not reurn the focus to the field.

yuriy-fix commented 7 months ago

As mentioned by @TatuLund, this is not a bug in MultiSelectCombobox: opening dialog to show an error message for the field could violate a11y requirements. Is there any reason why dialog need to be used instead of the field's error message?

probert94 commented 7 months ago

Sometimes it is not necessarily an error but an information regarding the selected value. For example when selecting the client in a new order an info with the open invoices might be shown.

I understand that the combobox get's the focus after closing the overlay but in my opinion this should only happen, if the overlay had the focus before.

knoobie commented 7 months ago

The binder has support for different error levels - see as example: https://github.com/vaadin/flow/pull/11384#issuecomment-879309037

This would allow to display e.g. information without fully invalidating the field on the server. (Make sure to use proper styling for different error levels and a prefix like "Info:")

probert94 commented 7 months ago

@knoobie however, the shown information is limited to only a few words, more complex information has to be shown somewhere else. I agree that often the error / hint on the field is the better choice but there are still cases in which a dialog makes sense in my opinion.