Skip to content
New issue

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

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

Already on GitHub? # to your account

TreeGrid expand does not expand the node at the bottom of the grid #2055

Closed
jcgueriaud1 opened this issue Oct 6, 2020 · 3 comments · Fixed by #2058
Closed

TreeGrid expand does not expand the node at the bottom of the grid #2055

jcgueriaud1 opened this issue Oct 6, 2020 · 3 comments · Fixed by #2058
Labels
BFP Bug fix prioritised by a customer bug Something isn't working

Comments

@jcgueriaud1
Copy link
Contributor

jcgueriaud1 commented Oct 6, 2020

Description

In a TreeGrid, The last row is expanded from the server side.
If you scroll to the end of the grid then the row is "expanded" (the icon looks like expanded) but the the children are not visible.
If you resize the view or if you call again scrollToEnd() then the children are visible.

Expected outcome

The children should be visible and the scrolltoend should scroll to the end of the TreeGrid

Actual outcome

The children are invisible and the scrolltoend is scrolling to the parent to the end of the TreeGrid

Live Demo

I've attached the entire project in Vaadin 14.3.8. (I can reproduce it in 17.0.6)

@Route(value = "")
public class FocusTreeGridView extends VerticalLayout {

    private DepartmentData departmentData = new DepartmentData();


    public FocusTreeGridView() {

        TreeGrid<Department> grid = buildGrid();
/*
        add(new Button("expand last item", e -> {
            grid.expand(departmentData.getRootDepartments().stream().filter(department -> department.getId() == 300).findFirst().get());
        }));*/
        add(new Button("scrolltoEnd", e -> grid.scrollToEnd()));

        addAndExpand(grid);
        setPadding(false);
        setSizeFull();
    }

    private TreeGrid<Department> buildGrid() {
        TreeGrid<Department> grid = new TreeGrid<>();
        grid.setItems(departmentData.getRootDepartments(),
            departmentData::getChildDepartments);
        grid.addHierarchyColumn(Department::getName).setHeader("Department Name").setKey("name");
        grid.setWidthFull();
        grid.expand(departmentData.getRootDepartments().stream().filter(department -> department.getId() == 300).findFirst().get());
        return grid;
    }
}

public class DepartmentData {
    private static final List<Department> DEPARTMENT_LIST = createDepartmentList();

    private Department expandedItem;

    private static List<Department> createDepartmentList() {
        List<Department> departmentList = new ArrayList<>();

        departmentList
                .add(new Department(1, "Product Development", null, "Päivi"));
        departmentList.add(
                new Department(11, "Flow", departmentList.get(0), "Pekka"));
        departmentList.add(new Department(111, "Flow Core",
                departmentList.get(1), "Pekka"));
        departmentList.add(new Department(112, "Flow Components",
                departmentList.get(1), "Gilberto"));
        departmentList.add(
                new Department(12, "Design", departmentList.get(0), "Pekka"));
        departmentList.add(
                new Department(13, "DJO", departmentList.get(0), "Thomas"));
        departmentList.add(
                new Department(14, "Component", departmentList.get(0), "Tomi"));
        departmentList.add(new Department(2, "HR", null, "Anne"));
        departmentList.add(
                new Department(21, "Office", departmentList.get(7), "Anu"));
        departmentList.add(
                new Department(22, "Employee", departmentList.get(7), "Minna"));
        departmentList.add(new Department(3, "Marketing", null, "Niko"));
        departmentList.add(
                new Department(31, "Growth", departmentList.get(10), "Ömer"));
        departmentList.add(new Department(32, "Demand Generation",
                departmentList.get(10), "Marcus"));
        departmentList.add(new Department(33, "Product Marketing",
                departmentList.get(10), "Pekka"));
        departmentList.add(new Department(34, "Brand Experience",
                departmentList.get(10), "Eero"));
        departmentList.add(
                new Department(41, "Sub-Office ", departmentList.get(9), "Anu1"));
        departmentList.add(
                new Department(52, "Sub-Office 2", departmentList.get(15), "Anu2"));
        for (int i = 0; i < 98; i++) {
            departmentList
                    .add(new Department(200+i, "T "+i, null, "Test"));
        }
        Department department = new Department(300, "T 300", null, "Test");
        departmentList.add(department);
        addChildren(departmentList, department);

        departmentList
            .add(new Department(500, "Last", null, "Last"));
        return departmentList;

    }

    private static void addChildren(List<Department> departmentList, Department parent) {
        for (int i = 0; i < 60; i++) {
            departmentList
                .add(new Department(301+i, "T 300-"+i, parent, "Test"));
        }
    }

    public List<Department> getDepartments() {
        return DEPARTMENT_LIST;
    }

    public List<Department> getRootDepartments() {
        return DEPARTMENT_LIST.stream()
                .filter(department -> department.getParent() == null)
                .collect(Collectors.toList());
    }

    public List<Department> getChildDepartments(Department parent) {
        return getChildDepartments(parent, 0);
    }
    public List<Department> getChildDepartments(Department parent, int offset) {
        List<Department> departmentList = DEPARTMENT_LIST.stream().filter(
                department -> Objects.equals(department.getParent(), parent))
                .collect(Collectors.toList());
        return departmentList.subList(offset, departmentList.size());
    }

    public boolean hasChildren(Department parent) {
        return DEPARTMENT_LIST.stream().anyMatch(
                department -> Objects.equals(department.getParent(), parent));
    }

}

Steps to reproduce

Go on http://localhost:8080/ and click on the button.

Browsers Affected

Tested on Opera, Chrome and FF (MacOs)

Screen Recording 2020-10-06 at 18.13.22.mov.zip

skeleton-starter-flow-14.zip

@tulioag
Copy link
Contributor

tulioag commented Oct 7, 2020

I can reproduce in master by placing the following java file in the integration-tests module and accessing it at http://localhost:9998/focustreegrid

package com.vaadin.flow.component.treegrid.it;

import com.vaadin.flow.component.html.NativeButton;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.component.treegrid.TreeGrid;
import com.vaadin.flow.router.Route;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

@Route
public class FocusTreeGridView extends VerticalLayout {

    private DepartmentData departmentData = new DepartmentData();

    public FocusTreeGridView() {

        TreeGrid<Department> grid = buildGrid();
/*
        add(new Button("expand last item", e -> {
            grid.expand(departmentData.getRootDepartments().stream().filter(department -> department.getId() == 300).findFirst().get());
        }));*/
        add(new NativeButton("scrolltoEnd", e -> grid.scrollToEnd()));

        addAndExpand(grid);
        setPadding(false);
        setSizeFull();
    }

    private TreeGrid<Department> buildGrid() {
        TreeGrid<Department> grid = new TreeGrid<>();
        grid.setItems(departmentData.getRootDepartments(),
            departmentData::getChildDepartments);
        grid.addHierarchyColumn(Department::getName)
            .setHeader("Department Name").setKey("name");
        grid.setWidthFull();
        grid.expand(departmentData.getRootDepartments().stream()
            .filter(department -> department.getId() == 300).findFirst().get());
        return grid;
    }

    public static class DepartmentData {
        private static final List<Department> DEPARTMENT_LIST = createDepartmentList();

        private Department expandedItem;

        private static List<Department> createDepartmentList() {
            List<Department> departmentList = new ArrayList<>();

            departmentList
                .add(new Department(1, "Product Development", null, "Päivi"));
            departmentList.add(
                new Department(11, "Flow", departmentList.get(0), "Pekka"));
            departmentList.add(
                new Department(111, "Flow Core", departmentList.get(1),
                    "Pekka"));
            departmentList.add(
                new Department(112, "Flow Components", departmentList.get(1),
                    "Gilberto"));
            departmentList.add(
                new Department(12, "Design", departmentList.get(0), "Pekka"));
            departmentList.add(
                new Department(13, "DJO", departmentList.get(0), "Thomas"));
            departmentList.add(
                new Department(14, "Component", departmentList.get(0), "Tomi"));
            departmentList.add(new Department(2, "HR", null, "Anne"));
            departmentList.add(
                new Department(21, "Office", departmentList.get(7), "Anu"));
            departmentList.add(
                new Department(22, "Employee", departmentList.get(7), "Minna"));
            departmentList.add(new Department(3, "Marketing", null, "Niko"));
            departmentList.add(
                new Department(31, "Growth", departmentList.get(10), "Ömer"));
            departmentList.add(
                new Department(32, "Demand Generation", departmentList.get(10),
                    "Marcus"));
            departmentList.add(
                new Department(33, "Product Marketing", departmentList.get(10),
                    "Pekka"));
            departmentList.add(
                new Department(34, "Brand Experience", departmentList.get(10),
                    "Eero"));
            departmentList.add(
                new Department(41, "Sub-Office ", departmentList.get(9),
                    "Anu1"));
            departmentList.add(
                new Department(52, "Sub-Office 2", departmentList.get(15),
                    "Anu2"));
            for (int i = 0; i < 98; i++) {
                departmentList
                    .add(new Department(200 + i, "T " + i, null, "Test"));
            }
            Department department = new Department(300, "T 300", null, "Test");
            departmentList.add(department);
            addChildren(departmentList, department);

            departmentList.add(new Department(500, "Last", null, "Last"));
            return departmentList;

        }

        private static void addChildren(List<Department> departmentList,
            Department parent) {
            for (int i = 0; i < 60; i++) {
                departmentList
                    .add(new Department(301 + i, "T 300-" + i, parent, "Test"));
            }
        }

        public List<Department> getDepartments() {
            return DEPARTMENT_LIST;
        }

        public List<Department> getRootDepartments() {
            return DEPARTMENT_LIST.stream()
                .filter(department -> department.getParent() == null)
                .collect(Collectors.toList());
        }

        public List<Department> getChildDepartments(Department parent) {
            return getChildDepartments(parent, 0);
        }

        public List<Department> getChildDepartments(Department parent,
            int offset) {
            List<Department> departmentList = DEPARTMENT_LIST.stream().filter(
                department -> Objects.equals(department.getParent(), parent))
                .collect(Collectors.toList());
            return departmentList.subList(offset, departmentList.size());
        }

        public boolean hasChildren(Department parent) {
            return DEPARTMENT_LIST.stream().anyMatch(
                department -> Objects.equals(department.getParent(), parent));
        }

    }

    public static class Department {
        private int id;
        private String name;
        private String manager;
        private boolean archive;
        private Department parent;

        public Department(int id, String name, Department parent,
            String manager) {
            this.id = id;
            this.name = name;
            this.manager = manager;
            this.parent = parent;
        }

        public int getId() {
            return id;
        }

        public void setId(int id) {
            this.id = id;
        }

        public String getName() {
            return name;
        }

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

        public String getManager() {
            return manager;
        }

        public void setManager(String manager) {
            this.manager = manager;
        }

        public Department getParent() {
            return parent;
        }

        public void setParent(Department parent) {
            this.parent = parent;
        }

        public boolean isArchive() {
            return archive;
        }

        public void setArchive(boolean archive) {
            this.archive = archive;
        }

        @Override
        public String toString() {
            return name;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o)
                return true;
            if (o == null || getClass() != o.getClass())
                return false;

            Department that = (Department) o;

            return id == that.id;
        }

        @Override
        public int hashCode() {
            return id;
        }
    }
}

@tomivirkki tomivirkki added the bug Something isn't working label Oct 7, 2020
@tomivirkki
Copy link
Member

This is an issue in the <vaadin-grid> Web Component. Seems that in this case, ensureSubCacheForScaledIndex never gets invoked for the parent item ("T 300")...

  • should happen here for items that are already cached and then scrolled into the viewport
  • or here for items that are already in the viewport when data for them is received.

@tomivirkki tomivirkki added the BFP Bug fix prioritised by a customer label Oct 7, 2020
jcgueriaud1 added a commit to vaadin-component-factory/selection-grid-flow that referenced this issue Oct 14, 2020
jcgueriaud1 added a commit to vaadin-component-factory/selection-grid-flow that referenced this issue Oct 14, 2020
jcgueriaud1 added a commit to vaadin-component-factory/selection-grid-flow that referenced this issue Oct 14, 2020
jcgueriaud1 added a commit to vaadin-component-factory/selection-grid-flow that referenced this issue Oct 14, 2020
@tomivirkki
Copy link
Member

#2058 closed the issue based on the title "TreeGrid expand does not expand the node at the bottom of the grid". There was an actual issue in the Web Component that got fixed and now items get properly expanded when scrolled to.

The description in the "Expected outcome" is another thing however: "The children should be visible and the scrolltoend should scroll to the end of the TreeGrid". This isn't a bug per se but rather a UX enhancement request. We discussed this internally and concluded with the following enhancements. The second one especially is a continuation to the discussion on what should happen in general when the grid's size changes dynamically (was it due to item getting expanded or something else);

For now, you can invoke scrollToEnd() again on parent node expand to have grid scrolled to end also after child items are loaded.

jcgueriaud1 added a commit to vaadin-component-factory/selection-grid-flow that referenced this issue Oct 16, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
BFP Bug fix prioritised by a customer bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants