Enable tearing out tabs to create new windows#14935
Merged
Conversation
I think I have a vision for it now. 25 TODOs left.
It doesn't save or restore, but it does seem to not crash. 24 TODOs left
21 TODOs left
They weren't from the persisted JSON, but they did come back 21 TODOs left
… always work as expected 20 TODOs left
The trick is, that if you do this while debugging, the AppState will get persisted as you're stepping through, which can make debugging this really tricky
(cherry picked from commit 7bad8c9)
17 TODOs remain
16 TODOs
14 TODOs left
Down to just 10 TODOs left
…tor the WindowEmperor randomly now, which is no good
…e just dtor the WindowEmperor randomly now, which is no good" This reverts commit 7e91bdb.
3 TODOs left
2 TODOs left
…labeth # Conflicts: # src/cascadia/Remoting/Monarch.cpp # src/cascadia/Remoting/Monarch.h # src/cascadia/Remoting/Monarch.idl # src/cascadia/Remoting/Peasant.cpp # src/cascadia/Remoting/Peasant.h # src/cascadia/Remoting/Peasant.idl # src/cascadia/Remoting/WindowManager.cpp # src/cascadia/Remoting/WindowManager.h # src/cascadia/Remoting/WindowManager.idl # src/cascadia/TerminalApp/TerminalPage.cpp # src/cascadia/TerminalApp/TerminalPage.h # src/cascadia/TerminalApp/TerminalPage.idl # src/cascadia/TerminalApp/TerminalWindow.cpp # src/cascadia/TerminalApp/TerminalWindow.h # src/cascadia/TerminalApp/TerminalWindow.idl # src/cascadia/UnitTests_Remoting/RemotingTests.cpp # src/cascadia/WindowsTerminal/AppHost.cpp # src/cascadia/WindowsTerminal/AppHost.h
…nexpected-party # Conflicts: # src/cascadia/Remoting/Monarch.h # src/cascadia/Remoting/Monarch.idl # src/cascadia/Remoting/WindowManager.h # src/cascadia/Remoting/WindowManager.idl # src/cascadia/TerminalApp/TabManagement.cpp # src/cascadia/TerminalApp/TerminalPage.cpp # src/cascadia/TerminalApp/TerminalPage.h # src/cascadia/UnitTests_Remoting/RemotingTests.cpp
microsoft-github-policy-service
bot
added
Issue-Scenario
Area-UserInterface
DHowett
requested changes
Mar 30, 2023
| } | ||
| } | ||
|
|
||
| winrt::fire_and_forget TerminalPage::_onTabDroppedOutside(winrt::IInspectable sender, |
Member
There was a problem hiding this comment.
can you remind me why we have to stash for this? this seems totally selfcontained
Member
There was a problem hiding this comment.
apart from the position, but that's not the whole tab!
| // .TabLayout(). We can re-evaluate that as a part of TODO: GH#12633 | ||
| if (const auto& layout = LoadPersistedLayout()) | ||
| // Pass in information about the initial state of the window. | ||
| // * If we were suppost to start from serialized "content", do that, |
Member
There was a problem hiding this comment.
you marked as resolved, but the comment still shows as "suppost" for me?
| if (_contentBounds) | ||
| { | ||
| // If we've been created as a torn-out window, then we'll need to | ||
| // use that size instead. _contentBounds is in raw pixels. Huzzah! |
microsoft-github-policy-service
bot
added
Needs-Author-Feedback
carlos-zamora
approved these changes
Mar 30, 2023
Member
carlos-zamora
left a comment
carlos-zamora
left a comment
There was a problem hiding this comment.
Looks good to me. Any outstanding stuff will probably get caught when Dustin's review switches to an approve. Nice work!
DHowett
approved these changes
Mar 31, 2023
Member
Author
|
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This is the last one 🎉
Summary
In the final chapter of our tale, we present a PR of great significance. It grants the power to tear tabs from their windows and create a new window where they may be dropped, one not necessarily of the Terminal sort. The dimensions of the original window are transferred to this new abode, and its placement on the screen is determined by the user's placement of the tab.
This is the last main chapter of the tear-out saga, and the dawning of the new age.
Closes #5000
Related to #1256
Detailed description
We're really leaning on the existing
RequestNewWindowevent that the monarch already had - honestly, most of that was so simple that it could have just been in the parent PRs. We just need to add new support for passing in a content blob of json, and making sure the Terminal always uses that over commandline args. Easy enough.There's a bit of wackiness here in adjusting the positioning just right so that the new window appears in the right place, but it feels... pretty good all things considered. I'll grab a gif when my machine is less borked.