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

add option to dock terminal at top #207721

Merged
merged 16 commits into from
Jul 8, 2024
Merged

Conversation

mxts
Copy link
Contributor

@mxts mxts commented Mar 14, 2024

for #201452

@mxts
Copy link
Contributor Author

mxts commented Mar 14, 2024

@microsoft-github-policy-service agree company="Avexsoft"

@mxts
Copy link
Contributor Author

mxts commented Mar 17, 2024

@abhijit-chikane would you be so kind to help with community PR approvals? thanks!

@meganrogge meganrogge assigned sbatten and unassigned meganrogge Mar 25, 2024
@mxts
Copy link
Contributor Author

mxts commented Apr 6, 2024

@sbatten would you be able to help? Thank you.

@mxts
Copy link
Contributor Author

mxts commented May 7, 2024

@sbatten can help? thanks

@mxts
Copy link
Contributor Author

mxts commented May 11, 2024

@benibenj @sbatten @meganrogge possible to give some feedback on this please?

I would have opt for an extension if it was possible to move the terminal but VSCode has a restriction here https://code.visualstudio.com/api/extension-capabilities/overview#no-dom-access

I use a 27" monitor, without this feature, I constantly have to look up and down from the top section to the lowest line on the screen and really causes strain on the neck. Thanks.

@benibenj benibenj assigned benibenj and unassigned sbatten May 15, 2024
@benibenj
Copy link
Contributor

Have you already tried moving the terminal into the editor? This would allow you to place the terminal at the top.
image

@mxts
Copy link
Contributor Author

mxts commented May 15, 2024

@benibenj how do you do that? I tried dragging with the mouse and it would not work.

Shows a no entry sign
image

Also, would it activate with ctrl+~?

@mxts
Copy link
Contributor Author

mxts commented May 15, 2024

@ agree

@mxts
Copy link
Contributor Author

mxts commented May 15, 2024

@ agree company="Avexsoft"

@benibenj
Copy link
Contributor

benibenj commented May 15, 2024

Right click on one of the terminal instances.

image

If you only have one terminal instance you can just left click on it in the panel bar.

image

There also is a command Terminal: Create New Terminal in Editor Area

@mxts
Copy link
Contributor Author

mxts commented May 15, 2024

Thanks, managed to move it, but ctrl+~ does not activate it, it opens a new one at the bottom😅

I mean, it isn't just the terminal, it's everything in the panel, i.e. problems, output etc, terminal is just the most frequently used.

@mxts
Copy link
Contributor Author

mxts commented May 17, 2024

@benibenj i gave your suggestions a shot, but it created other problems, is a merge possible? Thank you.

@mxts
Copy link
Contributor Author

mxts commented May 27, 2024

@benibenj what do you think? thanks

@benibenj
Copy link
Contributor

I'll have a look at this in June and discuss with the team if this is something we want to support

@benibenj
Copy link
Contributor

benibenj commented Jul 8, 2024

There is still a couple things missing:

  • Panel Border bottom when it is at the top:
image

=> in panelpart.css: add border bottom width 1 when panel at top

=> in panelPart.ts: update styles should set container.style.borderBottomColor and layout should remove 1px of the height when at top

  • Show correct maximize icon (should point to bottom when panel at top):
image
  • When changing the alignment of the panel, the panel is drawn at the bottom
    Code_-_OSS_A8vOAfz5ih

  • When reloading the panel is rendered at the bottom

I have some smaller changes I would like to do, but somehow it does not allow me to push commits to this PR so I might do them after we merge this PR.

  • Run Find All References on Position.BOTTOM, there seems to be some checks regarding bottom position when maximizing which is not considering the top position. Maybe they should?

@benibenj
Copy link
Contributor

benibenj commented Jul 8, 2024

@mxts would you be open to grant me permissions to push commits to this PR? This would allow me to help you out with some of the required changes. There should be a button called Allow edits from maintainers

@mxts
Copy link
Contributor Author

mxts commented Jul 8, 2024

Certainly! But how do I do that? I'm trying to look for allow maintainers to edit

@benibenj
Copy link
Contributor

benibenj commented Jul 8, 2024

@mxts
Copy link
Contributor Author

mxts commented Jul 8, 2024

image

I just don't have it, an alternative is that i invited to avexsoft.. maybe then u can commit to it.

@benibenj
Copy link
Contributor

benibenj commented Jul 8, 2024

I guess there is not mention of it further down in the sidebar? I just found online that it would be bellow the list of participants

image image

@mxts
Copy link
Contributor Author

mxts commented Jul 8, 2024

image

Nothing

@benibenj
Copy link
Contributor

benibenj commented Jul 8, 2024

That's weird. I've never seen this before. It's telling me I do not have permission to push to the PR. Possibly because the branch belongs to an organisation. I might have to create a fork to be able to make some changes

@mxts
Copy link
Contributor Author

mxts commented Jul 8, 2024

Ok, let me know if there is any settings I can change.

@benibenj
Copy link
Contributor

benibenj commented Jul 8, 2024

Let's try this: I created a PR with my changes against your fork. If you review and approve it, it will be part of this PR.
Please also give it a try and make sure it works as expected

avexsoft#1

@mxts
Copy link
Contributor Author

mxts commented Jul 8, 2024

I have merged the changes, but have some trouble compiling VSCode... let me try and sort it out.

@benibenj
Copy link
Contributor

benibenj commented Jul 8, 2024

I approved it. This will allow the pipeline to run. Let me know after you tested and are happy with the behaviour. If all tests pass and you agree with my changes, we can merge the PR :)

@Tyriar Tyriar self-assigned this Jul 8, 2024
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

terminal/ looks good 👏

@mxts
Copy link
Contributor Author

mxts commented Jul 8, 2024

yes @benibenj please merge thanks!

@benibenj benibenj merged commit 63274b6 into microsoft:main Jul 8, 2024
6 checks passed
aaronchucarroll pushed a commit to aaronchucarroll/vscode that referenced this pull request Jul 10, 2024
* add option to dock terminal at top

* fixed formatting

* Update editorPart.ts

requested changes

* panel position fixes and cleanup

---------

Co-authored-by: BeniBenj <besimmonds@microsoft.com>
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Aug 22, 2024
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.

9 participants