-
Notifications
You must be signed in to change notification settings - Fork 27.9k
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
#34685 Collapse search results one level at a time #37451
Conversation
* The returned promise returns a boolean for whether the elements were collapsed or not. | ||
*/ | ||
collapseAll(elements?: any[], recursive?: boolean): WinJS.Promise; | ||
collapseDeepestExpandedLevel(): WinJS.Promise; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave collapseAll
in place, having collapseDeepestExpandedLevel
as a new method
for (var i = 0; i < levelToCollapse; i++) { | ||
items = items | ||
.map(node => node.getChildren()) | ||
.reduce((prev, current) => prev.concat(current), []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a helper for this, arrays.flatten
src/vs/workbench/browser/viewlet.ts
Outdated
if (viewer.getHighlight()) { | ||
return TPromise.as(null); // Global action disabled if user is in edit mode from another action | ||
} | ||
|
||
viewer.collapseAll(); | ||
viewer.collapseDeepestExpandedLevel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have two actions that do the same thing. Let's replace one with an action that calls collapseDeepestExpandedLevel
, and leave the other. And let's be explicit in which trees were are changing the behavior of. We should change it for search results in this PR and maybe the file explorer. If we want other trees to have this behavior, we can change that later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've scoped it to just search results for now
@@ -427,7 +427,7 @@ export class CollapseAllAction extends Action { | |||
return TPromise.as(null); // Global action disabled if user is in edit mode from another action | |||
} | |||
|
|||
this.viewer.collapseAll(); | |||
this.viewer.collapseDeepestExpandedLevel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This action shouldn't be called CollapseAllAction anymore
Thanks! |
Fixes #34685
Changes the behavior of the "Collapse All" button to collapse results one level at a time instead of immediately to root.