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

[fix-panel-auto-maximize] removes condition which contraints panelHeight #41800

Merged

Conversation

shobhitchittora
Copy link
Contributor

@shobhitchittora shobhitchittora commented Jan 18, 2018

Closes: #41625

Checklist -

  • No Lint errors
  • Tests pass
  • Hand tested the fix

@shobhitchittora
Copy link
Contributor Author

@isidorn I removed the condition which was creating the problem ( as below ). Why was it added in the first place? Anything I missed?

https://github.com/Microsoft/vscode/blob/0bddf58ab12619c49cfb0cac3b24f0fa23bfe91c/src/vs/workbench/browser/layout.ts#L471

@isidorn
Copy link
Contributor

isidorn commented Jan 18, 2018

@shobhitchittora thanks for the PR. The check was there if the panel got explictly maximized(using the action) that it shuold stay maximized. Though I prefer that we remove this, so your PR is fine.
However now the previousMaxPanelHeight variable is not used at all so make sure to remove it a couple of lines above.

@isidorn isidorn added this to the January 2018 milestone Jan 18, 2018
@isidorn isidorn added the layout General VS Code workbench layout issues label Jan 18, 2018
@shobhitchittora
Copy link
Contributor Author

shobhitchittora commented Jan 18, 2018

@isidorn Thanks for pointing that out. Just curious! Shouldn't the unused var be flagged by eslint in the editor itself and not allow me to push? Instead It was showing up as (green underlined) -

screen shot 2018-01-19 at 4 57 57 am

What are your views on this?

PS: regarding the issue I also checked with triggering window.action.toggleMaximizedPanel and it works ✨.

@isidorn
Copy link
Contributor

isidorn commented Jan 19, 2018

@shobhitchittora I get tslint errors the way I have vscode setup and that is how I found this. Not sure why you did not see it though

Thanks for the PR 🍻

@isidorn isidorn merged commit 1c4fbc9 into microsoft:master Jan 19, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
layout General VS Code workbench layout issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminal panel automatically maximized
2 participants