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.

Sign up for GitHub

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

Jump to bottom

Certain pane zoom operations make the terminal go blank #7252

Closed
zadjii-msft opened this issue Aug 11, 2020 · 4 comments · Fixed by #7973
Closed

Certain pane zoom operations make the terminal go blank #7252

zadjii-msft opened this issue Aug 11, 2020 · 4 comments · Fixed by #7973
Assignees
@zadjii-msft
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Milestone
Terminal v1.5

Comments

@zadjii-msft
Copy link
Member

  • open two panes
  • zoom the first one (definitely repros with cmdpal)
  • exit

expected:

  • just the second pane is visible

actual:

  • the first pane is removed from the UI tree, and the second is never re-attached.
@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. Priority-1 A description (P1) labels Aug 11, 2020
@zadjii-msft zadjii-msft self-assigned this Aug 11, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 11, 2020
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 12, 2020
@zadjii-msft zadjii-msft added this to the Terminal v1.4 milestone Aug 26, 2020
@DHowett DHowett changed the title Closing a zoomed pane makes the terminal go black Closing a zoomed pane makes the terminal go blank Aug 26, 2020
@DHowett DHowett pinned this issue Aug 26, 2020
@DHowett DHowett changed the title Closing a zoomed pane makes the terminal go blank Certain pane zoom operations makee the terminal go blank Aug 26, 2020
@DHowett
Copy link
Member

DHowett commented Aug 26, 2020

@cinnamon-msft reported that she hit this all the time during her GIF recording, and we yanked the announce for it. I've pinned this issue in case users run into it 😄

@DHowett DHowett changed the title Certain pane zoom operations makee the terminal go blank Certain pane zoom operations make the terminal go blank Aug 26, 2020
@carlos-zamora carlos-zamora mentioned this issue Sep 14, 2020
Merged
DHowett pushed a commit that referenced this issue Sep 18, 2020
@carlos-zamora
[1.3 STABLE ONLY] Unbind togglePaneZoom (#7630)
9767abd 
Disable `togglePaneZoom` as a binding. Remove it from defaults.json and the schema.

## References
#7252 - re-enable this binding when this blocking bug is resolved

Co-authored-by: Carlos Zamora <cazamor@microsoft.com>
@jjspace jjspace mentioned this issue Oct 2, 2020
Closed
@PankajBhojwani PankajBhojwani unpinned this issue Oct 6, 2020
@PankajBhojwani PankajBhojwani pinned this issue Oct 6, 2020
@zadjii-msft
Copy link
Member Author

OKAY I see what's happening here. The ActivePaneChanged handler in TerminalPagedoesn't re-attach the tab content to the tree, it just updates hte title of thewindow.

So when the pane is exited, the pane's control is removed and re-attached to the parent grid, which isn't in the XAML tree. And no one can go tell the TerminalPage that it needs to re-setup the tab content again.

The Page manually does this in a few places, when various pane actions are about to take place, it'll unzoom. It would be way easier if the Tab could just manage the content of the page.

Or if the Tab just had a Content that was observable, that when that changed, the page would auto readjust. That does sound like a LOT of work though.

@zadjii-msft zadjii-msft mentioned this issue Oct 20, 2020
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Oct 20, 2020
@ghost ghost closed this as completed in #7973 Oct 21, 2020
ghost pushed a commit that referenced this issue Oct 21, 2020
@zadjii-msft
Fix exiting a zoomed pane (#7973)
ccf9f03 
## Summary of the Pull Request

Fixes the bug where `exit`ing inside a closed pane would leave the Terminal blank.

Additionally, removes `Tab::GetRootElement` and replaces it with the _observable_ `Tab::Content`. This should be more resilient in the future.

Also adds some tests, though admittedly not for this exact scenario. This scenario requires a cooperating TerminalConnection that I can drive for the sake of testing, and _ain't nobody got time for that_.

## References
* Introduced in #6989 

## PR Checklist
* [x] Closes #7252
* [x] I work here
* [x] Tests added/passed 🎉 
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

From notes I had left in `Tab.cpp` while I was working on this:
```
OKAY I see what's happening here the ActivePaneChanged Handler in TerminalPage
doesn't re-attach the tab content to the tree, it just updates the title of the
window.

So when the pane is `exit`ed, the pane's control is removed and re-attached to
the parent grid, which _isn't in the XAML tree_. And no one can go tell the
TerminalPage that it needs to re set up the tab content again.

The Page _manually_ does this in a few places, when various pane actions are
about to take place, it'll unzoom. It would be way easier if the Tab could just
manage the content of the page.

Or if the Tab just had a Content that was observable, that when that changed,
the page would auto readjust. That does sound like a LOT of work though.
```

## Validation Steps Performed

Opened panes, closed panes, exited panes, zoomed panes, moved focus between panes, panes, panes, panes
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Oct 21, 2020
@DHowett DHowett unpinned this issue Oct 22, 2020
DHowett pushed a commit that referenced this issue Oct 27, 2020
@zadjii-msft @DHowett
Fix exiting a zoomed pane (#7973)
8a758f7 
Fixes the bug where `exit`ing inside a closed pane would leave the Terminal blank.

Additionally, removes `Tab::GetRootElement` and replaces it with the _observable_ `Tab::Content`. This should be more resilient in the future.

Also adds some tests, though admittedly not for this exact scenario. This scenario requires a cooperating TerminalConnection that I can drive for the sake of testing, and _ain't nobody got time for that_.

* Introduced in #6989

* [x] Closes #7252
* [x] I work here
* [x] Tests added/passed 🎉
* [n/a] Requires documentation to be updated

From notes I had left in `Tab.cpp` while I was working on this:
```
OKAY I see what's happening here the ActivePaneChanged Handler in TerminalPage
doesn't re-attach the tab content to the tree, it just updates the title of the
window.

So when the pane is `exit`ed, the pane's control is removed and re-attached to
the parent grid, which _isn't in the XAML tree_. And no one can go tell the
TerminalPage that it needs to re set up the tab content again.

The Page _manually_ does this in a few places, when various pane actions are
about to take place, it'll unzoom. It would be way easier if the Tab could just
manage the content of the page.

Or if the Tab just had a Content that was observable, that when that changed,
the page would auto readjust. That does sound like a LOT of work though.
```

Opened panes, closed panes, exited panes, zoomed panes, moved focus between panes, panes, panes, panes

(cherry picked from commit ccf9f03)
@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #7973, which has now been successfully released as Windows Terminal v1.4.3141.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #7973, which has now been successfully released as Windows Terminal Preview v1.5.3142.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Milestone
Terminal v1.5
Development

Successfully merging a pull request may close this issue.

Fix exiting a zoomed pane microsoft/terminal
3 participants
@DHowett @zadjii-msft @cinnamon-msft