Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TreeGrid] Should refresh only visible items instead of triggering full data reset where possible #6713

Open
vursen opened this issue Oct 11, 2024 · 2 comments
Labels
needs discussion No decision yet, discussion needed UX User experience issue vaadin-grid

Comments

@vursen
Copy link
Contributor

vursen commented Oct 11, 2024

Description

setDropMode, setDragFilter, setPartNameGenerator, and many other TreeGrid methods call dataCommunicator.reset to re-render items, typically to update certain attributes. This operation is effectively equivalent to refreshAll and triggers a full reset of the hierarchy, which causes an annoying scroll position reset on the client-side.

In cases where the grid's hierarchy isn't expected to change – which is true for all the mentioned methods – it should be enough to simply call refreshItem for items that are currently visible.

Expected outcome

The Flow framework should provide an API to retrieve items that are currently in the viewport. With this API, the Flow component could call refreshItem for visible items instead of doing a full reset.

Minimal reproducible example

This issue has become especially problematic after #6339 added a data communicator reset to setDropMode. When setting the drop mode dynamically on drag start, the scroll can now jump to a quite random position, significantly disrupting the dragging process.

@Route("tree-grid-bug")
public class TreeGridBugPage extends Div {
    public TreeGridBugPage() {
        TreeGrid<String> treeGrid = new TreeGrid<>();
        treeGrid.addHierarchyColumn(item -> item);
        treeGrid.setRowsDraggable(true);

        treeGrid.setDragFilter(item -> item.startsWith("Child-"));
        treeGrid.setDropFilter(item -> item.startsWith("Child-"));

        TreeData<String> treeData = new TreeData<>();
        for (int i = 0; i < 100; i++) {
            String item = "Root-" + i;
            treeData.addItem(null, item);
            for (int j = 0; j < 10; j++) {
                treeData.addItem(item, "Child-" + i + "-" + j);
            }
        }
        treeGrid.setTreeData(treeData);

        treeGrid.addDragStartListener(e -> {
            treeGrid.setDropMode(GridDropMode.ON_TOP_OR_BETWEEN);
        });
        treeGrid.addDropListener(e -> {
            treeGrid.setDropMode(null);
        });

        add(treeGrid);
    }
}

Steps to reproduce

Screen.Recording.2024-10-11.at.15.45.42.mov

Environment

Vaadin version(s): 24.5 and earlier
OS: Mac OS

Browsers

No response

@vursen
Copy link
Contributor Author

vursen commented Oct 11, 2024

@vursen
Copy link
Contributor Author

vursen commented Oct 11, 2024

I'm attaching the branch with my experiment to implement such an API in Flow:

https://github.com/vaadin/flow/compare/get-requested-range-items-method

The solution isn't complete. The API can currently return more items than necessary because DataCommunicator keeps expanded items in the key mapper after they have been visited once. Some additional logic is needed to filter out such items when they are out of the requested range.

@rolfsmeds rolfsmeds added the needs discussion No decision yet, discussion needed label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion No decision yet, discussion needed UX User experience issue vaadin-grid
Projects
None yet
Development

No branches or pull requests

2 participants