vaadin / flow-components

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

TreeGrid scrolls back to top after a refresh. #1236

Open mcfar opened 5 years ago

mcfar commented 5 years ago

If you select a row in a tree grid and it is outside the viewport, and then call a refreshAll() on it, it scrolls to top of the grid. If you have a row selected and call a refresh, I'd expect it to retain the selected row in the viewPort. We have a use case where the user can change the display value for the hierarchy column. When the new caption is changed, the tree grid calls a refreshAll() so the new caption will be used.

Example `public class TreeGridMainView extends VerticalLayout { private static final String NAME = "name"; private static final String DESCRIPTION = "description"; private TreeDataProvider dataProvider; private TreeGrid treeGrid; private String caption;

public TreeGridMainView() {
    HorizontalLayout buttonLayout = new HorizontalLayout();
    Button name = new Button("Name");
    name.addClickListener(event -> setCaptionProperty(NAME));
    Button description = new Button("Description");
    description.addClickListener(event -> setCaptionProperty(DESCRIPTION));
    buttonLayout.add(name, description);

    initializeDataProvider();
    treeGrid = new TreeGrid<>(dataProvider);
    add(buttonLayout, treeGrid);

    treeGrid.addColumn(TemplateRenderer
            .<TreeNode> of(
                    "<vaadin-grid-tree-toggle " + "leaf='[[item.leaf]]' expanded='{{expanded}}' level='[[level]]'>"
                            + "<span>[[item.name]]</span>" + "</vaadin-grid-tree-toggle>")
            .withProperty("leaf", item -> !dataProvider.hasChildren(item))
            .withProperty("name", this::generateCaption)).setFlexGrow(1);

    caption = NAME;
}

public void setCaptionProperty(String property) {
    caption = property;
    treeGrid.getDataCommunicator().getKeyMapper().removeAll();
    dataProvider.refreshAll();
}

private String generateCaption(TreeNode node) {
    String property = null;

    if (NAME.equals(caption)) {
        property = node.getName();
    } else if (DESCRIPTION.equals(caption)) {
        property = node.getDescription();
    } else {
        property = node.getName();
    }

    return property;
}

/**
 * 
 */
private void initializeDataProvider() {
    TreeData<TreeNode> treeData = new TreeData<>();
    TreeNode root = new TreeNode("root", "top level");
    treeData.addRootItems(root);
    treeData.addItems(root, generateChildren(root, 20));
    for (TreeNode child : treeData.getChildren(root)) {
        treeData.addItems(child, generateChildren(child, 10));
        for (TreeNode grandChild : treeData.getChildren(child)) {
            treeData.addItems(grandChild, generateChildren(grandChild, 7));
        }
    }
    dataProvider = new TreeDataProvider<>(treeData);
}

private List<TreeNode> generateChildren(TreeNode parent, int nosChildren) {
    List<TreeNode> children = new ArrayList<>();
    for (int i = 0; i < nosChildren; i++) {
        StringBuilder childName = new StringBuilder();
        childName.append("child: ");
        childName.append(i);
        childName.append(", of parent: ");
        childName.append(parent.getName());
        TreeNode child = new TreeNode(childName.toString(), "child " + i);
        children.add(child);
    }
    return children;
}

}`

`public class TreeNode { private String name; private String description; private UUID code = UUID.randomUUID();

public TreeNode(String name, String description) {
    this.name = name;
    this.description = description;
}

public String getName() {
    return name;
}

public void setName(String name) {
    this.name = name;
}

public String getDescription() {
    return description;
}

public void setDescription(String description) {
    this.description = description;
}

public UUID getCode() {
    return code;
}

@Override
public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + ((code == null) ? 0 : code.hashCode());
    return result;
}

@Override
public boolean equals(Object obj) {
    if (this == obj) {
        return true;
    }
    if (obj == null) {
        return false;
    }
    if (!(obj instanceof TreeNode)) {
        return false;
    }
    TreeNode other = (TreeNode)obj;
    if (code == null) {
        if (other.code != null) {
            return false;
        }
    } else if (!code.equals(other.code)) {
        return false;
    }
    return true;
}

@Override
public String toString() {
    StringBuilder builder = new StringBuilder();
    builder.append("TreeNode [name=").append(name).append(", description=").append(description).append(", code=")
            .append(code).append("]");
    return builder.toString();
}

}`

conor-hackett commented 1 year ago

Any update on this?

I have a use case where a treegrid will be regularly updated with changes to data in the backend. Items being added or removed by other user sessions will necessitate refreshAll() being called to remain up to date. Each update resulting in the scroll bar resetting to the top is blocking progress as it results in poor user experience.

stefanuebe commented 5 months ago

We have the same issue. The reason seems to be, that the initial scroll height is not calculated based on all potentially available rows, but the root rows only.

One workaround could be to extend the hierarchical data provider api to allow the dev to set the expected amount of total rows parallel to the root rows. Based on this the grid can precalculate its expected scroll height. Hoever this value should not have the "last word", i.e., when the resulting size is more the three grid scroll height should grow as needed (as it is now already the case).

e.g.

public interface HierarchicalDataProvider<T, F> extends DataProvider<T, F> {

    /**
      * Returns the expected amount of total rows (root rows plus all child rows). This value is taken to precalculate the 
      * expected scroll height of the using component (e. g. a tree grid). 
      * <p/>
      * Default calls size().
      * @return total amount of rows to be expected
      */
    default int totalSize(Query<T, F> query) {
        return size(query);
    }

}
TatuLund commented 5 months ago

I documented workaround here: https://github.com/vaadin/flow-components/issues/6307#issuecomment-2122250952

stefanuebe commented 5 months ago

The workaround is unfortunately not suitable for all situations. A large TG with expanded rows has a much higher refresh time using the workaround (refreshAll ~5s, workaround ~35s).

dobibart commented 3 months ago

The workaround doesn't work for root items being added or removed. They do not appear in the grid, although they are being listed in the root items when calling getRootItems().