Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Commit 0e9376a

Browse files
peterflynnpeterflynn
committed
Cleaner fix for #2761 (Project tree selection incorrect after opening file
by means other than sidebar) - Clear selection in project tree if current document not exposed in tree anywhere. Backs out pull #2869, which made the behavior of Find in Files inconsistent with other means of opening and didn't fix other cases of the bug. Adds unit tests for several of these edge cases.
1 parent 6f2dabd commit 0e9376a

File tree

3 files changed

+159
-15
lines changed

3 files changed

+159
-15
lines changed

‎src/project/ProjectManager.js

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -195,17 +195,32 @@ define(function (require, exports, module) {
195195
function _documentSelectionFocusChange() {
196196
var curDoc = DocumentManager.getCurrentDocument();
197197
if (curDoc && _hasFileSelectionFocus()) {
198-
$("#project-files-container li").is(function (index) {
199-
var entry = $(this).data("entry");
200-
if (entry && entry.fullPath === curDoc.file.fullPath && !_projectTree.jstree("is_selected", $(this))) {
201-
//we don't want to trigger another selection change event, so manually deselect
202-
//and select without sending out notifications
203-
_projectTree.jstree("deselect_all");
204-
_projectTree.jstree("select_node", $(this), false);
198+
var nodeFound = $("#project-files-container li").is(function (index) {
199+
var $treeNode = $(this),
200+
entry = $treeNode.data("entry");
201+
if (entry && entry.fullPath === curDoc.file.fullPath) {
202+
if (!_projectTree.jstree("is_selected", $treeNode)) {
203+
if ($treeNode.parents(".jstree-closed").length) {
204+
//don't auto-expand tree to show file - but remember it if parent is manually expanded later
205+
_projectTree.jstree("deselect_all");
206+
_lastSelected = $treeNode;
207+
} else {
208+
//we don't want to trigger another selection change event, so manually deselect
209+
//and select without sending out notifications
210+
_projectTree.jstree("deselect_all");
211+
_projectTree.jstree("select_node", $treeNode, false); // sets _lastSelected
212+
}
213+
}
205214
return true;
206215
}
207216
return false;
208217
});
218+
219+
// file is outside project subtree, or in a folder that's never been expanded yet
220+
if (!nodeFound) {
221+
_projectTree.jstree("deselect_all");
222+
_lastSelected = null;
223+
}
209224
} else if (_projectTree !== null) {
210225
_projectTree.jstree("deselect_all");
211226
_lastSelected = null;
@@ -915,11 +930,8 @@ define(function (require, exports, module) {
915930
function showInTree(entry) {
916931
return _findTreeNode(entry)
917932
.done(function ($node) {
918-
_projectTree.jstree("deselect_node", _lastSelected);
919-
_lastSelected = null;
920-
_projectTree.jstree("select_node", $node);
921-
_lastSelected = $node;
922-
_redraw(true, true);
933+
// jsTree will automatically expand parent nodes to ensure visible
934+
_projectTree.jstree("select_node", $node, false);
923935
});
924936
}
925937

‎src/search/FindInFiles.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,6 @@ define(function (require, exports, module) {
306306
.done(function (doc) {
307307
// Opened document is now the current main editor
308308
EditorManager.getCurrentFullEditor().setSelection(match.start, match.end, true);
309-
ProjectManager.showInTree(doc.file);
310309
});
311310
});
312311
resultsDisplayed++;

‎test/spec/ProjectManager-test.js

Lines changed: 135 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,32 @@
2323

2424

2525
/*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */
26-
/*global define: false, require: false, describe: false, it: false, expect: false, beforeEach: false, afterEach: false, waitsFor: false, runs: false */
26+
/*global $, define, require, describe, it, expect, beforeEach, afterEach, waitsFor, runs, waitsForDone */
2727
define(function (require, exports, module) {
2828
'use strict';
2929

3030
// Load dependent modules
3131
var ProjectManager, // Load from brackets.test
32+
CommandManager, // Load from brackets.test
33+
Commands = require("command/Commands"),
3234
SpecRunnerUtils = require("spec/SpecRunnerUtils");
3335

3436
describe("ProjectManager", function () {
3537

3638
this.category = "integration";
3739

3840
var testPath = SpecRunnerUtils.getTestPath("/spec/ProjectManager-test-files"),
41+
testWindow,
3942
brackets;
4043

4144
beforeEach(function () {
42-
SpecRunnerUtils.createTestWindowAndRun(this, function (testWindow) {
45+
SpecRunnerUtils.createTestWindowAndRun(this, function (w) {
46+
testWindow = w;
47+
4348
// Load module instances from brackets.test
4449
brackets = testWindow.brackets;
4550
ProjectManager = testWindow.brackets.test.ProjectManager;
51+
CommandManager = testWindow.brackets.test.CommandManager;
4652
});
4753
});
4854

@@ -171,6 +177,133 @@ define(function (require, exports, module) {
171177
});
172178
});
173179

180+
describe("Selection indicator", function () {
181+
182+
function expectSelected(fullPath) {
183+
var $projectTreeItems = testWindow.$("#project-files-container > ul").children();
184+
var $selectedItem = $projectTreeItems.find("a.jstree-clicked");
185+
if (!fullPath) {
186+
expect($selectedItem.length).toBe(0);
187+
} else {
188+
expect($selectedItem.length).toBe(1);
189+
expect($selectedItem.parent().data("entry").fullPath).toBe(fullPath);
190+
}
191+
}
192+
193+
it("should deselect after opening file not rendered in tree", function () {
194+
SpecRunnerUtils.loadProjectInTestWindow(testPath);
195+
var promise,
196+
exposedFile = testPath + "/file.js",
197+
unexposedFile = testPath + "/directory/file.js";
198+
199+
runs(function () {
200+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: exposedFile });
201+
waitsForDone(promise);
202+
});
203+
runs(function () {
204+
expectSelected(exposedFile);
205+
206+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: unexposedFile });
207+
waitsForDone(promise);
208+
});
209+
runs(function () {
210+
expectSelected(null);
211+
});
212+
});
213+
214+
215+
function findExtantNode(fullPath) {
216+
var $treeItems = testWindow.$("#project-files-container li"),
217+
$result;
218+
$treeItems.is(function () {
219+
var $treeNode = testWindow.$(this),
220+
entry = $treeNode.data("entry");
221+
if (entry && entry.fullPath === fullPath) {
222+
$result = $treeNode;
223+
return true;
224+
}
225+
return false;
226+
});
227+
return $result;
228+
}
229+
function toggleFolder(fullPath, open) {
230+
var $treeNode = findExtantNode(fullPath);
231+
232+
var expectedClass = open ? "jstree-open" : "jstree-closed";
233+
expect($treeNode.hasClass(expectedClass)).toBe(false);
234+
235+
$treeNode.children("a").click();
236+
237+
// if a folder has never been expanded before, this will be async
238+
waitsFor(function () {
239+
return $treeNode.hasClass(expectedClass);
240+
}, (open ? "Open" : "Close") + " tree node", 1000);
241+
}
242+
243+
it("should reselect previously selected file when made visible again", function () {
244+
SpecRunnerUtils.loadProjectInTestWindow(testPath);
245+
var promise,
246+
initialFile = testPath + "/file.js",
247+
folder = testPath + "/directory/",
248+
fileInFolder = testPath + "/directory/file.js";
249+
250+
runs(function () {
251+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: initialFile });
252+
waitsForDone(promise);
253+
});
254+
runs(function () {
255+
expectSelected(initialFile);
256+
toggleFolder(folder, true); // open folder
257+
});
258+
runs(function () { // open file in folder
259+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: fileInFolder });
260+
waitsForDone(promise);
261+
});
262+
runs(function () {
263+
expectSelected(fileInFolder);
264+
toggleFolder(folder, false); // close folder
265+
});
266+
runs(function () {
267+
expectSelected(folder);
268+
toggleFolder(folder, true); // open folder again
269+
});
270+
runs(function () {
271+
expectSelected(fileInFolder);
272+
});
273+
});
274+
275+
it("should deselect after opening file hidden in tree, but select when made visible again", function () {
276+
SpecRunnerUtils.loadProjectInTestWindow(testPath);
277+
var promise,
278+
initialFile = testPath + "/file.js",
279+
folder = testPath + "/directory/",
280+
fileInFolder = testPath + "/directory/file.js";
281+
282+
runs(function () {
283+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: initialFile });
284+
waitsForDone(promise);
285+
});
286+
runs(function () {
287+
expectSelected(initialFile);
288+
toggleFolder(folder, true); // open folder
289+
});
290+
runs(function () {
291+
toggleFolder(folder, false); // close folder
292+
});
293+
runs(function () { // open file in folder
294+
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: fileInFolder });
295+
waitsForDone(promise);
296+
});
297+
runs(function () {
298+
expectSelected(null);
299+
toggleFolder(folder, true); // open folder again
300+
});
301+
runs(function () {
302+
expectSelected(fileInFolder);
303+
});
304+
});
305+
});
306+
174307
describe("File Display", function () {
175308
it("should not show useless directory entries", function () {
176309
var shouldShow = ProjectManager.shouldShow;

0 commit comments

Comments
 (0)