Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workbench: Adds actions for moveEditorToFirstGroup, moveEditorToSecondGroup, moveEditorToThirdGroup #40635

Merged
merged 4 commits into from
Dec 28, 2017

Conversation

shobhitchittora
Copy link
Contributor

@shobhitchittora shobhitchittora commented Dec 21, 2017

Adds the following actions for workbench tabs -

(For Mac Only - TODO: Need to confirm the same in Win & UNIX)

  1. moveEditorToFirstGroup - ^ ⌘ 1
  2. moveEditorToSecondGroup - ^ ⌘ 2
  3. moveEditorToThirdGroup - ^ ⌘ 3

Fixes: #40525

@shobhitchittora
Copy link
Contributor Author

shobhitchittora commented Dec 21, 2017

@bpasero Are there any tests to extend upon?

@bpasero bpasero modified the milestones: On Deck, December 2017/January 2018 Dec 21, 2017
@@ -363,6 +363,9 @@ registry.registerWorkbenchAction(new SyncActionDescriptor(MoveGroupLeftAction, M
registry.registerWorkbenchAction(new SyncActionDescriptor(MoveGroupRightAction, MoveGroupRightAction.ID, MoveGroupRightAction.LABEL, { primary: KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_K, KeyCode.RightArrow) }), 'View: Move Editor Group Right', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(MoveEditorToPreviousGroupAction, MoveEditorToPreviousGroupAction.ID, MoveEditorToPreviousGroupAction.LABEL, { primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.LeftArrow, mac: { primary: KeyMod.CtrlCmd | KeyMod.WinCtrl | KeyCode.LeftArrow } }), 'View: Move Editor into Previous Group', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(MoveEditorToNextGroupAction, MoveEditorToNextGroupAction.ID, MoveEditorToNextGroupAction.LABEL, { primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.RightArrow, mac: { primary: KeyMod.CtrlCmd | KeyMod.WinCtrl | KeyCode.RightArrow } }), 'View: Move Editor into Next Group', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(MoveEditorToFirstGroupAction, MoveEditorToFirstGroupAction.ID, MoveEditorToFirstGroupAction.LABEL, { primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.KEY_1, mac: { primary: KeyMod.CtrlCmd | KeyMod.WinCtrl | KeyCode.KEY_1 } }), 'View: Move Editor into First Group', category);
Copy link
Member

Choose a reason for hiding this comment

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

@shobhitchittora keybinding KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.KEY_1 is already taken by "Toggle Editor Layout" and cannot be used again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct me if I'm wrong -

  1. The primary key binding scheme is applied to Windows and Linux builds. Mac has it's own primary binding object specified.
  2. KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.KEY_1 is MAC-primary for "Toggle Editor Layout", while it's WIN/LINUX-primary for "MoveEditorToFirstGroupAction".
  3. Any other suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

@shobhitchittora you are right, sorry I did not see the mac specific keybinding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking into it 👍🏻.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


export class MoveEditorToFirstGroupAction extends Action {

public static readonly ID = 'workbench.action.moveEditorToFirstGroupAction';
Copy link
Member

Choose a reason for hiding this comment

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

@shobhitchittora remove the trailing Action from the action ids (e.g. workbench.action.moveEditorToFirstGroup)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -1553,3 +1553,75 @@ export class MoveEditorToNextGroupAction extends Action {
return TPromise.as(true);
}
}

export class MoveEditorToFirstGroupAction extends Action {
Copy link
Member

Choose a reason for hiding this comment

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

@shobhitchittora all these actions are very similar in their implementation. Can we have a base class (abstract) that all classes extend from and in the base class we simply have a method to ask for the position?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 💅🏻.
@bpasero Please review now.

@shobhitchittora
Copy link
Contributor Author

shobhitchittora commented Dec 26, 2017

@bpasero Any idea why Travis is failing?

screen shot 2017-12-28 at 10 33 55 am

@bpasero bpasero merged commit c60114f into microsoft:master Dec 28, 2017
@bpasero
Copy link
Member

bpasero commented Dec 28, 2017

LGTM thanks 👍

@hardianlawi
Copy link

Hi,

Why couldn't I find these shortcuts in my keyboard references now? I think I used to be able to press shift+Alt+2 and shift+Alt+3 to move the editors (it will create it if the group does not exist)

Thank you

@hardianlawi
Copy link

The VSCode version im using

image

@bpasero
Copy link
Member

bpasero commented Jul 6, 2018

@hardianlawi they got removed. Read this and this.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to move editor to specific group
3 participants