Add support for moving panes and tabs between windows #14866
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
…ing AppLogic that I hate so I'm gonna start over
This reverts commit a5255ba.
We'll need this for #5000, for ainulindale. This refactoring will be annoying enough as it is so we may as well do it as a first, separate PR.
…ns though, so it immediately exits
…ndale # Conflicts: # src/cascadia/WindowsTerminal/AppHost.h
…XAML island is started. I think this can work in the parent at least
…It launches and crashes immediately. We'll keep shuffling code.
At this point, I determined that I would need to make some big changes to AppHost and decided that it was time to commit before moving on.
…pressive It exits itself after 30s, but hey it worked
cherry-picked from 9870eb1
There's a lot of renaming, signature changes here. But StartupActions is a good mechanism for passing panes around, more or less. cherry-picked from cfe879d
…cess boundary cherry-picked from 1f2bb76
but it doesn't render, and the old pane keeps on choochin. And it only works with a window name, not and ID And the `Dispatcher` definitely needs to get re-wired for the new thread, cause dragging it across a DPI boundary (aka, resize) crashes the window
This is basically the comment from aaefdf4. The other comment wasn't relevant anymore
This reverts commit 0a6c21f.
lhecker
reviewed
Mar 28, 2023
lhecker
reviewed
Mar 28, 2023
DHowett
reviewed
Mar 28, 2023
(cherry picked from commit 3152ff9)
(cherry picked from commit 3f5a559)
….com/microsoft/terminal into dev/migrie/oop/3/quenta-silmarillion
DHowett
approved these changes
Mar 29, 2023
|
Pass audit too! |
|
And fix |
DHowett
reviewed
Mar 30, 2023
| @@ -39,8 +39,17 @@ namespace winrt::TerminalApp::implementation | |||
| return it != _content.end() ? it->second : ControlInteractivity{ nullptr }; | |||
| } | |||
|
|
|||
| void ContentManager::_closedHandler(winrt::Windows::Foundation::IInspectable sender, | |||
| winrt::Windows::Foundation::IInspectable e) | |||
| void ContentManager::Detach(const Microsoft::Terminal::Control::TermControl& control) | |||
There was a problem hiding this comment.
Part of me feels like we should only use ContentIds here in ContentManager. There's a crossing of concerns, giving ContentManager access (in any form!) to non-free-threaded UI objects!
There was a problem hiding this comment.
But... what does this function do? It looks like it doesn't need to be inside the content manager at all, since it mostly operates on control and never returns the detached content or even confirms whether it did detach anything..!
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.
Lo! Harken to me, for I shall divulge the heart of the tab tear-out saga. Verily, this PR shall bestow upon thee the power to move tabs and panes between windows by means of pre-defined actions. Though be warned, it does not yet grant thee the power to drag and drop them as thou mayest desire. Yet, the same plumbing that underpins this work shall remain steadfast. Behold, the majority of this undertaking concerns the elevation of the RequestMoveContent event from the TerminalPage to the very summit of the Monarch. From thence, a great AttachContent method shall descend back to the lowest depths. Furthermore, there are minor revisions to TermControl that shall enable thee to better detach the content and attach it to a new one.
This is the most important part of the tab tear-out saga. This PR enables the user to move tabs and panes between windows using pre-defined actions. It does not enable the user to drag/drop them yet, but the same fundamental plumbing will still apply. Most of the PR is plumbing the
RequestMoveContentevent up from theTerminalPageup to theMonarch, and then plumbing anAttachContentmethod back down. There are also small changes toTermControlto better support detaching the content and attaching to a new one.For testing, I recommend:
{ "keys": "f1", "command": { "action": "moveTab", "window": "1" } }, { "keys": "f2", "command": { "action": "moveTab", "window": "2" } }, { "keys": "f3", "command": { "action": "movePane", "window": "1" } }, { "keys": "f4", "command": { "action": "movePane", "window": "2" } }, { "keys": "shift+f3", "command": { "action": "movePane", "window": "1", "index": 3 } }, { "keys": "shift+f4", "command": { "action": "movePane", "window": "2", "index": 3 } },