Flow: Button click shortcut listener does not recognize value change when using control key

[stefanuebe]

A simple use case: a form with some text fields, a rich text editor and a save button. The user changes stuff and wants to submit the form by using the ctrl + enter key.

The text fields are used as they are (value change mode ON_CHANGE), bound with a simple binder. To achieve the "on enter", we set a click shortcut with ENTER and CONTROL.

Now, when filling all fields and using the mouse to click the button, everything works fine.
When filling the fields and using the shortcut, the currently focused field does not get submitted.
Using a self defined shortcut listener and the button method clickInClient does not solve the problem

The problem seems to be the usage of the control key. When setting the short cut listener to ENTER only, it works finely (except for that the rich text editor focus needs to handle additional checkings)

Setting the value change mode to EAGER makes it work as a workaround.

[stefanuebe]

Tested with Vaadin 14, RC1 in bower mode on a Windows server and Windows client with chrome.

package org.vaadin.issue;

import com.vaadin.flow.component.AttachEvent;
import com.vaadin.flow.component.Key;
import com.vaadin.flow.component.button.Button;
import com.vaadin.flow.component.dialog.Dialog;
import com.vaadin.flow.component.html.Span;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.component.textfield.TextArea;
import com.vaadin.flow.component.textfield.TextField;
import com.vaadin.flow.data.binder.Binder;
import com.vaadin.flow.router.Route;

@Route(value = "button")
public class ButtonChangeView extends VerticalLayout {

    private SimpleBean simpleBean = new SimpleBean();

    @Override
    protected void onAttach(AttachEvent attachEvent) {

        VerticalLayout info = new VerticalLayout(new Span("Results"));
        Button open = new Button("Open", event -> {
            Dialog dialog = new Dialog();

            TextField aField = new TextField("A");
            TextField bField = new TextField("B");
            TextArea cField = new TextArea("C");

            Binder<SimpleBean> binder = new Binder<>();
            binder.bind(aField, SimpleBean::getA, SimpleBean::setA);
            binder.bind(bField, SimpleBean::getB, SimpleBean::setB);
            binder.bind(cField, SimpleBean::getC, SimpleBean::setC);

            binder.readBean(simpleBean);

            Button save = new Button("Save", event1 -> {
                if (binder.hasChanges() && binder.writeBeanIfValid(simpleBean)) {
                    info.add(new Span(simpleBean.toString()));
                    dialog.close();
                }
            });

            save.addClickShortcut(Key.ENTER);

            dialog.add(aField, bField, cField, save);
            dialog.open();
        });

        add(open, info);
    }

    private static class SimpleBean {
        private String a;
        private String b;
        private String c;

        public String getA() {
            return a;
        }

        public void setA(String a) {
            this.a = a;
        }

        public String getB() {
            return b;
        }

        public void setB(String b) {
            this.b = b;
        }

        public String getC() {
            return c;
        }

        public void setC(String c) {
            this.c = c;
        }

        @Override
        public String toString() {
            return "SimpleBean{" +
                    "a='" + a + '\'' +
                    ", b='" + b + '\'' +
                    ", c='" + c + '\'' +
                    '}';
        }
    }
}

[denis-anisimov]

I don't see this is a bug at all.

What I've got from your description is:
value change mode ON_CHANGE.

Is it correct ?

Then it may not work as you have described since the currently focused field doesn't notify the
server about changes _until the value is changed_.
And the value is not changed: it's changed/updated e.g. when you press ENTER.
Also I think changing the focus will change the value.

Ctrl + Enter doesn't change the value. So all other values are changed except the value for currently
focused field.
That's why it's not submitted.

You may not use value change mode ON_CHANGE if you want this kind of functionality.

I'm closing this issue as invalid.
Please reopen if you think this is incorrect/disagree.

[stefanuebe]

I disagree with that.

From a Java developer point of view the shortcut listener calls click on the button, thus I expect the components to behave the same way, as if the user would manually move his mouse to the button and click it (especially when I use the method "clickInClient").

Not doing it this way, but telling the developer "for that you have to use EAGER on all your fields" is a bad DX in my view: Either you cannot use shortcuts or you have a flood of network requests for each character that is typed in - especially when the form is not only 3-4 simple textfields but consisting of text areas or other editor components, which will send their whole content to the server all the time.

I could live with a scenario, where the normal button click + on change takes the current state from the server. But when having the special clickInClient I would expect at least, that this method behaves like user interaction (calling focus on the button and then click).

[denis-anisimov]

I don't get your points.

Do you understand what value change mode ON_CHANGE means ?

Please explain me your understanding.

In your scenario there is no value change. So the behavior is expected.
There is no need to use EAGER.
Anything meaningful except ON_CHANGE .

Will you expect the required behavior If you set TIMEOUT with a long timeout ?
I guess no, so why do you expect it using ON_CHANGE if there is no any change.

Well, I don't want to argue about this. I'm not going to fix this anyway.
I'm just trying to understand whether this is a bug at all or not.

I think this is not a bug.
The behavior is a bit confusing. So let it be an enhancement.

[stefanuebe]

Some simple use cases to show my issue:

We have a form with a server side binder, read some values from the bean. The user changes now all fields (by using the keyboard and typing new characters in each field).

Variants of the use case are:

1. the button has been assigned a shortcut listener listening on Enter
2. the button has been assigned a shortcut listener listening on Ctrl+Enter, since there is some textarea, which is not really usable with Variant 1

Variant 1, usecase 1:
The user changes the values and clicks actively on the mouse. The last changed field loses its focus and sends the value to the server. The server has all updated values and writes the bean correctly. Success.

Variant 1, usecase 2:
The user changes the values and uses the Enter key. The last changed field does not lose its focus but the browser seem to know that using enter should somehow do the same trick and sends the value to the server. The server has all updated values and writes the bean correctly. Success.

Variant 2, usecase 1:
same as Variant 1, uc1,

Variant 2, usecase 2:
The user changes the values and uses Ctrl+Enter key. The last changed field does not lose its focus. The browser does not know what that means, thus it does not send the value to the server. The server has one value not updated writes the bean incorrectly. Fail

Outcome
Now, from a frontend only perspective, I agree with you, that there has been no value change, since the field did not lose its focus nor did a simple enter event happen. But, from the perspective of the user and the Java developer, who do not have an idea of such browser related technical internal stuff, this simply is an issue inside the framework.

I also agree, that the normal "click" method cannot check the client side, if there might be some unsent, but from the user's perspective changed value, that needs to be sent, but, the clickInClient method should (otherwise it makes absolutely no sense to have this method).

So, to sum up everything what the request for fix (or enhance) is:
Using Button.clickInClient should set the focus on the button first before clicking it, to simulate the real click, which includes a auto focus of the button, since the user clicks on it. This should - in my plain understanding - also fire the change event of the text field first. Everyone is happy, since the form works as it should.

[Springrbua]

I agree with @stefanuebe, adding a click shortcut, I expect it to work the same as if I manually click the Button.
However, it would also be nice to have a solution for other ShortcutListeners as well.
In my case, I have some "commands", which are hidden inside an overflow-menu, thus they are not visible on the UI. A click shortcut would not be triggered in this case, and I cannot call clickInClient manually. However, I want to make sure, that the value of the currently focused element is synchronized with the server, before my listener gets fired.
Using an other ValueChangeMode is, in my opinion, not a suitable solution, ON_CHANGE is exactly what I want, but I also want it to sync the value when some special shortcuts are triggered.
Maybe the ShortcutRegistration could offer an API for this case? Something like syncFocusedFieldValueWithServer?

[SebastianKuehnau]

I can only agree with the messages from @Springrbua and @stefanuebe. The usability of the web page would be much better if an input value from the field could be transferred via shortcut even if the field still has the focus.

As a workaround you can deselect the focus using client side code:

getElement().executeJs("document.activeElement.blur()").then(jsonValue -> {
     Notification.show(textField.getValue());
});

but since the Vaadin classes has Shortcuts and ShortcutRegistration are completely encapsulated, you can't hook any code here either and that reduces the DX experience here.