Filter button in search box

[colin-grant-work]

Signed-off-by: Colin Grant [email protected]

What it does

This PR fixes #8380 by adding filtering functionality to the tree search box to parallel VSCode's tree search functionality. This makes breaking changes to the logic of the TreeSearch.filter function and the TreeWidget.doUpdateRows function.

I have a couple of questions:

• I've introduced a new field _filteredNodesAndParents in TreeSearch to hold all of the nodes that should be visible when the filter is activated. It might be possible to use one of the existing fields (_filteredNodes), but it should turned into a hash rather than array to avoid traversing the whole thing for every visibility check.
• I've introduced a shouldDisplayNode method in the TreeWidget elsewhere @akosyakov has said that visibility in TreeWidgets should rely on the node's .visible to determine visibility. I don't believe that any current Theia code modifies the .visible field of a TreeNode, so I opted not to modify it either - especially as that would have required looping over all nodes again after the filter, not just the nodes that pass the filters. In general, there's a division between using a node's fields directly or using a service (e.g. LabelProvider) to derive usable data, and I'm not sure on which side to come down.

How to test

1. Open a tree widget with a search box (e.g. file explorer).
2. Activate the search box by typing while the Tree Widget is active.
3. Type text that selects some nodes in the tree.
4. Click the filter button to the right of the textbox.
5. Observe that all highlighted children and all their parents remain visible.
6. Observe that you can expand and collapse nodes without losing the search box.
7. Close the search box by clicking the x icon or hitting escape.
8. Observe that the tree returns to normal.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[kenneth-marut-work]

Nice improvements, from a functionality standpoint there are a couple things that feel odd about the filter, but most are outside the scope of this MR. I think the main issue I'm seeing is that after applying the filter to the tree, the up/down actions don't traverse the nodes in an obvious way. For example, if the filter is applied, but a directory is closed then you don't get an automatic expansion when child nodes are highlighted.

I've also noticed a strange traversal route when you reach the first child of a parent node, see the GIF below:

[colin-grant-work]

@kenneth-marut-work, thanks for catching that difference in the path traversal. I had missed a reference to the _filteredNodes field, and it turned out the way I'd changed its contents affected the way the traversal worked. For now, I've switched to using a separate field to track what should be visible, and that seems to have fixed the problem.

[vince-fugnitto]

I'll continue the review tomorrow, likely after we perform the release 👍

[vince-fugnitto]

The changes work very well ! 👍
It's a very nice improvement to be able to filter down the nodes in the trees:

• verified the explorer
• verified the outline-view

The scm-view can be improved as when filtering down changes in the view (with the tree mode), we lost the parent changes entry (which has toolbar items).

[colin-grant-work]

Hmm, good find. I'll see why that parent isn't being preserved like other parents.

[colin-grant-work]

I had made a silly error that was terminating the traversal of the parent nodes after a single parent was added. I've fixed it, and it appears that the SCM view is working as it should. 🤞

[vince-fugnitto]

The changes work very well for the most part 👍
The only issue I encountered was with the scm view, when a node is correctly identified as a match, collapsing the tree results in the identified node being lost:

I do think its minor however if the bug cannot be fixed properly, but does result in a weird state for end-users.

[colin-grant-work]

Another good catch. I've added code to hide the search widget when the SCM tree changes view modes, since that invalidates the search results anyway. I tested the file explorer, and it doesn't have problems with collapse all because collapsing doesn't fundamentally change the tree the way that changing view modes changes the SCM tree.

[vince-fugnitto]

The changes work very well, I verified the following use-cases:

Use Case	Result
explorer (single root)	👍
explorer (multi-root)	👍
scm (tree)	👍
scm (list)	👍
outline	👍

The code changes also look very good.

[paul-marechal]

Code LGTM