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 2 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
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_SPLITVIEW_TOGGLE = "cmd.splitViewToggle"; // MainViewManager.js _handleSwitchPane

// File shell callbacks - string must MATCH string in native code (appshell/command_callbacks.h)
exports.HELP_ABOUT = "help.about"; // HelpCommandHandlers.js _handleAboutDialog()
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_SPLITVIEW_TOGGLE" : "Toggle Panes",
Copy link
Collaborator

Choose a reason for hiding this comment

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

As this command is for shuffling focus across panes, can we name it accordingly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we write it like this?
"CMD_SHUFFLE_FOCUS_ACROSS_PANES" : "Shuffle Focus Across Panes",

Copy link
Collaborator

Choose a reason for hiding this comment

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

@petetnt What could be a suitable var name here ?

Copy link
Collaborator

@petetnt petetnt Oct 26, 2016

Choose a reason for hiding this comment

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

Something like SWITCH_PANE_FOCUS? 🚲 🏠

edit: changed from TOGGLE as it sounds like on/off


// Help menu commands
"HELP_MENU" : "Help",
Expand Down
4 changes: 2 additions & 2 deletions src/view/MainViewManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -1658,8 +1658,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
18 changes: 18 additions & 0 deletions src/view/Pane.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ define(function (require, exports, module) {
ViewUtils = require("utils/ViewUtils"),
ProjectManager = require("project/ProjectManager"),
paneTemplate = require("text!htmlContent/pane.html");
var KeyBindingManager = brackets.getModule("command/KeyBindingManager");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This var name declaration can be clubbed with the previous one.


/**
* Internal pane id
Expand Down Expand Up @@ -208,6 +209,19 @@ define(function (require, exports, module) {
return {indexRequested: requestIndex, index: index};
}

/**
* Toggle between panes
*/
function _handleSwitchPane() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This entire logic of shuffling focus across panes could be in MainViewManager. Pane should contain operations specific to creation, state mutation and handling other things inside a Pane. As this operation is a user requested focus preemption, it could be at a higher level above Panes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try moving this function to MainViewManager. Thank you!

var $firstPane = $('#first-pane'), $secondPane = $('#second-pane');
if($firstPane.hasClass('active-pane')) {
$secondPane.click();
}
else {
$firstPane.click();
}
}

/**
* @typedef {!$el: jQuery, getFile:function():!File, updateLayout:function(forceRefresh:boolean), destroy:function(), getScrollPos:function():?, adjustScrollPos:function(state:Object=, heightDelta:number)=, getViewState:function():?*=, restoreViewState:function(viewState:!*)=, notifyContainerChange:function()=, notifyVisibilityChange:function(boolean)=} View
*/
Expand Down Expand Up @@ -385,6 +399,10 @@ define(function (require, exports, module) {
break;
}

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

// Listen to document events so we can update ourself
DocumentManager.on(this._makeEventName("fileNameChange"), _.bind(this._handleFileNameChange, this));
DocumentManager.on(this._makeEventName("pathDeleted"), _.bind(this._handleFileDeleted, this));
Expand Down