vaadin / flow-components

Java counterpart of Vaadin Web Components
100 stars 66 forks source link

Grid/GridPro: cannot select row if grid.select called from cell focus listener #2373

Closed drewharvey closed 2 years ago

drewharvey commented 2 years ago

Description

Chrome and FF Vaadin 22 beta 3

Happens for both Grid and GridPro

See the demo and code below. The row selects and quickly deselects. It is slightly different in FF and Chrome. If I move away to a different window on my computer and then go back to the browser window, the selection shows correctly. This only happens when I call grid.select(...) inside of a cell focus listener.

Firefox: Happens on every click.

Chrome: Happens when you click near the outer boundaries of the cell (this problem does not occur if you click on the cell's content).

Live Demo (optional)

Firefox: grid-deselect-ff

Chrome: grid-deselect-issue

Minimal reproducible example

// setup grid (or GridPro)
Grid<String> grid = new Grid<>();
grid.setWidth("500px");
grid.setHeight("200px");

grid.addCellFocusListener(e -> {
    if (e.getItem().isPresent())
        grid.select(e.getItem().get());
});

grid.addColumn(item -> item)
        .setHeader("Sdffs");

grid.addColumn(item -> item)
        .setHeader("DOB");

grid.addColumn(item -> item)
        .setHeader("DOB");

grid.addColumn(item -> item)
        .setHeader("DOB");

grid.setItems(Arrays.asList("Kevin", "John", "Ted"));

add(grid);

Use Case

I want to keep the row selection (blue background) in sync with the currently focused cell (or active cell editor). It makes sense that if a cell is focused or is being edited then that row should be the selected row.

drewharvey commented 2 years ago

Turns out this issue is worse in Firefox. It seems to happen in Firefox no matter where in the cell you click. Updating ticket to show Firefox as well as Chrome.

sissbruecker commented 2 years ago

Did some quick testing and can verify the bug. First, it works OK when navigating by keyboard. The problem comes solely from clicking into cells. The custom focus event listener is interfering with the default single selection mode implementation, which toggles the selected item when clicking on the same row.

There seems to be a race condition between:

What's interesting is that the server response actually seems to come in between the browsers focus and click event - not sure how that can happen so quickly, as both events should be triggered right after another in the same frame. I might be missing something grid specific about the order of events. This issue might actually just be reproducable when running the app locally, but not when deploying the app on a server.

Workarounds are tricky, because you would somehow need to delay selecting the item on focus, until the click listener was processed and you can safely set selected items + active item.

One thing that works is disabling the deselect on click mechanism. In that case the selection will not be cleared in the click listener:

((GridSingleSelectionModel) grid.getSelectionModel())
                    .setDeselectAllowed(false);
sissbruecker commented 2 years ago

Investigated this some more. I think the behaviour is as intended due to the order of how browser events are processed. When clicking on a cell, the event flow looks like this:

One thing to note here is that this case only occurs when the server-roundtrip (1) finishes before the click event (2) is triggered. When adding a delay to the server-roundtrip, then the issue does not occur.

Given that, changing the selection in the focus phase is not a proper solution, as it interferes with the single selection mode logic that runs afterwards. A proper solution for this use-case should be treated as an enhancement to implement a selection mode that either ties the selection to the cell focus, or to the keyboard navigation.

As for workarounds:

  1. Disable the deselect on click logic in the single selection mode, which prevents (3):
    ((GridSingleSelectionModel) grid.getSelectionModel())
                .setDeselectAllowed(false);
  2. Implement custom logic to run the focus event listener after the click listener. That could look something like this:
    // In `frontend/src/cell-focus-selection.js`
    window.Vaadin = window.Vaadin || {};
    window.Vaadin.Flow = window.Vaadin.Flow || {};
    window.Vaadin.Flow.enableGridSelectOnCellFocus = function (grid) {
      grid.addEventListener("cell-focus", e => {
        setTimeout(() => {
          const eventItem = e.detail.context.item;
          const isSelected = grid.selectedItems && eventItem && grid.selectedItems.some(item => item && item.key === eventItem.key);
          if (isSelected) return;
          grid.$connector.doSelection([eventItem], true);
        });
      });
    }
    // In Flow component
    grid.addAttachListener(attachEvent -> {
        grid.getElement().executeJs("window.Vaadin.Flow.enableGridSelectOnCellFocus(this)");
    });