Skip to content

siw: add expand-all toolbar item#9749

Merged
vince-fugnitto merged 1 commit intomasterfrom
vf/siw-expand-all
Jul 20, 2021
Merged

siw: add expand-all toolbar item#9749
vince-fugnitto merged 1 commit intomasterfrom
vf/siw-expand-all

Conversation

@vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Jul 15, 2021

What it does

Fixes: #7714

The pull-request introduces the expand-all toolbar item to the search-in-workspace (siw) widget to expand the result tree when it is collapsed. The toolbar toggles between the collapse-all and expand-all items based on the state of the result tree:

expand-all.mov

The search-in-workspace toolbar items are also updated to their codicon counterparts.

image

How to test

  1. start the application, open the search-in-workspace widget and perform a search with multiple results
  2. perform collapse-all - confirm the toolbar toggles to expand-all and the tree is collapsed
  3. perform expand-all - confirm the toolbar toggles to collapse-all and the tree is expanded
  4. repeat the steps in a multi-root workspace

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves search in workspace issues related to the search-in-workspace labels Jul 15, 2021
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well for me, and the code looks good. Two very optional comments.

Copy link
Contributor

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually collapsing all nodes should change the icon to Expand all but it doesn't,
Not a major problem but it would be good to investigate it.

toggle_to_expand_all

@colin-grant-work
Copy link
Contributor

Manually collapsing all nodes should change the icon to Expand all but it doesn't,
Not a major problem but it would be good to investigate it.

Good catch. To make that work, we'll need to forward the expansion events from the tree to the onDidChange for the tabbar items.

The commit adds an `expand-all` toolbar item to the `search-in-workspace` widget to expand the result tree
when it is collapsed. The commit also updates the icons in the widget to
use the codicon counterparts.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto
Copy link
Member Author

vince-fugnitto commented Jul 19, 2021

@alvsan09 I believe it is addressed if you'd like to retry 👍

expand-all-fix.mp4

Copy link
Contributor

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works as expected and the code looks good to me ! 👍

Symbolic approval as I am not a committer..

@colin-grant-work
Copy link
Contributor

I reaffirm my earlier approval in light of the new changes. 👍

@vince-fugnitto vince-fugnitto merged commit 7689b00 into master Jul 20, 2021
@vince-fugnitto vince-fugnitto deleted the vf/siw-expand-all branch July 20, 2021 15:16
@github-actions github-actions bot added this to the 1.16.0 milestone Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement issues that are enhancements to current functionality - nice to haves search in workspace issues related to the search-in-workspace

Projects

None yet

Development

Successfully merging this pull request may close these issues.

search-in-workspace: update 'collapse' toolbar item to also 'expand'

3 participants