Feature/part resize: Add key bindings to resize focused view by cleidigh · Pull Request #22861 · microsoft/vscode

cleidigh · 2017-03-19T21:50:31Z

See issue #22645

Part Resize Feature Notes:

Proposed feature to address issue #22645:
Keyboard bindings to expand or contract the main axis of the focused view

Two key bindings expand view, contract view

key bindings based on part focus (potential reduction of key collisions)

expansion or contraction follow main axis of part and orientation

eg sidebar has focus, expand moves border right if on left
or left if sidebar on right
eg panel has focus expand moves border up...
editor focus: expand moves right border right (h. layout)
top up (v. layout) or opposite if single editor
approach still works if a new part eg secondary sidebar
is added in the future

Possible (mnemonic) key bindings: (probably none assigned by default)

[expand part]: ctrl+alt+shift+ >
[contract part]: ctrl+alt+shift+ <

[expand part]: ctrl+alt+shift+ x
[contract part]: ctrl+alt+shift+ c

[expand part]: ctrl+alt+shift+ )
[contract part]: ctrl+alt+shift+ (

[restore default part size]: ctrl+alt+shift+ d
[restore default part size]: ctrl+alt+shift+ r

Potential Future Issues:

A part with no "primary axis" (expanding contract ambiguous)

Main file work:

Main file, follows toggleSideBarVisibility.ts design approach

workbench command registrations
action functions

.\src\vs\workbench\browser\actions\resizePart.ts

Workbench Support Functions

set up per Part
access to workbench layout
.\src\vs\workbench\electron-browser\workbench.ts

Layout Support Functions

adjust main access dimensions pre- layout call
possible reset to default option
.\src\vs\workbench\browser\layout.ts

The prototype implementation sports resize
of the sidebar, the panel and one editor.

After looking more into the code base as well
as the navigation contribution by @misoguy
I think some files need to be moved around.
my first implementation was based on toggleSideBarVisibility
but I think the more recent code add
the commands to actions.ts

The next step is to flush out support for the editors
where more than one is visible. and re- factor
per suggestion input.

Thanks for considering...

Bringing fork up to date

update fork

mention-bot · 2017-03-19T21:50:35Z

@cleidigh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bpasero and @egamma to be potential reviewers.

msftclas · 2017-03-19T21:50:35Z

@cleidigh,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

msftclas · 2017-03-19T22:08:16Z

@cleidigh, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

cleidigh · 2017-03-19T22:14:54Z

my apologies and get hub ignorance
I have to exclude the several local working files
like tasks.json etc

cleidigh · 2017-03-19T22:56:58Z

okay I think I have this correct now

misoguy · 2017-03-20T00:05:45Z

src/vs/workbench/browser/actions/resizePart.ts

+	}
+}
+
+export class ContractViewAction extends Action {


It seems to me that a lot of the code in ContractViewAction looks the same with ExpandViewAction except for the increment value which could be abstracted to a BaseResizeViewAction and an argument INCREMENT_DIRECTION (probably 1 or -1) could be passed down.

@misoguy
Agree, I've looked at your code a lot as well as comments. I was anxious
to get some initial feedback before doing more re- factoring.
I also think moving the resize actions into actions.ts like your code
is the newer approach. When I started I modeled my code after the toggleSidebarVisibility.ts
not sure why these several toggle functions are in separate files, anybody?

Hope you give it a try, seems like we have some common interests...

cleidigh · 2017-03-24T16:14:55Z

@bpasero
quick update
silo size checking took a little longer than expected
everything else just checked off just have to move to actions
then I'll push my commits, hopefully in a couple hours

cleidigh · 2017-03-24T18:43:29Z

@bpasero
okay this is ready for review
I'm sure there will be things to address
but I want to see I have nothing structurally wrong
and you have a chance to see the behaviors

note : I made the choice to favor the focused part and increase the editor size, shrinking both
the others if there are three editors. Maintaining starting ratio. This happens until minimum
sizes met. If you contract after this the ratios may have been lost an equal distribution
will occur.

bpasero

@cleidigh good job! this seems to work very nice, a couple of feedback attached 👍

bpasero · 2017-03-24T20:25:46Z

src/vs/workbench/browser/parts/editor/editorGroupsControl.ts

 	setGroupOrientation(orientation: GroupOrientation): void;
 	getGroupOrientation(): GroupOrientation;

+	requestActiveGroupSizeChange(groupSizeChange: number): boolean;


Can we maybe call this similar to the other one resizePart in partService, e.g. resizeGroup and you pass in the position of the group to resize?

done
was never wedded to the name ;-)

bpasero · 2017-03-24T20:26:50Z

src/vs/workbench/electron-browser/workbench.ts

+			case Parts.SIDEBAR_PART:
+			case Parts.PANEL_PART:
+			case Parts.EDITOR_PART:
+				this.workbenchLayout.setPartSizeChange(part, sizeChange);


setPartSizeChange => resizePart ?

bpasero · 2017-03-24T20:37:46Z

src/vs/workbench/browser/parts/editor/editorGroupsControl.ts

+		return true;
+	}
+
+	protected boundSiloSize(siloPosition: Position, sizeChangePx: number): number {


protected => private?

bpasero · 2017-03-24T20:37:51Z

src/vs/workbench/browser/parts/editor/editorGroupsControl.ts

+		return newSiloSize;
+	}
+
+	protected distributeRemainingSilosSize(remPosition1: Position, remPosition2: Position, availableSize: number): void {


protected => private?

cleidigh · 2017-03-24T21:53:22Z

F2 helps a lot !
but it seemed to miss some files, maybe there's a scope thing I don't understand

bpasero · 2017-03-25T16:34:25Z

@cleidigh excellent job 👍

cleidigh · 2017-03-25T16:58:18Z

@bpasero
awesome thanks - just happened to see you added the debug support for the shared process - will try out today

fyi - working on this project is very good for me. since I'm not able to work a regular job I'm very interested in becoming regular contributor. I'm not fast but I do have a decent amount of time
to devote to this. given my firmware/communication software experience I'm hoping to be able to
help with some of the lower level process/debugging/issues eventually. I also have found a few bugs in SCM I'm looking at now.

In the meantime I'm more than happy to help out with issues/bugs either for the March deadline
for upcoming April deadline . I know and have already looked at the help-wanted issues, however
I don't know what the priorities are. if you would like to give me some assignments that you think
would be useful and within my range , I'm all up for it. and thanks for working to review and to merge
the new code !

This participation and focused goals are a great thing for me...

cheers Christopher

bpasero · 2017-03-25T17:50:14Z

@cleidigh yes, I added a debug configuration that allows to debug both main and renderer process from within VS Code (provided you installed the Chrome Debug Adapter extension).

Thanks for willing to help out more, going after the help wanted bugs is a good start. Often it helps to first quickly ask if it is OK to start working on a particular issue, just to confirm that we have cycles to review a potential contribution in that area and it makes sense via PR.

As for priorities, we typically just go by number of votes on an issue to decide if something is important or not :)

cleidigh · 2017-03-25T18:02:21Z

@bpasero

tested newly built OSS
see my code - cool
testing debugging - appears that runtimeArg $(workspaceRoot) presumably replaced
by webroot debugging parameter prevents launch (alone or in compound)
remove and launch\attach and debugging work

{
"type": "chrome",
"request": "launch",
"name": "Launch VS Code",
"windows": {
"runtimeExecutable": "${workspaceRoot}/scripts/code.bat"
},
"osx": {
"runtimeExecutable": "${workspaceRoot}/scripts/code.sh"
},
"linux": {
"runtimeExecutable": "${workspaceRoot}/scripts/code.sh"
},
"urlFilter": "index.html",
"runtimeArgs": [
*** "${workspaceRoot}",
"--debug=5875"
],
"webRoot": "${workspaceRoot}"
}

bpasero · 2017-03-25T18:43:42Z

Weird, it really works for me out of the box. Are you using our insiders build or stable?

cleidigh · 2017-03-25T19:15:34Z

@bpasero
was using last nights Insiders
just tried stable - same problem - have to remove run time arg to work
sorry - should've mentioned the error - typical cannot connect to runtime with legacy protocol...

bpasero · 2017-03-25T23:29:56Z

@cleidigh are you on windows?

cleidigh · 2017-03-26T16:16:36Z

sorry made the trek downstairs and was out of pocket
yes - Windows 7 under an OS X VM (which I can't really use - no decent voice recognition)
Latest Chrome debugger

Unfortunately I can only directly contribute and test Windows VSC

Unrelated question - what version of typescript should be used for compiling OSS
I am getting version mismatches and I see a new TypescriptInsiders version
but it's not clear which set up to use and I get the warning even if I changed versions

misoguy · 2017-03-27T00:56:10Z

@cleidigh Great job! I have been following this PR but didn't really get the time to review it deeply till now :( It looks great 👍

cleidigh · 2017-03-27T03:29:40Z

@misoguy
Thanks a bunch! I learned a lot by looking at your code.

I think your navigation and the view size keyboard features will be a fine pair coming up together
in the next version ;-)

right now I'm looking at what next to attack, how about you?
one of the things would be some more keyboard accessibility for
debugging sessions

cleidigh added 18 commits March 13, 2017 13:35

Merge pull request #1 from Microsoft/master

4272abb

Bringing fork up to date

Merge pull request #3 from Microsoft/master

6460b7b

update fork

Merge pull request #4 from Microsoft/master

1bac80c

update fork

cleidigh commands

3fbf09b

debugger multi- launched fixes

b463b22

Base resize part/view implementation

3e0204b

Base resize part/view implementation

652ea2d

Base resize part/view implementation

4a1f0e5

Base resize part/view implementation

a7eabfa

Base resize part/view implementation

c5f6183

Base resize part/view implementation

6bf8966

base resize commits

e47a912

base resize part commits

9a78c1d

base resize part commits

b68d4a3

base resize part commit

8ebc563

base resize commit

54113b0

base resize part

e8c2273

update commands

e225f9f

msftclas added the cla-required label Mar 19, 2017

msftclas added cla-signed and removed cla-required labels Mar 19, 2017

cleidigh added 2 commits March 19, 2017 18:31

delete local files

fa1a5bd

fixed local changes

09e04fb

misoguy reviewed Mar 20, 2017

View reviewed changes

bpasero self-assigned this Mar 20, 2017

cleidigh added 5 commits March 23, 2017 19:27

first cleanup

db227ca

clean up

aa93438

Cleanup 3

b8eb384

cleanup 3

6ef227b

cleanup 3

8d4075d

cleidigh added 3 commits March 24, 2017 14:14

move resize code to actions - remove resizePart

c3d0be1

move resize view to actions

5e62b89

second push with full implementation

08b9216

cleidigh added 2 commits March 24, 2017 14:47

remove test console line

ef5a64e

removed print silos

c37d0df

bpasero requested changes Mar 24, 2017

View reviewed changes

cleidigh added 3 commits March 24, 2017 17:04

rename functions per comments

c15adf8

function renames the resizeGroup with position

773fdd9

fixed space for hygiene

1cd6b6e

bpasero merged commit 48ee232 into microsoft:master Mar 25, 2017

alanjds mentioned this pull request Nov 17, 2017

Resize windows via keyboard commands VSCodeVim/Vim#2044

Closed

github-actions bot locked and limited conversation to collaborators Mar 27, 2020

Conversation

cleidigh commented Mar 19, 2017

Uh oh!

mention-bot commented Mar 19, 2017

Uh oh!

msftclas commented Mar 19, 2017

Uh oh!

msftclas commented Mar 19, 2017

Uh oh!

cleidigh commented Mar 19, 2017

Uh oh!

cleidigh commented Mar 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cleidigh commented Mar 24, 2017

Uh oh!

cleidigh commented Mar 24, 2017

Uh oh!

bpasero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cleidigh commented Mar 24, 2017

Uh oh!

bpasero commented Mar 25, 2017

Uh oh!

cleidigh commented Mar 25, 2017

Uh oh!

bpasero commented Mar 25, 2017

Uh oh!

cleidigh commented Mar 25, 2017

Uh oh!

bpasero commented Mar 25, 2017

Uh oh!

cleidigh commented Mar 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bpasero commented Mar 25, 2017

Uh oh!

cleidigh commented Mar 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

misoguy commented Mar 27, 2017

Uh oh!

cleidigh commented Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cleidigh commented Mar 25, 2017 •

edited

Loading

cleidigh commented Mar 26, 2017 •

edited

Loading

cleidigh commented Mar 27, 2017 •

edited

Loading