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

[WIP] Configurable Panel Position #34267

Closed
wants to merge 1 commit into from
Closed

[WIP] Configurable Panel Position #34267

wants to merge 1 commit into from

Conversation

AkashGutha
Copy link
Contributor

Let's make the Bottom panel position configurable to bottom or side-by-side.

Also, integrate it as an editor tab which can be a with tabs carry arranged with tabs when set to side-by-side mode.

@mention-bot
Copy link

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

@AkashGutha
Copy link
Contributor Author

@isidorn let's get this done in two iterations.

@isidorn isidorn self-assigned this Sep 13, 2017
@isidorn
Copy link
Contributor

isidorn commented Sep 13, 2017

@AkashGutha thanks for showing interest in this. In order to tackle this we need to come up with a solution that looks good both horizontaly and verticaly. Notice that we currently have tabs on top of the panel - how would those tabs look in reduced vertical space? We would probably need some overflow design.
In your comment you are suggesting that the panel should be treated the same way as any editor with regards to layout. However I think that would only complicate our life and would not provide big benefits to the user. VS Code has limited flexibility when it comes to layouting and I believe the right approach would be to let the Panel be either at the bottom or to the very right.

My proposal is that at the top of the panel we would introduce an action that toggles the dock position (similar to what Chrome dev tools have, but we would only have 2 states to start). That action would just switch a boolean in the layout.ts and depending on that value the layout would either layout the panel horizontally to the bottom or vertically to the side.

If we end up with a solution that does not look or feel good we will have to reject it.

Let me know what you think.

fyi @stevencl @bpasero

dock

@AkashGutha
Copy link
Contributor Author

@isidorn Yes, definitely like the chrome inspect tab.
is there a specific person that is can to talk about existing implementation of the SashLayout.

And for the reduced horizontal space, how about using a combo box just like the terminal.
The only caveat I see is confusion when using it in terminal mode, where you might end up having two combo boxes sitting next to each other. Having said that, it may be intuitive to most of the devs.

@isidorn
Copy link
Contributor

isidorn commented Sep 13, 2017

@AkashGutha you can talk to me about the SashLayout and about any other questions you may have. If I do not know something I will find out.
But I think there is already a vertical Sash which you can use. So idealy this would require no changes to the Sash.

As for the reduced horizontal space, I think we should tackle that in step2. But showing some overflow as we do in the activity bar makes sense to me.

activityoverflow

@isidorn
Copy link
Contributor

isidorn commented Sep 13, 2017

Also any changes you make to vscode should be easy to just try out. Just have continues build running and run vscode out of source as described here.

@isidorn
Copy link
Contributor

isidorn commented Sep 13, 2017

fyi @egamma

@Tyriar
Copy link
Member

Tyriar commented Oct 2, 2017

integrate it as an editor tab

We should not include this in this change, there is a larger discussion that needs to happen on whether to pull parts of the panel into an editor tab.

similar to what Chrome dev tools have, but we would only have 2 states to start

This 👍

@heisian
Copy link

heisian commented Oct 2, 2017

what about embedding a terminal into an editor tab (after splitting)? seems a bit overkill to have to redo so much of the UI just for this.

i.e., i have a single file open. then I run split tab. in the new split on the right pane I can 'convert' that tab into a terminal.

@ngryman
Copy link

ngryman commented Oct 3, 2017

@Tyriar @heisian Please don't use editor tabs for this.

seems a bit overkill to have to redo so much of the UI just for this

I don't think so, here is why...

An editor tab is made for editing source files, and only that.

A Terminal has nothing to do with that. A terminal should be differentiated because its usage / life span / side-effects are obviously different. It generally stays alive during the whole session, running external processes and should be easily toggleable for quick access.

Embedding it in an editor tab would make this typical use case very tedious to achieve. I've used Atom a lot, and many extensions use this editor tab pattern. It ends up in a catastrophic user experience where you constantly have to maintain a splitted region where you put all your other stuff. Of course this region is not toggleable, so it basically eats a whole part of the screen for things you only get to access 10% of the time.

It's like mixing apples and oranges.

The current way of presenting the terminal is very good, a toggleable pane. It only needs a way to dock itself to other locations. Lots of editors (including Visual Studio) have done this for decades, for a good reason I believe.

@heisian
Copy link

heisian commented Oct 3, 2017

@ngryman good points - was merely suggesting a shortcut to the actual solution & agree it can get pretty problematic. plus anything that brings things closer to the way atom does things (and I loved & used it for years) could be a bad thing :P

@Tyriar
Copy link
Member

Tyriar commented Oct 3, 2017

It ends up in a catastrophic user experience where you constantly have to maintain a splitted region where you put all your other stuff.

@ngryman this is basically my stance on the topic; it's a lot easier to manage the terminal when there's only a single place for it and all commands act on that single area. This combined with the ability to split a terminal instance inside the panel #7504 is where I would like to see this go.

@simoniz0r
Copy link

simoniz0r commented Oct 9, 2017

it's a lot easier to manage the terminal when there's only a single place for it and all commands act on that single area. This combined with the ability to split a terminal instance inside the panel #7504 is where I would like to see this go.

IMO, that's not really a good enough solution for this. Sometimes I like to use VSCode in a window that's much wider than it is tall, and it would be really nice if the terminal could be on one of the sides. This, IMO, is not really usable at all:

image

It'd be really awesome if the terminal could be in all of that wasted space to the right and not taking up half of my view at the bottom. Something similar to how kdevelop allows its konsole tab to be placed on either the sides or the bottom would be really nice.

image

@heisian
Copy link

heisian commented Oct 9, 2017

@simoniz0r i'm pretty sure that's exactly what this pull request is about doing.

@ngryman
Copy link

ngryman commented Oct 9, 2017

@heisian @Tyriar I think we all agree on the features of this PR. How could we speed things up? Are there remaining points to clarify?

@AkashGutha Are you still working on this? Do you need some help?

@AkashGutha
Copy link
Contributor Author

@ngryman I'm quite busy with a few things. it will probably take one or two months before I even get started on this.
If anyone is interested they can take over this. :)

@ngryman
Copy link

ngryman commented Oct 10, 2017

@AkashGutha Alright thanks for the explanation.

@Tyriar @heisian Any pointers in docs/code to help me understand how it's done now and some ideas on how to implement this? I'll be happy to address this but I would need some insights first.

@Tyriar
Copy link
Member

Tyriar commented Oct 10, 2017

@isidorn will be the main contact on this PR

@isidorn
Copy link
Contributor

isidorn commented Oct 10, 2017

There are some prerequests before this can be done, namely #31503
Once that is tackled we will invest into this. As for code pointers checkout my initial comment

@AkashGutha
Copy link
Contributor Author

@ngryman if you check the file changes in this PR. you'll find the exact location of the terminal layout implementation and a starting point on how we can convert the existing Sash into the way we want it to function.
in the repo the file is at this location
src/vs/workbench/browser/layout.ts

Currently the Sash implementation is fixed to have one in Y and on in X.
SashY is used for the file menu/git/find et etc
SashX is used for the terminal.

We have to remove the SashX as a tight dependecy for the terminal and make it usable by both X and Y.

@isidorn correct me / add to this if we can find any other useful information.

@heisian
Copy link

heisian commented Oct 10, 2017

i was poking around in terminal.ts (found in two locations) as well as search.ts to get a sense of how both initialize themselves in their respective locations. still need to put more time into it. also glanced at @AkashGutha 's changes. will take a look at the PR @isidorn mentioned soon.

@ngryman
Copy link

ngryman commented Oct 10, 2017

Thanks guys. So, wrapping up.

To do:

  • Add a dock positions action, bottom and right for now.
  • Handle tabs overflow like in activity bar.

Questions:

  • How do we handle the case when the sidebar is put right? Do we swap the panel left or do we stack it right?

@ngryman
Copy link

ngryman commented Oct 14, 2017

I've started to dive into the code and the layout part could use a refactoring so it's less cryptic. Are you planning to do so?

@AkashGutha
Copy link
Contributor Author

@ngryman go ahead with the changes. we need those changes desperately.

@isidorn
Copy link
Contributor

isidorn commented Oct 16, 2017

@ngryman thanks for looking into the code, however I would recommend that you all wait a bit since I am looking into some feature which is a prerequest for this. Once that is done I will dive into this one. Refactoring the layout part that it still has the same functionality but is less cryptic we could accept as a seperate PR. Though before submiting code for that I would recommend you creata a PR where we first discuss what you plan to do.

@ngryman
Copy link

ngryman commented Oct 16, 2017

@isidorn Gotcha, I guess the layout code would need some cleanup before we had more complexity to it. I'll be glad to discuss about it in another PR.

@isidorn
Copy link
Contributor

isidorn commented Oct 23, 2017

@chryw I am slowly starting to look into this feature. Could you maybe look into the icon for this, as you can see in my comment from above we would need two different icons, one that reprsents moving the panel to the right and one to be bottom. Both for light and dark version.

I have started with RightColumnOfTwoColumns_16x and BottomRowOfTwoRows_16x from the VSicons, but not sure if they fit perfectly. Also there is none for the dark theme

panelposition

@ngryman
Copy link

ngryman commented Oct 23, 2017

@isidorn Thank you so much to address this one. I personally did not have time to dig into this lately, and the time I climb that learning curve, you'll have finished this PR and have 2 kids 😂

@chryw
Copy link
Contributor

chryw commented Oct 24, 2017

@isidorn I think those icons make sense. I attached VS Code version svg files.
PanelPositionIcons_vscode.zip

@isidorn
Copy link
Contributor

isidorn commented Oct 25, 2017

@chryw awesome, thanks

@isidorn
Copy link
Contributor

isidorn commented Oct 25, 2017

Closing this in favor of #36827

@isidorn isidorn closed this Oct 25, 2017
@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.

None yet

8 participants