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

Add a feature to toggle between panes in the core #10555 #12853

Merged
merged 20 commits into from
Oct 28, 2016
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
08a46b3
Added the feature to toggle between panes as requested by petetnt in …
arthur801031 Oct 26, 2016
2f793e7
Added the feature to toggle between panes as requested by petetnt in …
arthur801031 Oct 26, 2016
fc5a114
Bumping version Number to 1.9
shubhsnov Oct 26, 2016
b092dea
Merge pull request #12855 from adobe/adobe/shubham/VersionUpdate
swmitra Oct 26, 2016
0b63bc7
Edited the files and added a unit test for this feature as requested …
arthur801031 Oct 27, 2016
ce4350b
Edited the files and added a unit test for this feature as requested …
arthur801031 Oct 27, 2016
e099010
Removed duplicate/unnecessary code in Pane.js
arthur801031 Oct 27, 2016
77a8f4b
Removed additional duplicate/unnecessary code in Pane.js
arthur801031 Oct 27, 2016
4289bd5
Edited 'should switch pane when alt-w is pressed' function in MainVie…
arthur801031 Oct 27, 2016
8211931
Revert package.json and src/config.json to previous commit
arthur801031 Oct 27, 2016
40586ae
Added the feature to toggle between panes as requested by petetnt in …
arthur801031 Oct 26, 2016
569698f
Added the feature to toggle between panes as requested by petetnt in …
arthur801031 Oct 26, 2016
8823a85
Edited the files and added a unit test for this feature as requested …
arthur801031 Oct 27, 2016
567668a
Removed duplicate/unnecessary code in Pane.js
arthur801031 Oct 27, 2016
77ee794
Removed additional duplicate/unnecessary code in Pane.js
arthur801031 Oct 27, 2016
88b333e
Edited 'should switch pane when alt-w is pressed' function in MainVie…
arthur801031 Oct 27, 2016
717d13e
Revert package.json and src/config.json to previous commit
arthur801031 Oct 27, 2016
0848104
Modified test code to test CMD_SWITCH_PANE_FOCUS
arthur801031 Oct 28, 2016
954f39c
Modified test code to test CMD_SWITCH_PANE_FOCUS
arthur801031 Oct 28, 2016
8e5d581
Removed unnecessary code in MainViewManager-test.js
arthur801031 Oct 28, 2016
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "Brackets",
"version": "1.8.0-0",
"apiVersion": "1.8.0",
"version": "1.9.0-0",
"apiVersion": "1.9.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs rebase against master so we don't have these duplicate files, git fetch upstream master && git rebase upstream/master.

"homepage": "http://brackets.io",
"issues": {
"url": "http://github.com/adobe/brackets/issues"
Expand Down
1 change: 1 addition & 0 deletions src/command/Commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ define(function (require, exports, module) {
exports.CMD_SPLITVIEW_NONE = "cmd.splitViewNone"; // SidebarView.js _handleSplitNone()
exports.CMD_SPLITVIEW_VERTICAL = "cmd.splitViewVertical"; // SidebarView.js _handleSplitVertical()
exports.CMD_SPLITVIEW_HORIZONTAL = "cmd.splitViewHorizontal"; // SidebarView.js _handleSplitHorizontal()
exports.CMD_SWITCH_PANE_FOCUS = "cmd.switchPaneFocus"; // MainViewManager.js _switchPaneFocus()

// File shell callbacks - string must MATCH string in native code (appshell/command_callbacks.h)
exports.HELP_ABOUT = "help.about"; // HelpCommandHandlers.js _handleAboutDialog()
Expand Down
4 changes: 2 additions & 2 deletions src/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
"healthDataServerURL": "https://healthdev.brackets.io/healthDataLog"
},
"name": "Brackets",
"version": "1.8.0-0",
"apiVersion": "1.8.0",
"version": "1.9.0-0",
"apiVersion": "1.9.0",
"homepage": "http://brackets.io",
"issues": {
"url": "http://github.com/adobe/brackets/issues"
Expand Down
1 change: 1 addition & 0 deletions src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ define({
"CMD_SHOW_IN_EXPLORER" : "Show in Explorer",
"CMD_SHOW_IN_FINDER" : "Show in Finder",
"CMD_SHOW_IN_OS" : "Show in OS",
"CMD_SWITCH_PANE_FOCUS" : "Switch Pane Focus",

// Help menu commands
"HELP_MENU" : "Help",
Expand Down
43 changes: 40 additions & 3 deletions src/view/MainViewManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ define(function (require, exports, module) {
AsyncUtils = require("utils/Async"),
ViewUtils = require("utils/ViewUtils"),
Resizer = require("utils/Resizer"),
Pane = require("view/Pane").Pane;
Pane = require("view/Pane").Pane,
KeyBindingManager = brackets.getModule("command/KeyBindingManager");

/**
* Preference setting name for the MainView Saved State
Expand Down Expand Up @@ -844,6 +845,35 @@ define(function (require, exports, module) {
return result.promise();
}

/**
* Switch between panes
*/
function switchPaneFocus() {
var $firstPane = $('#first-pane'), $secondPane = $('#second-pane');
if($firstPane.hasClass('active-pane')) {
$secondPane.click();
}
else {
$firstPane.click();
}
}

/**
* Unit test for switching from first pane to second pane
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use SpecRunnerUtils.simulateKeyEvent instead; for examples see https://github.com/adobe/brackets/blob/f028b3ecdf2017498196f39ddbbe4a2364366c92/test/spec/InstallExtensionDialog-test.js and search for simulateKeyEvent

Copy link
Contributor Author

@arthur801031 arthur801031 Oct 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @petetnt I'm having trouble simulating 'Alt-w' key press using SpecRunnerUtils.simulateKeyEvent. Here is my attempt:

SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_ALT, "keydown", `$('#first-pane')[0]);
SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_W, "keydown", $('#first-pane')[0]);
SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_W, "keypress", $('#first-pane')[0]);
SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_W, "keyup", $('#first-pane')[0]);

Could you please let me know what I'm doing wrong? Thank you so much!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arthur801031 Dang, I remembered that SimulateKeyEvent has modifier keys support but it seems not: https://github.com/adobe/brackets/blob/master/test/spec/SpecRunnerUtils.js#L998

Let's modify the tests so that you use CommandManager.executeCommand to run cmd.switchPaneFocus and check that panes have switches after running that command.

Sorry for the extra trouble. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petetnt So we don't check whether or not 'Alt-W' is pressed? We only check if cmd.switchPaneFocus can actually switch pane between first pane and second pane.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arthur801031 exactly. I created an issue for adding the modifier key option #12859

*/
function switchPaneUnitTest1To2() {
var $firstPane = $('#first-pane');
$firstPane.click();
KeyBindingManager._handleKey('Alt-W');
}

/**
* Unit test for switching from second pane to first pane
*/
function switchPaneUnitTest2To1() {
KeyBindingManager._handleKey('Alt-W');
}

/**
* DocumentManager.pathDeleted Event handler to remove a file
* from the MRU list
Expand Down Expand Up @@ -1616,6 +1646,10 @@ define(function (require, exports, module) {
// get an event handler for workspace events and we don't listen
// to the event before we've been initialized
WorkspaceManager.on("workspaceUpdateLayout", _updateLayout);

// Listen to key Alt-W to toggle between panes
CommandManager.register(Strings.CMD_SWITCH_PANE_FOCUS, Commands.CMD_SWITCH_PANE_FOCUS, switchPaneFocus);
KeyBindingManager.addBinding(Commands.CMD_SWITCH_PANE_FOCUS, {key: 'Alt-W'});
}

/**
Expand Down Expand Up @@ -1658,8 +1692,8 @@ define(function (require, exports, module) {

return result;
}


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert this change as it's only unintentional whitespace modification.

/**
* Setup a ready event to initialize ourself
*/
Expand Down Expand Up @@ -1729,6 +1763,9 @@ define(function (require, exports, module) {

exports.getAllOpenFiles = getAllOpenFiles;
exports.focusActivePane = focusActivePane;
exports.switchPaneFocus = switchPaneFocus;
exports.switchPaneUnitTest1To2 = switchPaneUnitTest1To2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed, see above

exports.switchPaneUnitTest2To1 = switchPaneUnitTest2To1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed, see above


// Layout
exports.setLayoutScheme = setLayoutScheme;
Expand Down
25 changes: 25 additions & 0 deletions test/spec/MainViewManager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,31 @@ define(function (require, exports, module) {
expect(MainViewManager.getLayoutScheme()).toEqual({rows: 1, columns: 1});
});
});
it("should switch pane when alt-w is pressed", function () {
runs(function () {
MainViewManager.setLayoutScheme(1, 2);
});
runs(function () {
MainViewManager.switchPaneUnitTest1To2();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above for my comment on how to handle these; the tests are valid but they test if switchPaneUnitTest1To2 work, instead of testing if the keypress itself work ^^

expect(MainViewManager.getActivePaneId()).toEqual("second-pane");
});
runs(function () {
MainViewManager.switchPaneUnitTest2To1();
expect(MainViewManager.getActivePaneId()).toEqual("first-pane");
});

runs(function () {
MainViewManager.setLayoutScheme(2, 1);
});
runs(function () {
MainViewManager.switchPaneUnitTest1To2();
expect(MainViewManager.getActivePaneId()).toEqual("second-pane");
});
runs(function () {
MainViewManager.switchPaneUnitTest2To1();
expect(MainViewManager.getActivePaneId()).toEqual("first-pane");
});
});
it("should activate pane when editor gains focus", function () {
var editors = {},
handler = function (e, doc, editor, paneId) {
Expand Down