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

Centered editor layout shuold be per workspace not window #46277

Closed
jeffhube opened this issue Mar 21, 2018 · 11 comments
Closed

Centered editor layout shuold be per workspace not window #46277

jeffhube opened this issue Mar 21, 2018 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug layout General VS Code workbench layout issues verified Verification succeeded
Milestone

Comments

@jeffhube
Copy link

Issue Type: Bug

  1. Activate zen mode (Command Palette > Toggle Zen Mode)
  2. Open a new window (Command Palette > New Window)

Expected behavior: the new window is not using the zen mode centered layout
Actual behavior: the new window is using the zen mode centered layout

This seems similar to #45610 , except vscode isn't being closed.

VS Code version: Code 1.21.1 (79b44aa, 2018-03-14T14:46:47.128Z)
OS version: Windows_NT x64 10.0.16299

System Info
Item Value
CPUs Intel(R) Core(TM) i7-7600U CPU @ 2.80GHz (4 x 2904)
Memory (System) 15.89GB (8.61GB free)
Process Argv C:\Program Files\Microsoft VS Code\Code.exe
Screen Reader no
VM 0%
Extensions (14)
Extension Author (truncated) Version
xml Dot 1.9.2
tslint eg2 1.0.28
python ms- 2018.2.1
cpptools ms- 0.15.0
csharp ms- 1.14.0
vetur oct 0.11.7
typescript-hero rbb 2.3.1
rust rus 0.4.0
salesforcedx-vscode sal 42.7.0
salesforcedx-vscode-apex sal 42.7.0
salesforcedx-vscode-apex-debugger sal 42.7.0
salesforcedx-vscode-core sal 42.7.0
salesforcedx-vscode-lightning sal 42.7.0
salesforcedx-vscode-visualforce sal 42.7.0
Reproduces without extensions
@vscodebot
Copy link

vscodebot bot commented Mar 21, 2018

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@bpasero bpasero assigned isidorn and unassigned bpasero Mar 21, 2018
@isidorn
Copy link
Contributor

isidorn commented Mar 22, 2018

We decided for th ecntered editor layour to be preserved across workspaces https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/browser/parts/editor/editorGroupsControl.ts#L1059

This is just a design decision we took. So let's keep this issue open and wait for more feedback to potentially change that

If editor are horizontal or vertical is stored per workspace. So it makes sense to align with that imho

fyi @SrTobi

@isidorn isidorn changed the title Opening new window while in zen mode causes it to use centered layout Centered editor layout shuold be per workspace not window Mar 22, 2018
@isidorn isidorn added the layout General VS Code workbench layout issues label Mar 22, 2018
@isidorn isidorn added this to the March 2018 milestone Mar 22, 2018
@SrTobi
Copy link
Contributor

SrTobi commented Mar 22, 2018

No, I think @jeffhube is right... the problem is not that the centered editor is stored in StorageScope.GLOBAL but that it is stored at all when it was activated by transitioning to the zen mode. It's actually quite similar to the problems I mentioned in #45610 and #45773.

The main problem is the following: when we transition to zenmode we change all kind of things (we toggle the centered layout, we hide the sidebar etc) and we do it in a way that the user thinks it's temporal (which it kind of is, because it will be reset when we exit the zenmode) but the things that are changed do save their state permanent by saving it into the storage. That's why the new window thinks centered layout should still be active...

We could fix that (and by fix I mean “make it seem to work”) by changing the storage scope to workspace but as @isidorn pointed out that isn't a good solution because the centered layout should be aligned to the vertical/horizontal setting

I don't see how we can fix this without somehow rewriting the code for the save procedure of all the things zenmode changes... because they should not save their state if they are only temporarily changed because of the zenmode

If we want to address this (and I think we should) it could also be the solution for what I mentioned in #45773... luckily I haven't started to implement anything 😄

@isidorn
Copy link
Contributor

isidorn commented Mar 28, 2018

Nothing for this milestone since we are in endgame week, thus I am assigning it to the april milestone so we can discuss this in greater detail when I am back from vacation in about 2 weeks.

@isidorn isidorn modified the milestones: March 2018, April 2018 Mar 28, 2018
@isidorn isidorn added the under-discussion Issue is under discussion for relevance, priority, approach label Mar 28, 2018
@isidorn
Copy link
Contributor

isidorn commented Apr 23, 2018

I was on vacation for the previous milestone thus pushing this to May.
I will investigate how to best fix this next week. Suggestions are welcome

@isidorn isidorn modified the milestones: April 2018, May 2018 Apr 23, 2018
@SrTobi
Copy link
Contributor

SrTobi commented Apr 23, 2018

This is a very nasty problem... I think a solution would be to not save the state of the centered layout if it was activated because of transitioning to zen mode.

@isidorn isidorn closed this as completed in 4161a0d May 9, 2018
@isidorn
Copy link
Contributor

isidorn commented May 9, 2018

After looking into this I decided to store the centered editor layout per workspace not per user. Since all our other layout storage is stored per workspace.
The only difference being horizontal and vertical layout becuase that is a bit more permanat and is monitor dependant and not workspace dependant so I think it makes sense.

@SrTobi you can try it out in the insiders from tomorrow and if you still see some corner cases where this is not behaving as expected please create a new issue which would be great - thanks a lot!

@isidorn isidorn added bug Issue identified by VS Code Team member as probable bug and removed under-discussion Issue is under discussion for relevance, priority, approach labels May 9, 2018
@RMacfarlane RMacfarlane added the verified Verification succeeded label May 31, 2018
@uloco
Copy link

uloco commented Jun 15, 2018

I really like the centered layout feature. Unfortunately the centered view gets lost on diff views, even if inline diff is activated. Having a centered layout here also would make me really happy. :)))

@SrTobi
Copy link
Contributor

SrTobi commented Jun 15, 2018

Yeah, we did that intentionally because we thought that the user would want to use the full editor space when watching two files side by side.

Could you maybe elaborate why/how you are using the centered layout? Is your screen just that wide 😃 or is there another usecase here that needs the diff view centered?

@uloco
Copy link

uloco commented Jun 15, 2018

Using inline diff, you do not have two files side by side as you can see here:

jun-15-2018 14-44-30

So I want to stay in centered mode for ever :)

@SrTobi
Copy link
Contributor

SrTobi commented Jun 15, 2018

Ah ok, makes sense 😄 As I see it, the layout of vscode is currently being reworked and so is the centered mode. It isn't even working on master right now. We'll have to wait and see.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug layout General VS Code workbench layout issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants