GridKt._clickItem(grid, rowIndex) doesn't expand rows in TreeGrid

[brunovianarezende]

I have a TreeGrid with 3 root items, which are collapsed. When I click in the first one, I'd expect its children to be expanded, but it doesn't happen. This is my code:

    assertEquals(_size(treeGrid), 3);
    _clickItem(treeGrid, 0);
    assertTrue(_size(treeGrid) > 3);

if, instead, I do:

    assertEquals_size(treeGrid), 3);
    _clickItem(treeGrid, 0);
    treeGrid.expand(treeGrid.getSelectedItems());
    assertTrue(_size(treeGrid) > 3);

it works. I tried to do use _clickRenderer or to call _getCellComponent without success. I suspect that the problem is because the hierachical collum is rendered by using TemplateRender: https://github.com/vaadin/flow-components/blob/master/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/treegrid/TreeGrid.java#L522.

I wonder what should be done:

• make _clickItem/_clickRenderer work with TemplateRenderer (not sure if it is possible)
• make _clickItem call expand when grid is a TreeGrid
• document the issue and describe the workaround
• something else

[mvysny]

Hi Bruno, thank you for letting me know! You're right, the _clickItem() function name suggests that an actual mouse click is to be emulated; if the tree is expanded or collapsed in the actual TreeGrid implementation as a part of the click, then Karibu should emulate the expansion/collapse as well. (Similar to #96 where a selection event is emulated). Let me investigate on this a bit please.

[mvysny]

Hmm, this is probably a bit trickier. Please take a look at the following code (sorry for Kotlin but it's very simple and should be easy to follow):

class MainView : VerticalLayout() {
    init {
        treeGrid<String> {
            selectionMode = Grid.SelectionMode.NONE
            addHierarchyColumn { it } .setHeader("Foo")
            addColumn { "$it foo" } .setHeader("Non-hierarchical column")

            val td = TreeData<String>()
            td.addRootItems("a item", "b", "c")
            td.addItem("a item", "aa")
            td.addItem("a item", "ab")
            td.addItem("a item", "ac")
            dataProvider = TreeDataProvider<String>(td)

            addExpandListener { println("EXPAND: $it") }
            addCollapseListener { println("COLLAPSE: $it") }
            addItemClickListener { println("ITEM CLICK: $it") }
        }
    }
}

It shows the following grid:

What happens when I click individual parts of the grid?

1. Clicking on the ">" expand icon or on the "a item" text indeed expands the children (and fires the expand listener) but it doesn't fire the ItemClickEvent.
2. Clicking on the empty space between "> a item" and "a item foo" in the next column fires ItemClickEvent but doesn't expand the item nor does it fire the expand listener.
3. Clicking on the "a item foo" fires ItemClickEvent but doesn't expand.

The behavior changes a bit on the next line since "b" doesn't have any children:

1. Anywhere you click, only the ItemClickEvent is fired; nothing is ever expanded and the expand listener is never called.

This tells me that "item expansion" and "item click events" are somewhat exclusive: if one gets fired the other one doesn't and vice-versa. That leads me to believe that the _clickItem() should not in fact expand or collapse. However, a click on "> a item" is still a mouse click, which leads users to naturally expect for the _clickItem() to perform the expansion and/or collapse. Which leads us to a statement that _clickItem() both should and should not perform the expansion, which is a contradiction.

At the moment I don't have a clear idea on what to do. On one hand I strive to design my APIs to "work as expected/anticipated/obviously it should work like this :-D ", which would mean for _clickItem() to perform the expansion; yet I'm hesitant to implement that since the actual TreeGrid never fires both expand listener and ItemClickListener at the same time.

@brunovianarezende what are your thoughts on this please?

[mvysny]

I think the "make _clickItem/_clickRenderer work with TemplateRenderer (not sure if it is possible)" is probably not possible: the expansion/collapse is most probably implemented within the javascript implementation of the <vaadin-grid-tree-toggle> component; since Karibu doesn't run JavaScript code there's no generic solution that can make this work. Therefore, a workaround will be necessary :) So it's either:

• make _clickItem call expand when grid is a TreeGrid, OR
• document the issue and describe the workaround; the workaround could simply be to call the treeGrid.expand(treeGrid.getSelectedItems()); as you suggested.

I'm hesitantly starting to lean towards the latter; what do you think please? Alternatively, we could introduce _clickItemAndExpand() which would both click the item and expand the row, but I'm hesitant do to that since the TreeGrid itself never seems to do that either...

[brunovianarezende]

@mvysny, I've tested it locally and I can confirm the behavior you described: depending of the place you click in the row, the TreeGrid might do just one of click and expand, they don't happen simultaneously. This means we shouldn't do 'click and expand' at the same time.

I think _clickItem should do what its name says: 'click'; this will allow a user to test what happens when a row is clicked, but not expanded. And, then, we should just document that for expanding a TreeGrid row the user must call the expand method.

[mvysny]

Sounds good, thank you for your reply. I'll update the documentation accordingly.

[brunovianarezende]

Thanks for your time and for the amazing framework, Martin! :smile:

[mvysny]

No problem @brunovianarezende , happy to help :)