Skip to content

Add expand-all action in search - #85428

Merged
Jackson Kearl (JacksonKearl) merged 6 commits into
microsoft:masterfrom
okmttdhr:expand-button-in-search
Dec 13, 2019
Merged

Add expand-all action in search#85428
Jackson Kearl (JacksonKearl) merged 6 commits into
microsoft:masterfrom
okmttdhr:expand-button-in-search

Conversation

@okmttdhr

@okmttdhr okmttdhr, tada (okmttdhr) commented Nov 23, 2019

Copy link
Copy Markdown
Contributor

This PR fixes #28090

Image from Gyazo

(Please let me know if there's a smart way to implement unit tests).

@okmttdhr
okmttdhr, tada (okmttdhr) marked this pull request as ready for review November 23, 2019 01:28
@IViewletService private readonly viewletService: IViewletService,
@IPanelService private readonly panelService: IPanelService
) {
super(id, label, 'search-action codicon-chrome-restore');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How can I add a new codicon for expand-all ?
(codicon-chrome-restore is just a placeholder)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@misolori Can we get expand-all codicon? Probably just collapse-all but with a plus.

@JacksonKearl

Copy link
Copy Markdown
Contributor

Thanks for taking this on! As discussed in the original issue, I think it would be better if this were only visible when there are items to expand. The trick is the three states:

  • Everything collapsed => Show "Expand All"
  • Everything expanded => Show "Collapse All"
  • Otherwise => ???

As the default behavior for search is now to show all results expanded, I think we should lean towards providing Collapse functionality, and the "Expand All" action should only appear if everything is collapsed, with only the "Collapse All" being visible otherwise.

It might even make sense to only add a context menu entry for expanding all, as with #84087 the search results will almost always be fully expanded

cc roblourens for ideas

@roblourens

roblourens commented Nov 25, 2019

Copy link
Copy Markdown
Member

I agree, would rather not see two buttons at the same time. Just having a context menu option would be easier but it would be great to figure out the button behavior.

@okmttdhr

okmttdhr, tada (okmttdhr) commented Nov 26, 2019

Copy link
Copy Markdown
Contributor Author

Jackson Kearl (@JacksonKearl) roblourens
Thank you for your feedback!
I'll implement that button behavior!

@okmttdhr

Copy link
Copy Markdown
Contributor Author

Hello Jackson Kearl (@JacksonKearl) (CC: roblourens )
I updated this PR, could you please take a look at it?

@JacksonKearl

Jackson Kearl (JacksonKearl) commented Dec 13, 2019

Copy link
Copy Markdown
Contributor

Dec-12-2019 19-07-28

I'm noticing some unexpected behavior, where in a multiroot workspace if I expend the first node then run the action, everything expands. But if I expand any other node, the all collapse.

Not sure why my cursor isn't showing up in the gif, but I think it should still make sense.

Jackson Kearl added 2 commits December 12, 2019 20:46
@JacksonKearl

Copy link
Copy Markdown
Contributor

I fixed the issue with the first node being treated differently (the while-loop was skipping the first node, changed to do-while) and added a hook to update the action when the tree state is toggled outside of this action. Should be good to go once we get an icon. Thanks for your work on this!

@miguelsolorio

Copy link
Copy Markdown
Contributor

roblourens Jackson Kearl (@JacksonKearl) I updated Codicons in master to now include a expand-all icon:

image

@JacksonKearl
Jackson Kearl (JacksonKearl) merged commit c36e92d into microsoft:master Dec 13, 2019
@okmttdhr

Copy link
Copy Markdown
Contributor Author

I fixed the issue with the first node being treated differently (the while-loop was skipping the first node, changed to do-while) and added a hook to update the action when the tree state is toggled outside of this action. Should be good to go once we get an icon. Thanks for your work on this!

Thank you for your fixing and merging this PR 🙇✨☺️

@okmttdhr
okmttdhr, tada (okmttdhr) deleted the expand-button-in-search branch December 14, 2019 02:35
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Expand button in search

4 participants