Tab key press not committing editing component updates to the row model

[oilhands]

Description

When the user makes edits in the edit components the values are changed and stored only in the edit controls when the user presses the tab key to depart the edit component. When the user uses the space key or the enter key the edit controls and the row model are synced up just like normal. When using the tab key the prior row model's data is restored in the row for display to the user. When the user comes back to the edit component the updated edit value the user entered is displayed again.

Expected outcome

I would expect the edits to the DatePicker and ComboBox components to be applied to the row model and vice versa as appropriate when the tab key is pressed rather than the space or enter key.

Minimal reproducible example

package com.ciminc;

import java.time.LocalDate;

public class RowObject {

String value1;
String value2;
String value3;
String value4;
LocalDate localDate1;
LocalDate localDate2;
String combo1;

public String getValue1() {
    return value1;
}

public void setValue1(String value1) {
    this.value1 = value1;
}

public String getValue2() {
    return value2;
}

public void setValue2(String value2) {
    this.value2 = value2;
}

public String getValue3() {
    return value3;
}

public void setValue3(String value3) {
    this.value3 = value3;
}

public String getValue4() {
    return value4;
}

public void setValue4(String value4) {
    this.value4 = value4;
}

public LocalDate getLocalDate1() {
    return localDate1;
}

public void setLocalDate1(LocalDate localDate1) {
    this.localDate1 = localDate1;
}

public LocalDate getLocalDate2() {
    return localDate2;
}

public void setLocalDate2(LocalDate localDate2) {
    this.localDate2 = localDate2;
}

public String getCombo1() {
    return combo1;
}

public void setCombo1(String combo1) {
    this.combo1 = combo1;
}

}

package com.ciminc;

import com.vaadin.flow.component.combobox.ComboBox;
import com.vaadin.flow.component.datepicker.DatePicker;
import com.vaadin.flow.component.gridpro.GridPro;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.component.textfield.TextField;
import com.vaadin.flow.data.binder.Binder;
import com.vaadin.flow.router.Route;
import java.util.ArrayList;
import java.util.List;

@route
public class MainView extends VerticalLayout {

public MainView() {

    Binder<RowObject> binder = new Binder<>(RowObject.class);

    GridPro<RowObject> grid = new GridPro<>();
    grid.setEditOnClick(true);
    grid.setSingleCellEdit(true);

    TextField value1Field = new TextField();
    grid.addEditColumn(RowObject::getValue1)
            .custom(value1Field, (row, newValue) -> row.setValue1(newValue))
            .setHeader("Value 1");

    TextField value2Field = new TextField();
    grid.addEditColumn(RowObject::getValue2)
            .custom(value2Field, (row, newValue) -> row.setValue2(newValue))
            .setHeader("Value 2");

    ComboBox<String> comboBox1 = new ComboBox<>();
    List<String> comboObjects = new ArrayList<>();
    comboObjects.add("combo1");
    comboObjects.add("combo2");
    comboObjects.add("combo3");
    comboObjects.add("combo4");
    comboBox1.setItems(comboObjects);

    grid.addEditColumn(RowObject::getCombo1)
            .custom(comboBox1, (row, newValue) -> row.setCombo1(newValue))
            .setHeader("Combo 1");

    DatePicker localDatePicker1 = new DatePicker();
    localDatePicker1.setAutoOpen(false);

    grid.addEditColumn(RowObject::getLocalDate1)
            .custom(localDatePicker1, (row, newValue) -> row.setLocalDate1(newValue))
            .setHeader("Local Date 1");

    DatePicker localDatePicker2 = new DatePicker();
    localDatePicker2.setAutoOpen(false);
    grid.addEditColumn(RowObject::getLocalDate2)
            .custom(localDatePicker2, (row, newValue) -> row.setLocalDate2(newValue))
            .setHeader("Local Date 2");

    TextField value3Field = new TextField();
    grid.addEditColumn(RowObject::getValue3)
            .custom(value3Field, (row, newValue) -> row.setValue3(newValue))
            .setHeader("Value 3");

    TextField value4Field = new TextField();
    grid.addEditColumn(RowObject::getValue4)
            .custom(value4Field, (row, newValue) -> row.setValue4(newValue))
            .setHeader("Value 4");

    List<RowObject> rowObjects = new ArrayList<>();

    RowObject rowObject = new RowObject();
    rowObject.setValue1("value1 - a");
    rowObject.setValue2("value2 - a");
    rowObject.setValue3("value3 - a");
    rowObject.setValue4("value4 - a");
    rowObjects.add(rowObject);

    rowObject = new RowObject();
    rowObject.setValue1("value1 - b");
    rowObject.setValue2("value2 - b");
    rowObject.setValue3("value3 - b");
    rowObject.setValue4("value4 - b");
    rowObjects.add(rowObject);

    rowObject = new RowObject();
    rowObject.setValue1("value1 - c");
    rowObject.setValue2("value2 - c");
    rowObject.setValue3("value3 - c");
    rowObject.setValue4("value4 - c");
    rowObjects.add(rowObject);

    grid.setItems(rowObjects);
    add(grid);

    binder.forField(value1Field).bind(RowObject::getValue1, RowObject::setValue1);
    binder.forField(value2Field).bind(RowObject::getValue2, RowObject::setValue2);
    binder.forField(value3Field).bind(RowObject::getValue3, RowObject::setValue3);
    binder.forField(value4Field).bind(RowObject::getValue4, RowObject::setValue4);

    binder.forField(localDatePicker1).bind(RowObject::getLocalDate1, RowObject::setLocalDate1);
    binder.forField(localDatePicker2).bind(RowObject::getLocalDate2, RowObject::setLocalDate2);

    RowObject boundRowObject = new RowObject();
    binder.setBean(boundRowObject);

}

}

Steps to reproduce

Setup a grid with edit controls per normal with TextField and ComboBox and DatePicker components.
Add/edit a row.
Use tab to depart fields and see space/enter committed data in the row but not tab committed data

Environment

Vaadin version(s): 23.3.30
OS: Ubuntu 22.04.3 LTS
Component: GridPro

Browsers

Chrome, Firefox

[TatuLund]

Your implementation slightly differs from the example of using Binder with GridPro here

https://cookbook.vaadin.com/grid-pro-binder

Notably, you should call setBean every time editor is being opened and also reset the Binder when edit is ended. See the listeners. Furthermore as you use Binder, you do not need to call setter in the update call back as Binder already commits the value to the bean ".custom(value4Field, (row, newValue) -> row.setValue4(newValue))"

[oilhands]

I've updated the example and added the missing combo box column that was one of the issues. If you go to he combo and open it and arrow down to a selection and then tab to the next column the combo value is not updated in the grid but if you arrow or click back into the editor the value is displayed and then when you depart the field it updates and shows in the grid.

@Route
public class MainView extends VerticalLayout {

    public MainView() {

        Binder<RowObject> binder = new Binder<>(RowObject.class);

        GridPro<RowObject> grid = new GridPro<>();
        grid.setEditOnClick(true);
        grid.setSingleCellEdit(true);

        TextField value1Field = new TextField();
        grid.addEditColumn(RowObject::getValue1)
                .custom(value1Field, (row, newValue) -> {
                })
                .setHeader("Value 1");

        TextField value2Field = new TextField();
        grid.addEditColumn(RowObject::getValue2)
                .custom(value2Field, (row, newValue) -> {
                })
                .setHeader("Value 2");

        ComboBox<String> comboBox1 = new ComboBox<>();
        List<String> comboObjects = new ArrayList<>();
        comboObjects.add("combo1");
        comboObjects.add("combo2");
        comboObjects.add("combo3");
        comboObjects.add("combo4");
        comboBox1.setItems(comboObjects);

        grid.addEditColumn(RowObject::getCombo1)
                .custom(comboBox1, (row, newValue) -> {
                })
                .setHeader("Combo 1");

        DatePicker localDatePicker1 = new DatePicker();
        localDatePicker1.setAutoOpen(false);

        grid.addEditColumn(RowObject::getLocalDate1)
                .custom(localDatePicker1, (row, newValue) -> {
                })
                .setHeader("Local Date 1");

        DatePicker localDatePicker2 = new DatePicker();
        localDatePicker2.setAutoOpen(false);
        grid.addEditColumn(RowObject::getLocalDate2)
                .custom(localDatePicker2, (row, newValue) -> {
                })
                .setHeader("Local Date 2");

        TextField value3Field = new TextField();
        grid.addEditColumn(RowObject::getValue3)
                .custom(value3Field, (row, newValue) -> {
                })
                .setHeader("Value 3");

        TextField value4Field = new TextField();
        grid.addEditColumn(RowObject::getValue4)
                .custom(value4Field, (row, newValue) -> {
                })
                .setHeader("Value 4");

        List<RowObject> rowObjects = new ArrayList<>();

        RowObject rowObject = new RowObject();
        rowObject.setValue1("value1 - a");
        rowObject.setValue2("value2 - a");
        rowObject.setValue3("value3 - a");
        rowObject.setValue4("value4 - a");
        rowObjects.add(rowObject);

        rowObject = new RowObject();
        rowObject.setValue1("value1 - b");
        rowObject.setValue2("value2 - b");
        rowObject.setValue3("value3 - b");
        rowObject.setValue4("value4 - b");
        rowObjects.add(rowObject);

        rowObject = new RowObject();
        rowObject.setValue1("value1 - c");
        rowObject.setValue2("value2 - c");
        rowObject.setValue3("value3 - c");
        rowObject.setValue4("value4 - c");
        rowObjects.add(rowObject);

        grid.setItems(rowObjects);

        Button addButton = new Button("Add");
        addButton.addClickListener((event) -> {
            rowObjects.add(0, new RowObject());
            grid.getDataProvider().refreshAll();
            grid.select(rowObjects.get(0));
        });

        grid.addCellEditStartedListener(event -> {
            binder.setBean(event.getItem());
        });

        grid.addItemPropertyChangedListener(event -> {
            binder.setBean(null);
        });

        add(grid, addButton);

        binder.forField(value1Field).bind(RowObject::getValue1, RowObject::setValue1);
        binder.forField(value2Field).bind(RowObject::getValue2, RowObject::setValue2);
        binder.forField(value3Field).bind(RowObject::getValue3, RowObject::setValue3);
        binder.forField(value4Field).bind(RowObject::getValue4, RowObject::setValue4);

        binder.forField(localDatePicker1).bind(RowObject::getLocalDate1, RowObject::setLocalDate1);
        binder.forField(localDatePicker2).bind(RowObject::getLocalDate2, RowObject::setLocalDate2);

        binder.forField(comboBox1).bind(RowObject::getCombo1, RowObject::setCombo1);

    }

}

[TatuLund]

This is a very odd edge case, there seems to be timing glitch. Noting here, that this Binder setup is not officially supported, it is just a Recipe I wrote as I found it mostly works, naturally I did not test ComboBox. But it is possible to get the ComboBox working as well. You need to do two modifications to your code.

Add refreshing of the Grid item after ComboBox value has changed. Normally GridPro does refreshing of the item by itself, but seems to happen too early.

        comboBox1.addValueChangeListener(
                e -> grid.getDataProvider().refreshItem(binder.getBean()));

The other change is the property change listener, which removes the bound bean. You need to either remove it fully or change the setBean(null) call to happen after client roundtrip. Purpose of this call is to prevent showing the old field values when the editor opened next time, which may happen if the network latency is long enough.

        grid.addItemPropertyChangedListener(event -> {
            getElement().executeJs("return 0").then(r -> {
                binder.setBean(null);
            });
        });

[oilhands]

When I apply these changes to my production code the combo box add change listener refresh fails because the binder.getBean() is returning a null value. You've referenced a couple times that the provided cookbook solution and the way we are using the binder is non-standard or not a supported usage. What do you mean by that?

[TatuLund]

When I apply these changes to my production code the combo box add change listener refresh fails because the binder.getBean() is returning a null value.

Then there is another type of timing glitch in your application. I.e. value change is triggered later, and the binder.setBean(null) has been already executed. Then I would remove that part as apparently not needed in your case.

the way we are using the binder is non-standard or not a supported usage. What do you mean by that?

GridPro has not been designed this in mind, as you have observed there are various potential edge cases and pitfalls as timings can go this way or other way. So in order to make it work, it requires some tinkering. I think there should be a ticket about adding Binder support in GridPro, but I can't find it now.

[oilhands]

Nope. This is not resolved. It is still glitching and acting flaky. I was hoping that full Binder support was something that you'd be looking into.

[yuriy-fix]

This probably should be considered as an enhancement request to officially support Binder usage with GridPro.

[web-padawan]

This is a very odd edge case, there seems to be timing glitch.

The issue with Tab key press be fixed by #7822 for both ComboBox and DatePicker.
Let's discuss the Binder topic in a separate issue in https://github.com/vaadin/flow-components

[oilhands]

@TatuLund This fix pulled into the 23.5.8 release seems to be working well for us in our initial testing. 😃 Thanks!