-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Enable tearing out tabs to create new windows #14935
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
} | ||
} | ||
|
||
winrt::fire_and_forget TerminalPage::_onTabDroppedOutside(winrt::IInspectable sender, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remind me why we have to stash for this? this seems totally selfcontained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note as followup!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Any outstanding stuff will probably get caught when Dustin's review switches to an approve. Nice work!
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
RequestNewWindow
event 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.