vaadin / flow-components

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

Slow renderer/scroll horizontal in grid with many columns Vaadin 23.0.x #2930

Closed darjag closed 2 years ago

darjag commented 2 years ago

Description

Test Vaadin14 and Vaadin21 it's works OK

Expected outcome

faster horizontal scroll without lags

Actual outcome

Slow renderer columns, slow horizontal scrolling most visible on mobile devices, but also visible on desktop devices when compare to Vaadin 14 or 21

Minimal reproducible example

import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.List;

import com.vaadin.flow.component.grid.Grid;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.router.AfterNavigationEvent;
import com.vaadin.flow.router.AfterNavigationObserver;
import com.vaadin.flow.router.Route;
import lombok.Getter;

@Route("gridTest")
public class GridTextView extends VerticalLayout implements AfterNavigationObserver {

    Grid<LotOfColumns> grid = new Grid<>(LotOfColumns.class);

    public GridTextView() {
        setSizeFull();
        grid.setSizeFull();
        grid.setColumns("id",
                "c01", "c02", "c03", "c04", "c05", "c06", "c07", "c08", "c09", "c10",
                "c11", "c12", "c13", "c14", "c15", "c16", "c17", "c18", "c19", "c20",
                "c21", "c22", "c23", "c24", "c25", "c26", "c27", "c28", "c29", "c30",
                "c31", "c32", "c33", "c34", "c35", "c36", "c37", "c38", "c39", "c40",
                "c41", "c42", "c43", "c44", "c45", "c46", "c47", "c48", "c49", "c50",
                "c51", "c52", "c53", "c54", "c55", "c56", "c57", "c58", "c59", "c60",
                "c61", "c62", "c63", "c64", "c65", "c66", "c67", "c68", "c69", "c70",
                "c71", "c72", "c73", "c74", "c75", "c76", "c77", "c78", "c79", "c80",
                "c81", "c82", "c83", "c84", "c85", "c86", "c87", "c88", "c89", "c90",
                "c91", "c92", "c93", "c94", "c95", "c96", "c97", "c98", "c99", "c100");
        add(grid);
    }

    @Override
    public void afterNavigation(AfterNavigationEvent event) {
        List<LotOfColumns> items = new ArrayList<>(100);
        for (int i = 0; i < 100; i++) {
            items.add(new LotOfColumns(i));
        }
        grid.setItems(items);
    }

    @Getter
    public static class LotOfColumns {
        private long id;
        private String c01, c02, c03, c04, c05, c06, c07, c08, c09, c10;
        private String c11, c12, c13, c14, c15, c16, c17, c18, c19, c20;
        private String c21, c22, c23, c24, c25, c26, c27, c28, c29, c30;
        private String c31, c32, c33, c34, c35, c36, c37, c38, c39, c40;
        private String c41, c42, c43, c44, c45, c46, c47, c48, c49, c50;
        private String c51, c52, c53, c54, c55, c56, c57, c58, c59, c60;
        private String c61, c62, c63, c64, c65, c66, c67, c68, c69, c70;
        private String c71, c72, c73, c74, c75, c76, c77, c78, c79, c80;
        private String c81, c82, c83, c84, c85, c86, c87, c88, c89, c90;
        private String c91, c92, c93, c94, c95, c96, c97, c98, c99, c100;

        public LotOfColumns() {
            super();
        }

        public LotOfColumns(int row) {
            super();
            this.id = row;
            try {
                for (Field field : getClass().getDeclaredFields()) {
                    if (String.class.equals(field.getType())) {
                        field.set(this, "row" + row + field.getName());
                    }
                }
            } catch (ReflectiveOperationException e) {
                throw new RuntimeException(e);
            }
        }

        public long getId() {
            return id;
        }

        public String getC01() {
            return c01;
        }
    }
}

Steps to reproduce

  1. Open View /gridTest
  2. Try fast scroll horizontal

Environment

Browsers Affected

tomivirkki commented 2 years ago

I think this is a performance regression form https://github.com/vaadin/web-components/pull/2396

We should change __updateHorizontalScrollPosition back to not apply the custom CSS property --_grid-horizontal-scroll-position and instead use manual transform for cells (like currently used for RTL mode). --_grid-horizontal-scroll-position is still needed for the row focus mode, but its value can be updated on cell focus.

web-padawan commented 2 years ago

I guess this also applies to the new custom property added in #3566 so we would need to remove it as well.