Merge pull request #9507 from adobe/jeff/wsv-perf-mods

redmunds · redmunds · commit 5a05eaa0272f · 2014-10-09T17:37:08.000-07:00
Working Performance Set Improvements
diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js
@@ -470,13 +470,15 @@ define(function (require, exports, module) {
      * Semi-private: should only be called within this module or by Document.
      * @param {!Document} document  Document whose main/full Editor to create
      * @param {!Pane} pane  Pane in which the editor will be hosted
+     * @return {!Editor}
      */
     function _createFullEditorForDocument(document, pane) {
         // Create editor; make it initially invisible
         var editor = _createEditorForDocument(document, true, pane.$content);
         editor.setVisible(false);
         pane.addView(editor);
         $(exports).triggerHandler("_fullEditorCreatedForDocument", [document, editor, pane.id]);
+        return editor;
     }
  
     
@@ -535,32 +537,31 @@ define(function (require, exports, module) {
             editor = document._masterEditor;
         
         if (!editor) {
-            createdNewEditor = true;
-
             // Performance (see #4757) Chrome wastes time messing with selection
             // that will just be changed at end, so clear it for now
             if (window.getSelection && window.getSelection().empty) {  // Chrome
                 window.getSelection().empty();
             }
             
             // Editor doesn't exist: populate a new Editor with the text
-            _createFullEditorForDocument(document, pane);
-        } else if (editor.$el.parent() !== pane.$el) {
+            editor = _createFullEditorForDocument(document, pane);
+            createdNewEditor = true;
+        } else if (editor.$el.parent()[0] !== pane.$content[0]) {
             // editor does exist but is not a child of the pane so add it to the 
             //  pane (which will switch the view's container as well)
             pane.addView(editor);
         }
 
         // show the view
-        pane.showView(document._masterEditor);
+        pane.showView(editor);
 
         if (MainViewManager.getActivePaneId() === pane.id) {
             // give it focus
-            document._masterEditor.focus();
+            editor.focus();
         }
 
         if (createdNewEditor) {
-            _restoreEditorViewState(document._masterEditor);
+            _restoreEditorViewState(editor);
         }
     }
 
diff --git a/src/project/FileViewController.js b/src/project/FileViewController.js
@@ -129,8 +129,10 @@ define(function (require, exports, module) {
             return;
         }
 
-        _fileSelectionFocus = fileSelectionFocus;
-        $(exports).triggerHandler("fileViewFocusChange");
+        if (_fileSelectionFocus !== fileSelectionFocus) {
+            _fileSelectionFocus = fileSelectionFocus;
+            $(exports).triggerHandler("fileViewFocusChange");
+        }
     }
 
     /** 
diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js
@@ -34,7 +34,8 @@ define(function (require, exports, module) {
     "use strict";
     
     // Load dependent modules
-    var DocumentManager       = require("document/DocumentManager"),
+    var AppInit               = require("utils/AppInit"),
+        DocumentManager       = require("document/DocumentManager"),
         MainViewManager       = require("view/MainViewManager"),
         CommandManager        = require("command/CommandManager"),
         Commands              = require("command/Commands"),
@@ -69,6 +70,12 @@ define(function (require, exports, module) {
     var _classProviders = [];
         
 
+    /**
+     * #working-set-list-container 
+     * @type {jQuery}
+     */
+    var $workingFilesContainer;
+    
     /**
      * Constants for event.which values
      * @enum {number}
@@ -97,6 +104,12 @@ define(function (require, exports, module) {
         BELOWVIEW  = "belowview",
         ABOVEVIEW  = "aboveview";
     
+    /**
+     * Drag an item has to move 3px before dragging starts
+     * @constant
+     */
+    var _DRAG_MOVE_DETECTION_START = 3;
+    
     /**
      * Refreshes all Pane View List Views
      */
@@ -129,7 +142,6 @@ define(function (require, exports, module) {
      */
     function _updateListItemSelection(listItem, selectedFile) {
         var shouldBeSelected = (selectedFile && $(listItem).data(_FILE_KEY).fullPath === selectedFile.fullPath);
-        
         ViewUtils.toggleClass($(listItem), "selected", shouldBeSelected);
     }
 
@@ -263,19 +275,43 @@ define(function (require, exports, module) {
                 dragged = false,
                 startPageY = e.pageY,
                 lastPageY = startPageY,
-                itemHeight = $el.height(),
+                lastHit = { where: NOMANSLAND },
                 tryClosing = $(e.target).hasClass("can-close"),
-                offset = $el.offset(),
-                $copy = $el.clone(),
-                $ghost = $("<div class='open-files-container wsv-drag-ghost' style='overflow: hidden; display: inline-block;'>").append($("<ul>").append($copy).css("padding", "0")),
-                sourceView = _viewFromEl($el),
                 currentFile = MainViewManager.getCurrentlyViewedFile(),
                 activePaneId = MainViewManager.getActivePaneId(),
                 activeView = _views[activePaneId],
-                draggingCurrentFile = ($el.hasClass("selected") && sourceView.paneId === activePaneId),
-                startingIndex = MainViewManager.findInWorkingSet(sourceView.paneId, sourceFile.fullPath),
+                sourceView = _viewFromEl($el),
                 currentView = sourceView,
-                lastHit = { where: NOMANSLAND };
+                startingIndex = $el.index(),
+                itemHeight,
+                offset,
+                $copy,
+                $ghost,
+                draggingCurrentFile;
+            
+            function initDragging() {
+                itemHeight = $el.height();
+                offset = $el.offset();
+                $copy = $el.clone();
+                $ghost = $("<div class='open-files-container wsv-drag-ghost' style='overflow: hidden; display: inline-block;'>").append($("<ul>").append($copy).css("padding", "0"));
+                draggingCurrentFile = ($el.hasClass("selected") && sourceView.paneId === activePaneId);
+                
+                // setup our ghost element as position absolute
+                //  so we can put it wherever we want to while dragging
+                if (draggingCurrentFile && _hasSelectionFocus()) {
+                    $ghost.addClass("dragging-current-file");
+                }
+
+                $ghost.css({
+                    top: offset.top,
+                    left: offset.left,
+                    width: $el.width() + 8
+                });
+
+                // this will give the element the appearence that it's ghosted if the user
+                //  drags the element out of the view and goes off into no mans land
+                $ghost.appendTo($("body"));
+            }
             
             // Switches the view context to match the hit context
             function updateContext(hit) {
@@ -294,7 +330,6 @@ define(function (require, exports, module) {
                     hasScroller = false,
                     onTopScroller = false,
                     onBottomScroller = false,
-                    $workingFilesContainer =  $("#working-set-list-container"),
                     $container,
                     $hit,
                     $item,
@@ -564,7 +599,7 @@ define(function (require, exports, module) {
                 // The drag function
                 function drag(e) {
                     if (!dragged) {
-                        
+                        initDragging();
                         // sort redraw and scroll shadows
                         //  cause problems during drag so disable them
                         _suppressSortRedrawForAllViews(true);
@@ -576,7 +611,7 @@ define(function (require, exports, module) {
                         _deactivateAllViews(true);
                         
                         // add a "dragging" class to the outer container
-                        $("#working-set-list-container").addClass("dragging");
+                        $workingFilesContainer.addClass("dragging");
                         
                         // add a class to the element we're dragging if 
                         //  it's the currently selected file so that we 
@@ -654,13 +689,15 @@ define(function (require, exports, module) {
                     }
                 }
 
-                // move the drag affordance
-                $ghost.css("top", $ghost.offset().top + (e.pageY - lastPageY));
-
+                // Reposition the drag affordance if we've started dragging
+                if ($ghost) {
+                    $ghost.css("top", $ghost.offset().top + (e.pageY - lastPageY));
+                }
+                
                 // if we have't started dragging yet then we wait until
                 //  the mouse has moved 3 pixels before we start dragging
                 //  to avoid the item moving when clicked or double clicked
-                if (dragged || Math.abs(e.pageY - startPageY) > 3) {
+                if (dragged || Math.abs(e.pageY - startPageY) > _DRAG_MOVE_DETECTION_START) {
                     drag(e);
                 }
 
@@ -681,19 +718,21 @@ define(function (require, exports, module) {
             
             // Close down the drag operation
             function preDropCleanup() {
-                $("#working-set-list-container").removeClass("dragging");
-                $("#working-set-list-container .drag-show-as-selected").removeClass("drag-show-as-selected");
-                endScroll($el);
-                // re-activate the views (adds the "active" class to the view that was previously active)
-                _deactivateAllViews(false);
-                // turn scroll wheel back on
                 window.onmousewheel = window.document.onmousewheel = null;
                 $(window).off(".wsvdragging");
-                $ghost.remove();
-                $el.css("opacity", "");
+                if (dragged) {
+                    $workingFilesContainer.removeClass("dragging");
+                    $workingFilesContainer.find(".drag-show-as-selected").removeClass("drag-show-as-selected");
+                    endScroll($el);
+                    // re-activate the views (adds the "active" class to the view that was previously active)
+                    _deactivateAllViews(false);
+                    // turn scroll wheel back on
+                    $ghost.remove();
+                    $el.css("opacity", "");
                 
-                if (dragged && $el.next().length === 0) {
-                    scrollCurrentViewToBottom();
+                    if ($el.next().length === 0) {
+                        scrollCurrentViewToBottom();
+                    }
                 }
             }
         
@@ -800,21 +839,7 @@ define(function (require, exports, module) {
                 return;
             }
             
-            // setup our ghost element as position absolute
-            //  so we can put it wherever we want to while dragging
-            if (draggingCurrentFile && _hasSelectionFocus()) {
-                $ghost.addClass("dragging-current-file");
-            }
 
-            $ghost.css({
-                top: offset.top,
-                left: offset.left,
-                width: $el.width() + 8
-            });
-            
-            // this will give the element the appearence that it's ghosted if the user
-            //  drags the element out of the view and goes off into no mans land
-            $ghost.appendTo($("body"));
             
             e.stopPropagation();
         });
@@ -1435,6 +1460,11 @@ define(function (require, exports, module) {
         refresh(true);
     }
     
+    AppInit.htmlReady(function () {
+        $workingFilesContainer =  $("#working-set-list-container");
+    });
+    
+    
     // Public API
     exports.createWorkingSetViewForPane   = createWorkingSetViewForPane;
     exports.refresh                       = refresh;
diff --git a/src/view/MainViewManager.js b/src/view/MainViewManager.js
@@ -370,9 +370,8 @@ define(function (require, exports, module) {
                                                             oldPaneId]);
             
             _makePaneMostRecent(_activePaneId);
+            focusActivePane();
         }
-        
-        focusActivePane();
     }
     
     /**
@@ -1128,24 +1127,27 @@ define(function (require, exports, module) {
      * Do not use this API unless you have a document object without a file object
      * @param {!string} paneId - id of the pane in which to open the document
      * @param {!Document} doc - document to edit
-     * @param {{noPaneActivate:boolean}=} optionsIn - options
+     * @param {{noPaneActivate:boolean=, noPaneRedundancyCheck:boolean=}=} optionsIn - options
      * @private
      */
     function _edit(paneId, doc, optionsIn) {
-        var currentPaneId = _getPaneIdForPath(doc.file.fullPath),
-            options = optionsIn || {};
-
-            
+        var options = optionsIn || {},
+            currentPaneId;
+        
+        if (options.noPaneRedundancyCheck) {
+            // This flag is for internal use only to improve performance
+            // Don't check for the file to have been opened in another pane pane which could be time
+            //  consuming.  should only be used when passing an actual paneId (not a special paneId)
+            //  and the caller has already done a redundancy check.  
+            currentPaneId = _getPaneIdForPath(doc.file.fullPath);
+        }
+   
         if (currentPaneId) {
-            // If the doc is open in another pane
-            //  then switch to that pane and call open document
-            //  which will really just show the view as it has always done
-            //  we could just do pane.showView(doc._masterEditor) in that
-            //  case but Editor Manager may do some state syncing 
+            // If the doc is open in another pane then switch to that pane and call open document
+            //  which will really just show the view as it has always done we could just 
+            //  do pane.showView(doc._masterEditor) in that case but Editor Manager may do some 
+            //  state syncing 
             paneId = currentPaneId;
-            if (!options.noPaneActivate) {
-                setActivePaneId(paneId);
-            }
         }
         
         var pane = _getPane(paneId);
@@ -1159,21 +1161,31 @@ define(function (require, exports, module) {
         // open document will show the editor if there is one already
         EditorManager.openDocument(doc, pane);
         _makeFileMostRecent(paneId, doc.file);
+
+        if (!options.noPaneActivate) {
+            setActivePaneId(paneId);
+        }
     }
     
     /**
      * Opens a file in the specified pane this can be used to open a file with a custom viewer
      * or a document for editing.  If it's a document for editing, edit is called on the document 
      * @param {!string} paneId - id of the pane in which to open the document
      * @param {!File} file - file to open
-     * @param {{noPaneActivate:boolean}=} optionsIn - options
+     * @param {{noPaneActivate:boolean=, noPaneRedundancyCheck:boolean=}=} optionsIn - options
      * @return {jQuery.Promise}  promise that resolves to a File object or 
      *                           rejects with a File error or string
      */
     function _open(paneId, file, optionsIn) {
         var result = new $.Deferred(),
             options = optionsIn || {};
         
+        function doPostOpenActivation() {
+            if (!options.noPaneActivate) {
+                setActivePaneId(paneId);
+            }
+        }
+        
         if (!file || !_getPane(paneId)) {
             return result.reject("bad argument").promise();
         }
@@ -1203,9 +1215,6 @@ define(function (require, exports, module) {
             //  we could just do pane.showView(doc._masterEditor) in that
             //  case but Editor Manager may do some state syncing             
             paneId = currentPaneId;
-            if (!options.noPaneActivate) {
-                setActivePaneId(paneId);
-            }
         }
         
         // See if there is already a view for the file
@@ -1227,6 +1236,7 @@ define(function (require, exports, module) {
                             if (!ProjectManager.isWithinProject(file.fullPath)) {
                                 addToWorkingSet(paneId, file);
                             }
+                            doPostOpenActivation();
                             result.resolve(file);
                         })
                         .fail(function (fileError) {
@@ -1239,14 +1249,18 @@ define(function (require, exports, module) {
         } else {
             DocumentManager.getDocumentForPath(file.fullPath)
                 .done(function (doc) {
-                    _edit(paneId, doc, options);
+                    _edit(paneId, doc, $.extend({}, options, {
+                        noPaneActivate: true,
+                        noPaneRedundancyCheck: true
+                    }));
+                    doPostOpenActivation();
                     result.resolve(doc.file);
                 })
                 .fail(function (fileError) {
                     result.reject(fileError);
                 });
         }
-        
+
         return result;
     }
     
diff --git a/src/view/Pane.js b/src/view/Pane.js

Original file line number	Diff line number	Diff line change
`@@ -129,8 +129,10 @@ define(function (require, exports, module) {`
`129`	`129`	`return;`
`130`	`130`	`}`
`131`	`131`
`132`		`- _fileSelectionFocus = fileSelectionFocus;`
`133`		`- $(exports).triggerHandler("fileViewFocusChange");`
	`132`	`+ if (_fileSelectionFocus !== fileSelectionFocus) {`
	`133`	`+ _fileSelectionFocus = fileSelectionFocus;`
	`134`	`+ $(exports).triggerHandler("fileViewFocusChange");`
	`135`	`+ }`
`134`	`136`	`}`
`135`	`137`
`136`	`138`	`/**`