Skip to content
This repository has been archived by the owner on Dec 12, 2022. It is now read-only.

Adding many rows to grid makes display of rows after scrolling weird #578

Closed
anasmi opened this issue Mar 11, 2019 · 13 comments · Fixed by #777
Closed

Adding many rows to grid makes display of rows after scrolling weird #578

anasmi opened this issue Mar 11, 2019 · 13 comments · Fixed by #777
Assignees
Labels
Projects

Comments

@anasmi
Copy link

anasmi commented Mar 11, 2019

Code to reproduce:

Grid-Flow: 3.0.2

        Grid<String> grid = new Grid<>();
        grid.addColumn(new ComponentRenderer<>(att -> new Span("" + Math.random()))).setHeader("column 1");
        grid.addColumn(new ComponentRenderer<>(att -> new Span("" + Math.random()))).setHeader("column 2");
        grid.addColumn(new ComponentRenderer<>(att -> new Span("" + Math.random()))).setHeader("column 3");
        grid.addColumn(new ComponentRenderer<>(att -> new Span("" + Math.random()))).setHeader("column 4");
        grid.addColumn(new ComponentRenderer<>(att -> new Span("" + Math.random()))).setHeader("column 5");
        grid.addColumn(new ComponentRenderer<>(att -> new Span("" + Math.random()))).setHeader("column 6");
        grid.addColumn(new LocalDateRenderer<>(value -> LocalDate.now())).setHeader("date");

        grid.addColumn(new ComponentRenderer<>(att -> {
            if (att.equals("1")) {
                Icon yep = new Icon(VaadinIcon.CHECK);
                yep.setColor("lime");
                return yep;
            } else {
                Icon nope = new Icon(VaadinIcon.CLOSE);
                nope.setColor("red");
                return nope;
            }
        })).setHeader("Verified").setTextAlign(ColumnTextAlign.CENTER).setKey("districtVerified").setSortable(true);

        grid.setPageSize(20);

        grid.setItems(IntStream.rangeClosed(1, 120000).mapToObj(number -> "" + number));
        add(grid);

I hope this gif makes more sense, than the vague description:
grid_scroll

@heruan
Copy link
Member

heruan commented Mar 11, 2019

I'm experiencing this too in V13. Grid performance degrades while scrolling, and this is noticeable when there are many items. Not sure if component renderers are the issue, maybe @anasmi can test her code without any component renderer?

@anasmi
Copy link
Author

anasmi commented Mar 11, 2019

Checked, indeed not related to the Component Renderers. This code produces exactly the same rendering problems and glitch on clicking:

  Grid<String> grid = new Grid<>();
        grid.addColumn(string->"" + Math.random()).setHeader("column 1");
        grid.addColumn(string->"" + Math.random()).setHeader("column 2");
        grid.addColumn(string->"" + Math.random()).setHeader("column 3");
        grid.addColumn(string->"" + Math.random()).setHeader("column 4");
        grid.addColumn(string->"" + Math.random()).setHeader("column 5");
        grid.addColumn(string->"" + Math.random()).setHeader("column 6");
        grid.addColumn(value -> LocalDate.now()).setHeader("date");

        grid.setPageSize(20);

        grid.setItems(IntStream.rangeClosed(1, 120000).mapToObj(number -> "" + number));
        add(grid);

@denis-anisimov
Copy link

Reproducible with the master branch.
Below is UI code for the test (can be copy pasted).


@Route("scrolling-many-rows")
public class ScrollingManyRowsPage extends Div {

    public ScrollingManyRowsPage() {
        Grid<String> grid = new Grid<>();
        grid.addColumn(string -> String.valueOf(Math.random()))
                .setHeader("column 1");
        grid.addColumn(string -> String.valueOf(Math.random()))
                .setHeader("column 2");
        grid.addColumn(string -> String.valueOf(Math.random()))
                .setHeader("column 3");
        grid.addColumn(string -> String.valueOf(Math.random()))
                .setHeader("column 4");
        grid.addColumn(string -> String.valueOf(Math.random()))
                .setHeader("column 5");
        grid.addColumn(string -> String.valueOf(Math.random()))
                .setHeader("column 6");
        grid.addColumn(value -> LocalDate.now()).setHeader("date");

        grid.setPageSize(20);

        grid.setItems(IntStream.rangeClosed(1, 120000)
                .mapToObj(number -> String.valueOf(number)));
        add(grid);
    }

}

@denis-anisimov
Copy link

Worth to mention that this reminds me this bug : vaadin/vaadin-combo-box-flow#227

@denis-anisimov denis-anisimov modified the milestone: 2.0 maintenance Mar 11, 2019
@pleku pleku added the BFP label Mar 16, 2019
@anasmi anasmi removed the BFP label Mar 19, 2019
@jtomass jtomass added this to 📬  Inbox in vaadin-core Mar 29, 2019
@eriklumme
Copy link

I took a look yesterday out of curiosity, and one issue seems to be that it gets stuck in clear->fetch loops.

Basically when dragging the scrollbar, the DataCommunicator asks the connector to clear the previously requested range from the cache, and then it sends the new items.

Usually the clear method in the connector clears the cache, updates the DOM, but since the cleared items aren’t in the DOM, that’s it.

Sometimes, however, it seems the items that were cleared are in the DOM, but since they have been deleted from the cache, the component does the only sensible thing and fetches them again. This causes the DataCommunicator to clear the active items of the last request, and sends the previous items instead, which triggers the same thing. So it goes back and forth between two pages, clearing them and subsequently re-fetching them because they are actually needed.

The root cause I have no idea… sometimes when scrolling slowly, the values in a given row change completely, so it seems that the grid has no idea of what indices/rows are actually visible.

The loop goes server -> connector -> grid-scroller -> iron-list -> vaadin-grid -> data-provider -> connector -> server

@Peppe
Copy link

Peppe commented Sep 25, 2019

I did a little debugging as well. When the Grid gets into a broken state, it easily goes to a loop of requesting data. e.g. setRequestRange(int start, int length) can be called back and forth between value (1340,20) and (1320,20). Indicated that there is some issue with requesting the correct data, or caching received data.

@eriklumme
Copy link

Yes, when it gets the request to clear the old page, it updates the DOM, and finds real indices for these items, suggesting they are still in view.

My theory is that it goes:

  1. Fetch e.g. 1340, 20
  2. The server data communicator tells the client-side grid to clear the previously requested 1320, 20
  3. The client side removes 1320, 20 from the cache and updates the DOM. Updating the DOM, it notices that the rows for 1320, 20 are visible, and since they were just cleared from the cache, they need to be fetched.
  4. Fetch 1320, 20
  5. The server data communicator tells the client-side grid to clear the previously requested 1340, 20
  6. The client side removes 1340, 20 from the cache and updates the DOM. Updating the DOM, it notices that the rows for 1340, 20 are visible, and since they were just cleared from the cache, they need to be fetched.
  7. Fetch 1340, 20

And so it continues. If this is the case, then the core issue is why multiple pages are in view at once (or why it incorrectly thinks they are).

@Peppe
Copy link

Peppe commented Sep 25, 2019

That's interesting. I minified the problems and noticed that the problem only comes after 100.000 rows, which feels like a pure "human" number, and not a technical one. Here's the minimum case:

package com.example.app.spring;

import com.vaadin.flow.component.grid.Grid;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.router.Route;

import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

@Route("scrolling-many-rows")
public class ScrollingManyRowsPage extends Div {

    public ScrollingManyRowsPage() {
        List<Integer> items = IntStream.rangeClosed(1, 120000).boxed().collect(Collectors.toList());

        Grid<Integer> grid = new Grid<>();
        grid.addColumn(value -> "value " + value).setHeader("value");
        grid.setItems(items);
        add(grid);
    }

}

Some observations:

  1. There is a visual problem with dragging the scroll bar where the rendered rows moves down at all times. Even a minimal scroll wheel render fixes this. Probably not related to this bug, but a separate one.
    fast-scroll-bug

  2. 100.000 is a magical number. Everything works until you get to that row. After that it alternates between rendering and not rendering rows. When it doesn't render, it requests rows back and forth between two ranges.

over-100000-bug

  1. The short page length is not the reason for the problem, but it makes it more visible. The smaller the number, the bigger parts of the list is not rendered.

Talking here with others, it seems like it might be a rounding error that only appears after 100.000, which might in itself be a multiplication of different parameters, and happens on the client. Next would be to try to reproduce this without a server side, directly with

@Peppe
Copy link

Peppe commented Sep 25, 2019

I did the same case with client side Lit element only:

import { LitElement, html } from 'lit-element'; 
import "@vaadin/vaadin-grid"

class GridView extends LitElement { 

  constructor() {
    super();
    var element = this;
    customElements.whenDefined('vaadin-grid').then(function() {
      const grid = element.shadowRoot.getElementById('mygrid');
      grid.size = 120000;
      grid.dataProvider = function(params, callback) {
        console.log(`Fetch: ${params.page * params.pageSize}, Limit: ${params.pageSize}`);
        const response = Array.from({length: params.pageSize}, (v, k) => {return {value: params.page * params.pageSize + k}}); 
        setTimeout(() => callback(response), 500);
      };
    });
  }
  render() {
    return html` 
      <vaadin-grid id="mygrid">
        <vaadin-grid-column path="value"></vaadin-grid-column>
      </vaadin-grid>
    `;
  }
}

customElements.define('grid-view', GridView);

client-side

Problem 1) is there but problem 3) is not. I'll create a new ticket about 1) to vaadin-grid as it is a client side problem. 3) seems to be a connector problem in Flow.

@Peppe
Copy link

Peppe commented Sep 25, 2019

Separate issue for #1: https://github.com/vaadin/vaadin-grid/issues/1659

@tepi
Copy link

tepi commented Sep 26, 2019

I've found the issue in vaadin-grid-flow connector, a fix is coming up.

@tepi
Copy link

tepi commented Sep 26, 2019

PR at #777

@Peppe
Copy link

Peppe commented Sep 26, 2019

Nice work Tepi!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
No open projects
vaadin-core
  
📬  Inbox
Development

Successfully merging a pull request may close this issue.

7 participants