Merge pull request #3398 from adobe/pflynn/fix-2761

Cleaner fix for #2761 (Project tree selection incorrect after opening file by means other than sidebar)

src/project/ProjectManager.js

@@ -195,17 +195,32 @@ define(function (require, exports, module) {
     function _documentSelectionFocusChange() {
         var curDoc = DocumentManager.getCurrentDocument();
         if (curDoc && _hasFileSelectionFocus()) {
-            $("#project-files-container li").is(function (index) {
-                var entry = $(this).data("entry");
-                if (entry && entry.fullPath === curDoc.file.fullPath && !_projectTree.jstree("is_selected", $(this))) {
-                    //we don't want to trigger another selection change event, so manually deselect
-                    //and select without sending out notifications
-                    _projectTree.jstree("deselect_all");
-                    _projectTree.jstree("select_node", $(this), false);
+            var nodeFound = $("#project-files-container li").is(function (index) {
+                var $treeNode = $(this),
+                    entry = $treeNode.data("entry");
+                if (entry && entry.fullPath === curDoc.file.fullPath) {
+                    if (!_projectTree.jstree("is_selected", $treeNode)) {
+                        if ($treeNode.parents(".jstree-closed").length) {
+                            //don't auto-expand tree to show file - but remember it if parent is manually expanded later
+                            _projectTree.jstree("deselect_all");
+                            _lastSelected = $treeNode;
+                        } else {
+                            //we don't want to trigger another selection change event, so manually deselect
+                            //and select without sending out notifications
+                            _projectTree.jstree("deselect_all");
+                            _projectTree.jstree("select_node", $treeNode, false);  // sets _lastSelected
+                        }
+                    }
                     return true;
                 }
                 return false;
             });
+            
+            // file is outside project subtree, or in a folder that's never been expanded yet
+            if (!nodeFound) {
+                _projectTree.jstree("deselect_all");
+                _lastSelected = null;
+            }
         } else if (_projectTree !== null) {
             _projectTree.jstree("deselect_all");
             _lastSelected = null;
@@ -915,11 +930,8 @@ define(function (require, exports, module) {
     function showInTree(entry) {
         return _findTreeNode(entry)
             .done(function ($node) {
-                _projectTree.jstree("deselect_node", _lastSelected);
-                _lastSelected = null;
-                _projectTree.jstree("select_node", $node);
-                _lastSelected = $node;
-                _redraw(true, true);
+                // jsTree will automatically expand parent nodes to ensure visible
+                _projectTree.jstree("select_node", $node, false);
             });
     }
     

src/search/FindInFiles.js

@@ -306,7 +306,6 @@ define(function (require, exports, module) {
                                     .done(function (doc) {
                                         // Opened document is now the current main editor
                                         EditorManager.getCurrentFullEditor().setSelection(match.start, match.end, true);
-                                        ProjectManager.showInTree(doc.file);
                                     });
                             });
                             resultsDisplayed++;

test/spec/ProjectManager-test.js

@@ -23,12 +23,14 @@
 
 
 /*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */
-/*global define: false, require: false, describe: false, it: false, expect: false, beforeEach: false, afterEach: false, waitsFor: false, runs: false */
+/*global $, define, require, describe, it, expect, beforeEach, afterEach, waitsFor, runs, waitsForDone */
 define(function (require, exports, module) {
     'use strict';
     
     // Load dependent modules
     var ProjectManager,     // Load from brackets.test
+        CommandManager,     // Load from brackets.test
+        Commands            = require("command/Commands"),
         SpecRunnerUtils     = require("spec/SpecRunnerUtils");
 
     describe("ProjectManager", function () {
@@ -36,13 +38,17 @@ define(function (require, exports, module) {
         this.category = "integration";
 
         var testPath = SpecRunnerUtils.getTestPath("/spec/ProjectManager-test-files"),
+            testWindow,
             brackets;
 
         beforeEach(function () {
-            SpecRunnerUtils.createTestWindowAndRun(this, function (testWindow) {
+            SpecRunnerUtils.createTestWindowAndRun(this, function (w) {
+                testWindow = w;
+                
                 // Load module instances from brackets.test
                 brackets = testWindow.brackets;
                 ProjectManager = testWindow.brackets.test.ProjectManager;
+                CommandManager = testWindow.brackets.test.CommandManager;
             });
         });
 
@@ -171,6 +177,133 @@ define(function (require, exports, module) {
             });
         });
         
+        describe("Selection indicator", function () {
+            
+            function expectSelected(fullPath) {
+                var $projectTreeItems = testWindow.$("#project-files-container > ul").children();
+                var $selectedItem = $projectTreeItems.find("a.jstree-clicked");
+                if (!fullPath) {
+                    expect($selectedItem.length).toBe(0);
+                } else {
+                    expect($selectedItem.length).toBe(1);
+                    expect($selectedItem.parent().data("entry").fullPath).toBe(fullPath);
+                }
+            }
+            
+            it("should deselect after opening file not rendered in tree", function () {
+                SpecRunnerUtils.loadProjectInTestWindow(testPath);
+                var promise,
+                    exposedFile   = testPath + "/file.js",
+                    unexposedFile = testPath + "/directory/file.js";
+                
+                runs(function () {
+                    promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: exposedFile });
+                    waitsForDone(promise);
+                });
+                runs(function () {
+                    expectSelected(exposedFile);
+                    
+                    promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: unexposedFile });
+                    waitsForDone(promise);
+                });
+                runs(function () {
+                    expectSelected(null);
+                });
+            });
+            
+            
+            function findExtantNode(fullPath) {
+                var $treeItems = testWindow.$("#project-files-container li"),
+                    $result;
+                $treeItems.is(function () {
+                    var $treeNode = testWindow.$(this),
+                        entry = $treeNode.data("entry");
+                    if (entry && entry.fullPath === fullPath) {
+                        $result = $treeNode;
+                        return true;
+                    }
+                    return false;
+                });
+                return $result;
+            }
+            function toggleFolder(fullPath, open) {
+                var $treeNode = findExtantNode(fullPath);
+                
+                var expectedClass = open ? "jstree-open" : "jstree-closed";
+                expect($treeNode.hasClass(expectedClass)).toBe(false);
+                
+                $treeNode.children("a").click();
+                
+                // if a folder has never been expanded before, this will be async
+                waitsFor(function () {
+                    return $treeNode.hasClass(expectedClass);
+                }, (open ? "Open" : "Close") + " tree node", 1000);
+            }
+            
+            it("should reselect previously selected file when made visible again", function () {
+                SpecRunnerUtils.loadProjectInTestWindow(testPath);
+                var promise,
+                    initialFile  = testPath + "/file.js",
+                    folder       = testPath + "/directory/",
+                    fileInFolder = testPath + "/directory/file.js";
+                
+                runs(function () {
+                    promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: initialFile });
+                    waitsForDone(promise);
+                });
+                runs(function () {
+                    expectSelected(initialFile);
+                    toggleFolder(folder, true);     // open folder
+                });
+                runs(function () {                  // open file in folder
+                    promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: fileInFolder });
+                    waitsForDone(promise);
+                });
+                runs(function () {
+                    expectSelected(fileInFolder);
+                    toggleFolder(folder, false);    // close folder
+                });
+                runs(function () {
+                    expectSelected(folder);
+                    toggleFolder(folder, true);     // open folder again
+                });
+                runs(function () {
+                    expectSelected(fileInFolder);
+                });
+            });
+            
+            it("should deselect after opening file hidden in tree, but select when made visible again", function () {
+                SpecRunnerUtils.loadProjectInTestWindow(testPath);
+                var promise,
+                    initialFile  = testPath + "/file.js",
+                    folder       = testPath + "/directory/",
+                    fileInFolder = testPath + "/directory/file.js";
+                
+                runs(function () {
+                    promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: initialFile });
+                    waitsForDone(promise);
+                });
+                runs(function () {
+                    expectSelected(initialFile);
+                    toggleFolder(folder, true);     // open folder
+                });
+                runs(function () {
+                    toggleFolder(folder, false);    // close folder
+                });
+                runs(function () {                  // open file in folder
+                    promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: fileInFolder });
+                    waitsForDone(promise);
+                });
+                runs(function () {
+                    expectSelected(null);
+                    toggleFolder(folder, true);     // open folder again
+                });
+                runs(function () {
+                    expectSelected(fileInFolder);
+                });
+            });
+        });
+        
         describe("File Display", function () {
             it("should not show useless directory entries", function () {
                 var shouldShow = ProjectManager.shouldShow;