From 630f3026b32a2f08e388097dffad4696ae1e7195 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 7 Jun 2015 00:48:05 +0200 Subject: [PATCH 1/7] Fix several typos in MainViewManager docs --- src/view/MainViewManager.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/view/MainViewManager.js b/src/view/MainViewManager.js index 3f4c19068ca..e8d1f83a19f 100644 --- a/src/view/MainViewManager.js +++ b/src/view/MainViewManager.js @@ -25,12 +25,12 @@ /*global define, $ */ /** - * MainViewManager Manages the arrangement of all open panes as well as provides the controller + * MainViewManager manages the arrangement of all open panes as well as provides the controller * logic behind all views in the MainView (e.g. ensuring that a file doesn't appear in 2 lists) * * Each pane contains one or more views wich are created by a view factory and inserted into a pane list. - * There may be several panes managed by the MainViewManager with each pane containing a list of views. - * The panes are always visible and the layout is determined by the MainViewManager and the user. + * There may be several panes managed by the MainViewManager with each pane containing a list of views. + * The panes are always visible and the layout is determined by the MainViewManager and the user. * * Currently we support only 2 panes. * @@ -354,7 +354,7 @@ define(function (require, exports, module) { /** * Makes the Pane's current file the most recent - * @param {!string} paneId - id of the pane to mae th file most recent or ACTIVE_PANE + * @param {!string} paneId - id of the pane to make the file most recent, or ACTIVE_PANE * @param {!File} file - File object to make most recent * @private */ @@ -367,7 +367,7 @@ define(function (require, exports, module) { } /** - * Switch active pane to the specified pane Id (or ACTIVE_PANE/ALL_PANES, in which case this + * Switch active pane to the specified pane id (or ACTIVE_PANE/ALL_PANES, in which case this * call does nothing). * @param {!string} paneId - the id of the pane to activate */ @@ -515,7 +515,7 @@ define(function (require, exports, module) { /** - * Retrieves the WorkingSet for the given PaneId not including temporary views + * Retrieves the WorkingSet for the given paneId not including temporary views * @param {!string} paneId - id of the pane in which to get the view list, ALL_PANES or ACTIVE_PANE * @return {Array.} */ @@ -532,7 +532,7 @@ define(function (require, exports, module) { /** - * Retrieves the list of all open files inlcuding temporary views + * Retrieves the list of all open files including temporary views * @return {array.} the list of all open files in all open panes */ function getAllOpenFiles() { @@ -916,8 +916,8 @@ define(function (require, exports, module) { /** * Get the next or previous file in the MRU list. * @param {!number} direction - Must be 1 or -1 to traverse forward or backward - * @return {?{file:File, paneId:string}} The File object of the next item in the travesal order or null if there aren't any files to traverse. - * may return current file if there are no other files to traverse. + * @return {?{file:File, paneId:string}} The File object of the next item in the traversal order or null if there aren't any files to traverse. + * May return current file if there are no other files to traverse. */ function traverseToNextViewByMRU(direction) { var file = getCurrentlyViewedFile(), From f29454e657d3a1cb2ceaa2caaa6a4324affbba5e Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 7 Jun 2015 00:50:03 +0200 Subject: [PATCH 2/7] Use Ctrl+PageUp/PageDown to traverse files in list order --- src/base-config/keyboard.json | 12 ++++- src/command/Commands.js | 2 + src/command/DefaultMenus.js | 2 + src/document/DocumentCommandHandlers.js | 72 ++++++++++++++++++++++--- src/nls/root/strings.js | 2 + 5 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/base-config/keyboard.json b/src/base-config/keyboard.json index 17c2297fe65..174f0a3a369 100644 --- a/src/base-config/keyboard.json +++ b/src/base-config/keyboard.json @@ -1,4 +1,4 @@ -{ +{ "file.newDoc": [ "Ctrl-N" ], @@ -319,6 +319,16 @@ "platform": "mac" } ], + "navigate.nextDocListOrder": [ + { + "key": "Ctrl-PageDown" + } + ], + "navigate.prevDocListOrder": [ + { + "key": "Ctrl-PageUp" + } + ], "navigate.toggleQuickEdit": [ "Ctrl-E" ], diff --git a/src/command/Commands.js b/src/command/Commands.js index 34d86f9e360..7117334bb09 100644 --- a/src/command/Commands.js +++ b/src/command/Commands.js @@ -121,6 +121,8 @@ define(function (require, exports, module) { // NAVIGATE exports.NAVIGATE_NEXT_DOC = "navigate.nextDoc"; // DocumentCommandHandlers.js handleGoNextDoc() exports.NAVIGATE_PREV_DOC = "navigate.prevDoc"; // DocumentCommandHandlers.js handleGoPrevDoc() + exports.NAVIGATE_NEXT_DOC_LIST_ORDER = "navigate.nextDocListOrder"; // DocumentCommandHandlers.js handleGoNextDocListOrder() + exports.NAVIGATE_PREV_DOC_LIST_ORDER = "navigate.prevDocListOrder"; // DocumentCommandHandlers.js handleGoPrevDocListOrder() exports.NAVIGATE_SHOW_IN_FILE_TREE = "navigate.showInFileTree"; // DocumentCommandHandlers.js handleShowInTree() exports.NAVIGATE_SHOW_IN_OS = "navigate.showInOS"; // DocumentCommandHandlers.js handleShowInOS() exports.NAVIGATE_QUICK_OPEN = "navigate.quickOpen"; // QuickOpen.js doFileSearch() diff --git a/src/command/DefaultMenus.js b/src/command/DefaultMenus.js index ad90ac14976..9528f0609c1 100644 --- a/src/command/DefaultMenus.js +++ b/src/command/DefaultMenus.js @@ -150,6 +150,8 @@ define(function (require, exports, module) { menu.addMenuDivider(); menu.addMenuItem(Commands.NAVIGATE_NEXT_DOC); menu.addMenuItem(Commands.NAVIGATE_PREV_DOC); + menu.addMenuItem(Commands.NAVIGATE_NEXT_DOC_LIST_ORDER); + menu.addMenuItem(Commands.NAVIGATE_PREV_DOC_LIST_ORDER); menu.addMenuDivider(); menu.addMenuItem(Commands.NAVIGATE_SHOW_IN_FILE_TREE); menu.addMenuDivider(); diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index de581931029..73e4262dedb 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1462,9 +1462,56 @@ define(function (require, exports, module) { } } - /** Navigate to the next/previous (MRU) document. Don't update MRU order yet */ - function goNextPrevDoc(inc) { - var result = MainViewManager.traverseToNextViewByMRU(inc); + /** + * Finds the next/previous document in MRU or list order + * @param {!number} inc Delta indicating in which direction we're going + * @param {?boolean} listOrder Whether to navigate using MRU or list order. Defaults to MRU order + * @return {?{file:File, paneId:string}} The File object of the next item in the travesal order or null if there aren't any files to traverse. + * May return current file if there are no other files to traverse. + */ + function findNextPrevDoc(inc, listOrder) { + if (listOrder) { + var allFiles = MainViewManager.getWorkingSet(MainViewManager.ALL_PANES); + if (!allFiles.length) { + // No files in any working set, so we cannot switch to one + return null; + } + + var curFile = MainViewManager.getCurrentlyViewedFile(), + curFileIndex = _.findIndex(allFiles, curFile); + + curFileIndex += inc; + curFileIndex = (curFileIndex + allFiles.length) % allFiles.length; + + var newFile = allFiles[curFileIndex], + paneId; + + // Given the index, find the pane our file is in + MainViewManager.getPaneIdList().some(function (curPaneId) { + var workingSetSize = MainViewManager.getWorkingSetSize(curPaneId); + if (workingSetSize <= curFileIndex) { + curFileIndex -= workingSetSize; + } else { + paneId = curPaneId; + return true; // break the loop now that we have our paneId + } + }); + return { + file: newFile, + paneId: paneId + }; + } else { + return MainViewManager.traverseToNextViewByMRU(inc); + } + } + + /** + * Navigate to the next/previous (MRU or list order) document. Don't update MRU order yet + * @param {!number} inc Delta indicating in which direction we're going + * @param {?boolean} listOrder Whether to navigate using MRU or list order. Defaults to MRU order + */ + function goNextPrevDoc(inc, listOrder) { + var result = findNextPrevDoc(inc, listOrder); if (result) { var file = result.file, paneId = result.paneId; @@ -1481,16 +1528,26 @@ define(function (require, exports, module) { } } - /** Next Doc command handler **/ + /** Next Doc command handler (MRU order) **/ function handleGoNextDoc() { goNextPrevDoc(+1); - } - /** Previous Doc command handler **/ + + /** Previous Doc command handler (MRU order) **/ function handleGoPrevDoc() { goNextPrevDoc(-1); } + /** Next Doc command handler (list order) **/ + function handleGoNextDocListOrder() { + goNextPrevDoc(+1, true); + } + + /** Previous Doc command handler (list order) **/ + function handleGoPrevDocListOrder() { + goNextPrevDoc(-1, true); + } + /** Show in File Tree command handler **/ function handleShowInTree() { ProjectManager.showInTree(MainViewManager.getCurrentlyViewedFile(MainViewManager.ACTIVE_PANE)); @@ -1725,6 +1782,9 @@ define(function (require, exports, module) { CommandManager.register(Strings.CMD_NEXT_DOC, Commands.NAVIGATE_NEXT_DOC, handleGoNextDoc); CommandManager.register(Strings.CMD_PREV_DOC, Commands.NAVIGATE_PREV_DOC, handleGoPrevDoc); + CommandManager.register(Strings.CMD_NEXT_DOC_LIST_ORDER, Commands.NAVIGATE_NEXT_DOC_LIST_ORDER, handleGoNextDocListOrder); + CommandManager.register(Strings.CMD_PREV_DOC_LIST_ORDER, Commands.NAVIGATE_PREV_DOC_LIST_ORDER, handleGoPrevDocListOrder); + // Special Commands CommandManager.register(showInOS, Commands.NAVIGATE_SHOW_IN_OS, handleShowInOS); CommandManager.register(quitString, Commands.FILE_QUIT, handleFileQuit); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 0c994ada75e..4dd90c7d281 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -402,6 +402,8 @@ define({ "CMD_CSS_QUICK_EDIT_NEW_RULE" : "New Rule", "CMD_NEXT_DOC" : "Next Document", "CMD_PREV_DOC" : "Previous Document", + "CMD_NEXT_DOC_LIST_ORDER" : "Next Document (List Order)", + "CMD_PREV_DOC_LIST_ORDER" : "Previous Document (List Order)", "CMD_SHOW_IN_TREE" : "Show in File Tree", "CMD_SHOW_IN_EXPLORER" : "Show in Explorer", "CMD_SHOW_IN_FINDER" : "Show in Finder", From 0ad1f99eec51336c14d0e9cca356d44b2bbc4e04 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Fri, 12 Jun 2015 23:18:25 +0200 Subject: [PATCH 3/7] Move code to MainViewManager --- src/document/DocumentCommandHandlers.js | 30 +------------------------ src/view/MainViewManager.js | 27 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 73e4262dedb..39f43f030a8 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1471,35 +1471,7 @@ define(function (require, exports, module) { */ function findNextPrevDoc(inc, listOrder) { if (listOrder) { - var allFiles = MainViewManager.getWorkingSet(MainViewManager.ALL_PANES); - if (!allFiles.length) { - // No files in any working set, so we cannot switch to one - return null; - } - - var curFile = MainViewManager.getCurrentlyViewedFile(), - curFileIndex = _.findIndex(allFiles, curFile); - - curFileIndex += inc; - curFileIndex = (curFileIndex + allFiles.length) % allFiles.length; - - var newFile = allFiles[curFileIndex], - paneId; - - // Given the index, find the pane our file is in - MainViewManager.getPaneIdList().some(function (curPaneId) { - var workingSetSize = MainViewManager.getWorkingSetSize(curPaneId); - if (workingSetSize <= curFileIndex) { - curFileIndex -= workingSetSize; - } else { - paneId = curPaneId; - return true; // break the loop now that we have our paneId - } - }); - return { - file: newFile, - paneId: paneId - }; + return MainViewManager.traverseToNextViewInListOrder(inc); } else { return MainViewManager.traverseToNextViewByMRU(inc); } diff --git a/src/view/MainViewManager.js b/src/view/MainViewManager.js index e8d1f83a19f..b553377a2c5 100644 --- a/src/view/MainViewManager.js +++ b/src/view/MainViewManager.js @@ -929,6 +929,32 @@ define(function (require, exports, module) { return ViewUtils.traverseViewArray(_mruList, index, direction); } + /** + * Get the next or previous file in list order. + * @param {!number} direction - Must be 1 or -1 to traverse forward or backward + * @return {?{file:File, paneId:string}} The File object of the next item in the traversal order or null if there aren't any files to traverse. + * May return current file if there are no other files to traverse. + */ + function traverseToNextViewInListOrder(direction) { + var file = getCurrentlyViewedFile(), + curPaneId = getActivePaneId(), + allFiles = [], + index; + + getPaneIdList().forEach(function (paneId) { + var paneFiles = getWorkingSet(paneId).map(function (file) { + return { file: file, pane: paneId }; + }); + allFiles = allFiles.concat(paneFiles); + }); + + index = _.findIndex(allFiles, function (record) { + return (record.file === file && record.pane === curPaneId); + }); + + return ViewUtils.traverseViewArray(allFiles, index, direction); + } + /** * Indicates that traversal has begun. * Can be called any number of times. @@ -1726,6 +1752,7 @@ define(function (require, exports, module) { exports.beginTraversal = beginTraversal; exports.endTraversal = endTraversal; exports.traverseToNextViewByMRU = traverseToNextViewByMRU; + exports.traverseToNextViewInListOrder = traverseToNextViewInListOrder; // PaneView Attributes exports.getActivePaneId = getActivePaneId; From 3e897de803666457b65924c57b798dbfa6bb1d25 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sat, 13 Jun 2015 23:59:02 +0200 Subject: [PATCH 4/7] Remove unnecessary findNextPrevDoc function --- src/document/DocumentCommandHandlers.js | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 39f43f030a8..5f3b0d9af4c 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1463,27 +1463,18 @@ define(function (require, exports, module) { } /** - * Finds the next/previous document in MRU or list order + * Navigate to the next/previous (MRU or list order) document. Don't update MRU order yet * @param {!number} inc Delta indicating in which direction we're going * @param {?boolean} listOrder Whether to navigate using MRU or list order. Defaults to MRU order - * @return {?{file:File, paneId:string}} The File object of the next item in the travesal order or null if there aren't any files to traverse. - * May return current file if there are no other files to traverse. */ - function findNextPrevDoc(inc, listOrder) { + function goNextPrevDoc(inc, listOrder) { + var result; if (listOrder) { - return MainViewManager.traverseToNextViewInListOrder(inc); + result = MainViewManager.traverseToNextViewInListOrder(inc); } else { - return MainViewManager.traverseToNextViewByMRU(inc); + result = MainViewManager.traverseToNextViewByMRU(inc); } - } - /** - * Navigate to the next/previous (MRU or list order) document. Don't update MRU order yet - * @param {!number} inc Delta indicating in which direction we're going - * @param {?boolean} listOrder Whether to navigate using MRU or list order. Defaults to MRU order - */ - function goNextPrevDoc(inc, listOrder) { - var result = findNextPrevDoc(inc, listOrder); if (result) { var file = result.file, paneId = result.paneId; From 7b2d157da1d3903f13893b14b1cdd9c9c3e2c414 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Wed, 17 Jun 2015 18:35:34 +0200 Subject: [PATCH 5/7] Add unit tests --- test/spec/MainViewManager-test.js | 73 +++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/test/spec/MainViewManager-test.js b/test/spec/MainViewManager-test.js index ff7a95b8455..0525334169a 100644 --- a/test/spec/MainViewManager-test.js +++ b/test/spec/MainViewManager-test.js @@ -969,5 +969,78 @@ define(function (require, exports, module) { }); }); }); + + describe("Traversing Files", function () { + beforeEach(function () { + runs(function () { + MainViewManager.setLayoutScheme(1, 2); + }); + runs(function () { + promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: testPath + "/test.js", + paneId: "first-pane" }); + waitsForDone(promise, Commands.FILE_OPEN); + }); + runs(function () { + promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: testPath + "/test.css", + paneId: "first-pane" }); + waitsForDone(promise, Commands.FILE_OPEN); + }); + runs(function () { + promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: testPath + "/test.html", + paneId: "second-pane" }); + waitsForDone(promise, Commands.FILE_OPEN); + }); + runs(function () { + MainViewManager.addToWorkingSet("first-pane", getFileObject("test.js")); + MainViewManager.addToWorkingSet("first-pane", getFileObject("test.css")); + MainViewManager.addToWorkingSet("second-pane", getFileObject("test.html")); + }); + }); + + it("should traverse in list order", function () { + runs(function () { + MainViewManager.setActivePaneId("first-pane"); + }); + runs(function () { + promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: testPath + "/test.js", + paneId: "first-pane" }); + waitsForDone(promise, Commands.FILE_OPEN); + }); + runs(function () { + var traverseResult = MainViewManager.traverseToNextViewInListOrder(1); + + expect(traverseResult.file).toEqual(getFileObject("test.css")); + expect(traverseResult.pane).toEqual("first-pane"); + }); + }); + + it("should traverse between panes in list order", function () { + runs(function () { + MainViewManager.setActivePaneId("second-pane"); + + var traverseResult = MainViewManager.traverseToNextViewInListOrder(1); + + expect(traverseResult.file).toEqual(getFileObject("test.js")); + expect(traverseResult.pane).toEqual("first-pane"); + }); + }); + + it("should traverse to the first Working Set item if a file not in the Working Set is being viewed", function () { + runs(function () { + CommandManager.execute(Commands.FILE_CLOSE, { file: getFileObject("test.js") }); + promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: testPath + "/test.js", + paneId: "first-pane" }); + waitsForDone(promise, Commands.FILE_OPEN); + }); + runs(function () { + MainViewManager.setActivePaneId("first-pane"); + + var traverseResult = MainViewManager.traverseToNextViewInListOrder(1); + + expect(traverseResult.file).toEqual(getFileObject("test.css")); + expect(traverseResult.pane).toEqual("first-pane"); + }); + }); + }); }); }); From db4d5ee1d5f33d38ce5d049580df856685a05ee1 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Fri, 19 Jun 2015 18:17:49 +0200 Subject: [PATCH 6/7] Unit test changes --- test/spec/MainViewManager-test.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/spec/MainViewManager-test.js b/test/spec/MainViewManager-test.js index 0525334169a..c19d0c049fc 100644 --- a/test/spec/MainViewManager-test.js +++ b/test/spec/MainViewManager-test.js @@ -999,12 +999,14 @@ define(function (require, exports, module) { it("should traverse in list order", function () { runs(function () { - MainViewManager.setActivePaneId("first-pane"); - }); - runs(function () { - promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: testPath + "/test.js", - paneId: "first-pane" }); - waitsForDone(promise, Commands.FILE_OPEN); + // Make test.js the active file + promise = new $.Deferred(); + DocumentManager.getDocumentForPath(testPath + "/test.js") + .done(function (doc) { + MainViewManager._edit("first-pane", doc); + promise.resolve(); + }); + waitsForDone(promise, "MainViewManager._edit"); }); runs(function () { var traverseResult = MainViewManager.traverseToNextViewInListOrder(1); @@ -1016,8 +1018,6 @@ define(function (require, exports, module) { it("should traverse between panes in list order", function () { runs(function () { - MainViewManager.setActivePaneId("second-pane"); - var traverseResult = MainViewManager.traverseToNextViewInListOrder(1); expect(traverseResult.file).toEqual(getFileObject("test.js")); @@ -1027,6 +1027,7 @@ define(function (require, exports, module) { it("should traverse to the first Working Set item if a file not in the Working Set is being viewed", function () { runs(function () { + // Close test.js to then reopen it without being in the Working Set CommandManager.execute(Commands.FILE_CLOSE, { file: getFileObject("test.js") }); promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: testPath + "/test.js", paneId: "first-pane" }); From afc1d3b34aa3465c7203a7ca9d237e8847eb97ef Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Fri, 19 Jun 2015 18:22:09 +0200 Subject: [PATCH 7/7] Add unit test for reverse list order --- test/spec/MainViewManager-test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/spec/MainViewManager-test.js b/test/spec/MainViewManager-test.js index c19d0c049fc..3fa5447b567 100644 --- a/test/spec/MainViewManager-test.js +++ b/test/spec/MainViewManager-test.js @@ -1042,6 +1042,25 @@ define(function (require, exports, module) { expect(traverseResult.pane).toEqual("first-pane"); }); }); + + it("should traverse between panes in reverse list order", function () { + runs(function () { + // Make test.js the active file + promise = new $.Deferred(); + DocumentManager.getDocumentForPath(testPath + "/test.js") + .done(function (doc) { + MainViewManager._edit("first-pane", doc); + promise.resolve(); + }); + waitsForDone(promise, "MainViewManager._edit"); + }); + runs(function () { + var traverseResult = MainViewManager.traverseToNextViewInListOrder(-1); + + expect(traverseResult.file).toEqual(getFileObject("test.html")); + expect(traverseResult.pane).toEqual("second-pane"); + }); + }); }); }); });