vaadin / flow-components

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

Components are re-rendered when details open state changes in Grid #5960

Closed onuridrisoglu closed 9 months ago

onuridrisoglu commented 9 months ago

Description

In Grid, component columns are re-rendered when details open state change for a specific row. This causes flickering images, when a user clicks a row to open details.

Expected outcome

Rendered components stay the same as details state doesn't have any effect on the created component

Minimal reproducible example

public class MainView extends VerticalLayout {

    private Grid<String> grid;

    public MainView() {

                grid = new Grid<String>();
        grid.addComponentColumn(this::generateComponent);
        grid.setItemDetailsRenderer(new TextRenderer<String>(s -> s));
        grid.setItems("A", "B", "C");

        add(grid);

    }

    private Component generateComponent(String str) {
        Notification.show("Generating component for " + str);
        return new Span(str);
    }

}

Steps to reproduce

  1. click on a row to open details
  2. click on another row
  3. Generating component X notification will appear for both rows

Environment

Vaadin: 23.3.31 Flow: 23.3.27 Java: Eclipse Adoptium 11.0.16.1 OS: aarch64 Mac OS X 14.2.1 Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36 Live reload: Java active (Spring Boot Devtools): Front end active

Browsers

No response

sissbruecker commented 9 months ago

Note that the grid can not know whether the component renderer implementation uses the details state or not. It would be perfectly possible to implement a component renderer that renders differently based on whether the details for a row are visible or not by using Grid.isDetailsVisible. If we would prevent a re-render when the details state changes that would stop working.

I would suggest to implement the appropriate API for updating a rendered component instead of always creating new ones, which in this case would be a no-op:

grid.addComponentColumn(this::generateComponent, (component, item) -> component));

With this change each component is only created once per cell and then kept, which also improves performance in a number of scenarios (refresh item, refresh all, selection).

As we had similar cases before where we decided this is the expected behavior, I would suggest to close this issue as well. See also:

yuriy-fix commented 9 months ago

Closing as per comment provided above.