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

#34685 Collapse search results one level at a time #37451

Merged
merged 2 commits into from Nov 4, 2017

Conversation

RMacfarlane
Copy link
Contributor

Fixes #34685

Changes the behavior of the "Collapse All" button to collapse results one level at a time instead of immediately to root.

* The returned promise returns a boolean for whether the elements were collapsed or not.
*/
collapseAll(elements?: any[], recursive?: boolean): WinJS.Promise;
collapseDeepestExpandedLevel(): WinJS.Promise;
Copy link
Member

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), []);
Copy link
Member

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

if (viewer.getHighlight()) {
return TPromise.as(null); // Global action disabled if user is in edit mode from another action
}

viewer.collapseAll();
viewer.collapseDeepestExpandedLevel();
Copy link
Member

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.

Copy link
Contributor Author

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();
Copy link
Member

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

@roblourens
Copy link
Member

Thanks!

@roblourens roblourens merged commit 829731d into microsoft:master Nov 4, 2017
@roblourens roblourens added this to the November 2017 milestone Nov 6, 2017
@RMacfarlane RMacfarlane deleted the ramacfar-collapse branch December 4, 2017 16:55
@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.

Search: collapse but keep roots expanded
2 participants