Add a togglePaneZoom action for zooming a pane
#6989
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…es, but it works _pretty_ well
…om-zoom # Conflicts: # src/cascadia/TerminalApp/Tab.cpp
ghost
added
Area-UserInterface
zadjii-msft
commented
Aug 5, 2020
Sure bot, whatever you say
|
You owe me tests on all of these commands as soon as I bootstrap the new Helix testing. |
miniksa
approved these changes
Aug 6, 2020
There was a problem hiding this comment.
Not blocking over the one comment I have below.
It'd also be nice if we had an animation. Idk if you want that to be a part of #1001 though. If you do punt on the animation, could you just add a note on that issue (or open a new one).
I love the zoom icon in the tab :) |
|
FWIW: i prefer maximize/restore. we are almost an MDI right now. if we ever add the pane sub-titlebar people occasionally ask for, it will have the |
|
My only qualm with that is we need an action that just "toggles" the state. |
This was referenced Aug 7, 2020
|
@carlos-zamora animation moved to #7216 |
carlos-zamora
approved these changes
Aug 7, 2020
DHowett
changed the title
togglePaneZoom action for zooming a panetogglePaneZoom action for zooming a pane
ghost
pushed a commit
that referenced
this pull request
Aug 13, 2020
This is a minor fix from #6989. If there's only one pane in the Terminal, then we'd still "zoom" it and give it a border, but all the borders would be black. A single pane is already "zoomed", so it doesn't really make sense to try and zoom if there's only one.
ghost
pushed a commit
that referenced
this pull request
Aug 20, 2020
#6989 forgot to add `togglePaneZoom` to the schema, so this does that. WHILE I'M HERE: * The action names in the schema and the actual source were both in _random_ order, so I sorted them alphabetically. * I also added an unbound `togglePaneZoom` command to defaults.json, so users can use that command from the cmdpal w/o binding it manually.
MichelleTanPY
pushed a commit
to MichelleTanPY/terminal
that referenced
this pull request
Aug 20, 2020
…osoft#7346) microsoft#6989 forgot to add `togglePaneZoom` to the schema, so this does that. WHILE I'M HERE: * The action names in the schema and the actual source were both in _random_ order, so I sorted them alphabetically. * I also added an unbound `togglePaneZoom` command to defaults.json, so users can use that command from the cmdpal w/o binding it manually.
14 tasks
|
🎉 Handy links: |
ghost
mentioned this pull request
ghost
pushed a commit
that referenced
this pull request
Oct 21, 2020
## 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
DHowett
pushed a commit
that referenced
this pull request
Oct 27, 2020
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)
|
🎉 Handy links: |
|
🎉 Handy links: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
This PR adds the
togglePaneZoomaction, which can be used to make a pane expand to fill the entire contents of the window.References
PR Checklist
Additional comments
FOR DISCUSSION
Validation Steps Performed
Zoomed in and out a bunch. Tried closing panes while zoomed. Tried splitting panes while zoomed. Etc.