microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.1k stars 29.26k forks source link

Cannot "Hide Terminal" #141405

Open bpasero opened 2 years ago

bpasero commented 2 years ago

Testing #141357

image

I had dragged the outline view into the side panel where terminal was and would have expected I can now "Hide Terminal".

sbatten commented 2 years ago

@Tyriar do you do something to prevent hiding in any case?

Tyriar commented 2 years ago

Not intentionally at least, no idea how I would communicate that to the layout code.

Tyriar commented 2 years ago

@sbatten the cause is here:

https://github.com/microsoft/vscode/blob/a30e4b2b7651ecce46e85f93a4207afd67cd1e20/src/vs/workbench/contrib/terminal/browser/terminalGroupService.ts#L79-L83

I don't remember adding that.

sbatten commented 2 years ago

@Tyriar thanks, i think we can remove the restriction on view container location, what do you think?

Tyriar commented 2 years ago

@sbatten actually that might not be the cause after thinking on it more, I think that drives ensuring the view doesn't get closed if you have a setup like problems next to terminal. Would be a pain to manually show it again?

This might be it?

https://github.com/Microsoft/vscode/blob/74e7864370241df106ff7a330a8aaf2a3b6735a5/src/vs/workbench/contrib/terminal/browser/terminal.contribution.ts#L110

sbatten commented 2 years ago

@Tyriar I actually thought the last comment was on a separate issue about deleting the last terminal... I still think that code is relevant for that fix. For this issue, the new code is likely it. I don't recall why this would be that way.

sbatten commented 2 years ago

looks like a mistake

image
Tyriar commented 2 years ago

@sbatten might have been to try prevent the user from hiding the terminal and not bring it back. I think the restriction should go, I'll remove it.

Tyriar commented 2 years ago

@sbatten I just hid the terminal panel below and i don't think there's any way of bringing it back outside running a command/keybinding. Was it was added to protect against this case?

Screen Shot 2022-01-25 at 9 56 14 am

Hiding the panel/view works, it's just not when it's in a container view or whatever it's called.

sbatten commented 2 years ago

Ah, we don't surface the views on the panel's composite bar context menu so it gets lost unless there are multiple views. This is not a regression and all built-in panel's are not toggle-able at the moment, so I will punt this to february.