Added 'Collapse File Tree' feature #7026
Conversation
| @@ -245,6 +245,7 @@ define({ | |||
| "CMD_INSTALL_EXTENSION" : "Install Extension\u2026", | |||
| "CMD_EXTENSION_MANAGER" : "Extension Manager\u2026", | |||
| "CMD_FILE_REFRESH" : "Refresh File Tree", | |||
| "CMD_FILE_TREE_COLLAPSE" : "Collapse File Tree", | |||
There was a problem hiding this comment.
Maybe we should name it something like "Collapse Subtree" to make it clear it's not collapsing all nodes in the tree view.
|
Renamed @peterflynn |
|
Should we make this a "ctrl/cmd" + click command, similar to how is done in the Find in Files panel instead of adding a new command and menu item? |
|
That's also not a bad idea. Though I don't think many uses would be able to find that feature if they were looking for it. |
|
We can add a title tooltip as is done in the Find in Files panel. |
|
@TomMalbran I thought about suggesting that too -- especially since I dislike adding extra clutter to the context menu. But I think this is actually a different gesture: ctrl/cmd-click usually means collapse all, whereas this only collapses the specific subtree that you clicked on. It might be weird to overload the gesture for these two different things (though if there's precedent in for it in other editors, it'd be fine with me!). |
|
@larz0 Hi Larz may you please take a look at this and provide some guidance as how to proceed. |
|
We could add a new shortcut for the triangle, cmd/ctrl+alt click to expand/collapse subtrees (cmd+click for siblings). |
|
Initial UX review done. |
|
so -
|
|
Done, only thing missing is handling of the Mac's command key, I'm not sure if jQuery handles alt and cmd as the same key or not. |
|
@zaggino It doesn't. You can just use |
|
Thanks @TomMalbran, done. |
|
The shortcuts seem wrong.
|
|
I'm confused, I though Mac got Ctrl key but no Alt key... |
|
Oh there's a option key that servers as alt... my god. |
|
Ok - this should be fine now :) |
|
I don't think the "toggle siblings" feature should be done like this.
else {
// toggle siblings
var methodName = $node.is(".jstree-open") ? "close_node" : "open_node";
$node.parent().children("li").each(function () {
_projectTree.jstree(methodName, $(this));
});
return;
} |
|
Thanks @SAplayer, that was actually an issue I didn't realize. Fixed now. |
|
Cmd+alt(opt) click doesn't work for me for some reason. Can't wait for this PR, it's one of those small things that'll save so much time. 👍 |
|
Definitely works on my win, but I don't have cmd key to test it :-/ |
| // note: expanding using open_all is a bad idea due to poor performance | ||
| if ($node.is(".jstree-open")) { | ||
| _projectTree.jstree("close_all", $node); | ||
| return; |
There was a problem hiding this comment.
I think we should return true 'cause we handled the event. Same below.
There was a problem hiding this comment.
Is not returning to anything, it is just used so that it doesn't drop into the original behavior case.
There was a problem hiding this comment.
Original event is here: https://github.com/adobe/brackets/blob/master/src/thirdparty/jstree_pre1.0_fix_1/jquery.jstree.js#L357
Also not returning anything.
There was a problem hiding this comment.
this is the intended behaviour:
ctrl+click willl expand - collapse all siblings of the one that has been clicked
ctrl+alt+click will collapse just the one tree clicked, and all of its children (it doesn't work reverse for expand because it's slow)
point is if I have a tree open like
a
-b
-b1
-b2
-b3
-c
-c1
-cc1
-d
clicking ctrl+alt on a will cause all children to collapse, so if I open a again, I'll see only b,c,d and not b1,b2,b3,c1,cc1
There was a problem hiding this comment.
ahhh... I think I understand the problem now. You need to handle clicking on the label too not just the disclosure triangle. Alt+clicking on the label is handled by jstree and feels somewhat unpredictable.
|
I've just tested this on my Mac virtual machine and it works for me. |
|
UX review done. I'm going to pass this on and hopefully someone will have a Mac to test this on. |
|
FWIW, on Mac, it looks like Alt does this functionality in Finder. |
|
@JeffryBooher can you review again? Clicking on the labels is handled now. |
| if (event.altKey) { | ||
| // collapse subtree | ||
| // note: expanding using open_all is a bad idea due to poor performance | ||
| if ($node.is(".jstree-open")) { |
There was a problem hiding this comment.
I think the other part of my confusion was that cmd+alt doesn't open all. If there is a performance hit with open all then let's just open the node that was clicked on as if alt wasn't pressed.
|
@JeffryBooher fixed the latest comment |
|
Looks good. Merging. |
Added 'Collapse File Tree' feature
|
Ok, I think I understand the desired behavior now. I've confirmed the following:
Was a bit confused initially since (1) above doesn't change anything when expanding a folder. Tested on both Mac and Windows 7 (VM). |
|
@zaggino @JeffryBooher Fyi, looks like this introduced some JSLint nits in ProjectManager (via 151f3c1). I think I have to touch this file anyway in #7300, so I can fix it in my PR. |
|
Thanks @peterflynn ... I have to remember to disable JSHint because it overrides JSLint's checks ... |
contains the user prefs JSON files in its subtree) - ensure that FileSystem always knows we've read the JSON file before we try to write to it. We don't want to enable the 'blind' flag since it will mean we virtually always overwrite external changes to the prefs file, and we don't want to start using file watchers to observe external changes on the fly since we haven't yet deeply tested having multiple watch roots active at once. - Rename confusing "filename" vars in PreferencesBase - Fix JSLint errors in ProjectManager from #7026
When working with a file tree, I miss the ability to collapse whole section recursively - so when I open the top directory again later, not everything under it will also be expanded. This is a feature for it.
Sample:
I want to collapse everything under "test" folder and when I open "test" folder again, I don't want to see "node", "node-modules", "fs-extra" and "lib" expanded too.