Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat. add a menu item 'expand-select' to expand subtree in search view #206033

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

pagict
Copy link
Contributor

@pagict pagict commented Feb 23, 2024

Implementation of #206032

image

@RedCMD
Copy link
Contributor

RedCMD commented Feb 23, 2024

Middle click (scroll click/button 3) is often used for toggling between expanding and collapsing nested items
(just like folding ranges in the editor)

@pagict
Copy link
Contributor Author

pagict commented Feb 24, 2024

@microsoft-github-policy-service agree

Copy link
Contributor

@andreamah andreamah left a comment

Choose a reason for hiding this comment

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

Just a few small changes 👍

@andreamah
Copy link
Contributor

Middle click (scroll click/button 3) is often used for toggling between expanding and collapsing nested items (just like folding ranges in the editor)

I think the search view already supports this, where middle click expands one level. Is this what you meant, or is there still missing functionality?

@pagict
Copy link
Contributor Author

pagict commented Mar 1, 2024

Middle click (scroll click/button 3) is often used for toggling between expanding and collapsing nested items (just like folding ranges in the editor)

I think the search view already supports this, where middle click expands one level. Is this what you meant, or is there still missing functionality?

I meant for recursive expansion. I will update my code to make work, thx for reviewing.

@RedCMD
Copy link
Contributor

RedCMD commented Mar 1, 2024

I think the search view already supports this, where middle click expands one level. Is this what you meant, or is there still missing functionality?

yeah I meant for recursively nested items
The same as Folding ranges in the editor

@andreamah
Copy link
Contributor

I think the search view already supports this, where middle click expands one level. Is this what you meant, or is there still missing functionality?

yeah I meant for recursively nested items The same as Folding ranges in the editor

As in the middle click should recursively open nested items? Does this only happen in the editor with nested ranges, or does it happen in other tree views?

@RedCMD
Copy link
Contributor

RedCMD commented Mar 1, 2024

yes, I would like middle click to recursively open nested items.
I think it only works for Folding ranges atm
I don't think any other tree view supports it

I'm pretty sure there was something else that did support it
but I can't remember if it was even in VSCode or not

@pagict
Copy link
Contributor Author

pagict commented Mar 2, 2024

I tried and also checked the code, "middle mouse for recursively expansion" only works for foldings in the editor.

const recursive = e.event.middleButton || e.event.shiftKey;
if (recursive) {
for (const r of foldingModel.getRegionsInside(region)) {
if (r.isCollapsed === isCollapsed) {
toToggle.push(r);
}
}
}

@pagict pagict requested a review from andreamah March 4, 2024 03:18
@pagict pagict force-pushed the feat/search-expand-subtree branch from 580d02a to fe61dc6 Compare March 19, 2024 09:06
@andreamah andreamah added this to the April 2024 milestone Mar 22, 2024
@andreamah
Copy link
Contributor

Do we use the language "Expand Selected" anywhere else? Otherwise, I would prefer just "Expand" since it's implied that it's acting on selected elements.

@pagict
Copy link
Contributor Author

pagict commented Apr 1, 2024

Do we use the language "Expand Selected" anywhere else? Otherwise, I would prefer just "Expand" since it's implied that it's acting on selected elements.

no, the term "Expand Selected" does not in anywhere else. should I modify it as "Expand the Tree" or something similar, since this is quite a different behavior to what "Expand" implies.

@andreamah
Copy link
Contributor

image

The button that expands/collapses all (in the top toolbar) uses the term "Expand all" to expand all of the tree nodes, so I would expect that right-clicking and "expanding" a node would recursively open the node.

@pagict pagict force-pushed the feat/search-expand-subtree branch from fe61dc6 to 10dc3e8 Compare April 2, 2024 05:06
@pagict
Copy link
Contributor Author

pagict commented Apr 2, 2024

image

The button that expands/collapses all (in the top toolbar) uses the term "Expand all" to expand all of the tree nodes, so I would expect that right-clicking and "expanding" a node would recursively open the node.

update as "Expand All", please kindly review

@andreamah
Copy link
Contributor

I'm not too sure about "Expand All". Notice that the context menu has a "copy all" that actually addresses the entire search result, even if clicked on one node.
image

How about "Expand Recursively"?

@pagict
Copy link
Contributor Author

pagict commented Apr 3, 2024

I think they both are good, and how about "Expand from This"? or maybe should be more succinct.

I don't have any preferences on this

@andreamah
Copy link
Contributor

I think "Expand Recursively" would be the best from my perspective, since it seems less wordy in my opinion.

Copy link
Contributor

@andreamah andreamah left a comment

Choose a reason for hiding this comment

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

Thanks!

@andreamah andreamah merged commit 95885d7 into microsoft:main Apr 4, 2024
6 checks passed
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
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.

4 participants