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.

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 dragging tabs between windows #14901

Merged
merged 277 commits into from
Mar 30, 2023
Merged

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Feb 23, 2023

Behold, the penultimate chapter in the saga of tear-out! This significant update bestows upon the user the power to transport tabs betwixt Terminal windows. Alas, the drag and drop capabilities of TabView are not yet refined, so this PR primarily concerns itself with the intricacies of plumbing. When a tab is extracted and deposited elsewhere, it is necessary to have the recipient make an inquiry to the Monarch, who in turn will beseech the sender to transmit the tab content, akin to the act of moving a tab. Curious it may seem, but the method has proven effective.

The penultimate tear-out PR. This PR enables the user to move tabs from one Terminal window to another. The TabView drag/drop APIs have some rough edges, so this PR is mostly plumbing. When a tab is drag/dropped, we need to get the recipient to ask the Monarch to ask the sender to send the tab content, like a MoveTab action. Wacky, but it works.

There's a LONG tail of UX gaps. Those I'm going to track in #14900. It is more valuable for us to merge this now than to figure out workarounds immediately.

The next PR will be the last main PR in this saga - in which we enable dragging a tab out of the window and dropping to create a new window.

Detailed description

As I mentioned, it's mostly plumbing. The order that we get tab drag events is... unfortunate... for our use case. So we do a lot of sending RequestReceiveContentArgs up and down between windows, just to communicate who the tab was dropped on to whomever the tab was dragged from.

There's a diagram for this that I originally put in #5000 (comment):

sequenceDiagram
    participant Source
    participant Target
    participant Monarch

    Note Left of Source: _onTabDragStarting
    Source --> Source: stash dragged content
    Source --> Source: pack window ID into DataPackage

    Source ->> Target: Drag tab
    Note right of Target: _onTabStripDragOver
    Target ->> Target: AcceptedOperation(DataPackageOperation::Move)
    
    Source --> Target: Release mouse (to drop)
    
    Note right of Target: _onTabStripDrop
    Target --> Target: get WindowID from DataPackage
    Target -) Monarch: Request that WindowID sends content to us<br>RequestRecieveContent
    Monarch -) Source: Tell to send content to Target.Id<br>RequestSendContent, SendContent
    Source --> Source: detach our content
    Source -) Monarch: RequestMoveContent(stashed, target.id)
    Monarch -) Target: AttachContent(stashed)

    # Target -->> Source: 
    # Note Left of Source: TabViewTabDragStartingEventArgs<br>.OperationCompleted
    # Note Left of Source: _onTabDroppedCompleted
Loading

Really really though, let's try to avoid nits about the UX at this time. This PR works with what we've got. Mail threads are percolating. I've got 19 chapters worth of Hobbit branch names to use for those follow ups.

  Doesn't work with multiple windows open, but doesn't do _nothing_
  Cherry-picked from 4b4e057
  It remains to be seen if this worked for movePane, but one step at a time
  It remains to be seen if this worked for movePane, but one step at a time

(cherry picked from commit c282797)
…op/3/akallabeth

# Conflicts:
#	src/cascadia/TerminalApp/TerminalPage.h
…ontrol was kept alive by listeners to events on the Core, who's now on another thread
  Basically, because we were projecting the event straight trhough the control, from core->page, basically when we would do a PasteRequested, we'd call the handler registered on the first control, then also the handler added by the second.
…t ended up fixing this bug.

  All we actually needed was to do a _FontSizeChangedHandlers() to set up the font size and the markers in the TermControl.
  TerminalPage is the thing that ends up expanding iterable Command. It does
  this largely with copies - it makes a new map, a new vector, copies the
  Commands over, and does the work there before setting up the cmdpal.

  Except, it's not making a copy of the Commands, it's making a copy of the
  vector, with winrt objects all pointing at the Command objects that are
  ultimately owned by CascadiaSettings.

  This doesn't matter if there's only one TerminalPage - we'll only ever do that once.

  If there's many, on different threads, then one tpage will end up expanding
  the subcommands of one Command while another tpage is ALSO iterating on those
  subcommands. Hence why I'm getting `hresult_changed_state`s
  Doesn't work with multiple windows open, but doesn't do _nothing_

(cherry picked from commit 427a4a5)
…ut this doesn't fix the crash

(cherry picked from commit 700aadc)
  TerminalPage is the thing that ends up expanding iterable Command. It does
  this largely with copies - it makes a new map, a new vector, copies the
  Commands over, and does the work there before setting up the cmdpal.

  Except, it's not making a copy of the Commands, it's making a copy of the
  vector, with winrt objects all pointing at the Command objects that are
  ultimately owned by CascadiaSettings.

  This doesn't matter if there's only one TerminalPage - we'll only ever do that once.

  If there's many, on different threads, then one tpage will end up expanding
  the subcommands of one Command while another tpage is ALSO iterating on those
  subcommands. Hence why I'm getting `hresult_changed_state`s

(cherry picked from commit 2122eec)
@DHowett
Copy link
Member

DHowett commented Mar 30, 2023

Once this retargets to main, build cycle required!

Base automatically changed from dev/migrie/oop/3/quenta-silmarillion to main March 30, 2023 14:37
…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
@zadjii-msft zadjii-msft enabled auto-merge (squash) March 30, 2023 14:57
@zadjii-msft zadjii-msft merged commit 9514c11 into main Mar 30, 2023
@zadjii-msft zadjii-msft deleted the dev/migrie/oop/3/akallabeth branch March 30, 2023 15:20
@patrikhuber
Copy link

Really cool to see the progress of this! May I ask, what's the approximate timeframe of seeing this in the Terminal if installed from the Windows Store?

@zadjii-msft
Copy link
Member Author

@patrikhuber It'll probably be a while. We did have to rewrite the entire process model of the Terminal to support this. This will likely result in a long tail of edge cases that we want to iron out before shipping. Fortunately, I think the bot is still working for releases, so when this does finally go out, the bot will post here (and in #1256)

@patrikhuber
Copy link

Thank you Mike, I understand! I might be tempted then to try the GitHub release when it's available :-).

Thanks a lot for all the amazing work on Windows Terminal!

@sylph520
Copy link

Finally, really looking forward to the release of this feature, thanks mate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for tab tearoff and tab merge
5 participants