Skip to content

Commit

Permalink
Enable dragging tabs between windows (#14901)
Browse files Browse the repository at this point in the history
_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.


* Closes #1256
* Related to #5000
* Follow-ups get to go in #14900 


## 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):

```mermaid  
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
```

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.
  • Loading branch information
zadjii-msft authored Mar 30, 2023
1 parent 17a5b77 commit 9514c11
Show file tree
Hide file tree
Showing 21 changed files with 364 additions and 14 deletions.
35 changes: 35 additions & 0 deletions src/cascadia/Remoting/Monarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1109,4 +1109,39 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// drag/drop)
}
}

// Very similar to the above. Someone came and told us that they were the target of a drag/drop, and they know who started it.
// We will go tell the person who started it that they should send that target the content which was dragged.
void Monarch::RequestSendContent(const Remoting::RequestReceiveContentArgs& args)
{
TraceLoggingWrite(g_hRemotingProvider,
"Monarch_SendContent_Requested",
TraceLoggingUInt64(args.SourceWindow(), "source", "The window which started the drag"),
TraceLoggingUInt64(args.TargetWindow(), "target", "The window which was the target of the drop"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));

if (auto senderPeasant{ _getPeasant(args.SourceWindow()) })
{
senderPeasant.SendContent(args);

TraceLoggingWrite(g_hRemotingProvider,
"Monarch_SendContent_Completed",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));
}
else
{
TraceLoggingWrite(g_hRemotingProvider,
"Monarch_SendContent_NoWindow",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));

// TODO GH#5000
//
// In the case where window couldn't be found, then create a window
// for that name / ID. Do this as a part of tear-out (different than
// drag/drop)
}
}
}
1 change: 1 addition & 0 deletions src/cascadia/Remoting/Monarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
Windows::Foundation::Collections::IVector<winrt::hstring> GetAllWindowLayouts();

void RequestMoveContent(winrt::hstring window, winrt::hstring content, uint32_t tabIndex);
void RequestSendContent(const Remoting::RequestReceiveContentArgs& args);

TYPED_EVENT(FindTargetWindowRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs);
TYPED_EVENT(ShowNotificationIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/Remoting/Monarch.idl
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ namespace Microsoft.Terminal.Remoting
Windows.Foundation.IReference<UInt64> WindowID;
}

[default_interface] runtimeclass QuitAllRequestedArgs
{
[default_interface] runtimeclass QuitAllRequestedArgs {
QuitAllRequestedArgs();
Windows.Foundation.IAsyncAction BeforeQuitAllAction;
}
Expand Down Expand Up @@ -71,6 +70,7 @@ namespace Microsoft.Terminal.Remoting
Windows.Foundation.Collections.IVector<String> GetAllWindowLayouts();

void RequestMoveContent(String window, String content, UInt32 tabIndex);
void RequestSendContent(RequestReceiveContentArgs args);

event Windows.Foundation.TypedEventHandler<Object, FindTargetWindowArgs> FindTargetWindowRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> ShowNotificationIconRequested;
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/Remoting/Peasant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "Peasant.g.cpp"
#include "../../types/inc/utils.hpp"
#include "AttachRequest.g.cpp"
#include "RequestReceiveContentArgs.g.cpp"

using namespace winrt;
using namespace winrt::Microsoft::Terminal;
Expand Down Expand Up @@ -327,4 +328,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
}
return args->WindowLayoutJson();
}

void Peasant::SendContent(const Remoting::RequestReceiveContentArgs& args)
{
_SendContentRequestedHandlers(*this, args);
}
}
17 changes: 17 additions & 0 deletions src/cascadia/Remoting/Peasant.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "Peasant.g.h"
#include "RenameRequestArgs.h"
#include "AttachRequest.g.h"
#include "RequestReceiveContentArgs.g.h"

namespace RemotingUnitTests
{
Expand All @@ -25,6 +26,19 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_TabIndex{ tabIndex } {};
};

struct RequestReceiveContentArgs : RequestReceiveContentArgsT<RequestReceiveContentArgs>
{
WINRT_PROPERTY(uint64_t, SourceWindow);
WINRT_PROPERTY(uint64_t, TargetWindow);
WINRT_PROPERTY(uint32_t, TabIndex);

public:
RequestReceiveContentArgs(const uint64_t src, const uint64_t tgt, const uint32_t tabIndex) :
_SourceWindow{ src },
_TargetWindow{ tgt },
_TabIndex{ tabIndex } {};
};

struct Peasant : public PeasantT<Peasant>
{
Peasant();
Expand Down Expand Up @@ -52,6 +66,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
winrt::Microsoft::Terminal::Remoting::CommandlineArgs InitialArgs();

winrt::hstring GetWindowLayout();
void SendContent(const winrt::Microsoft::Terminal::Remoting::RequestReceiveContentArgs& args);

WINRT_PROPERTY(winrt::hstring, WindowName);
WINRT_PROPERTY(winrt::hstring, ActiveTabTitle);
Expand All @@ -70,6 +85,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TYPED_EVENT(GetWindowLayoutRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::GetWindowLayoutArgs);

TYPED_EVENT(AttachRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::AttachRequest);
TYPED_EVENT(SendContentRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::RequestReceiveContentArgs);

private:
Peasant(const uint64_t testPID);
Expand All @@ -87,4 +103,5 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
namespace winrt::Microsoft::Terminal::Remoting::factory_implementation
{
BASIC_FACTORY(Peasant);
BASIC_FACTORY(RequestReceiveContentArgs);
}
11 changes: 10 additions & 1 deletion src/cascadia/Remoting/Peasant.idl
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ namespace Microsoft.Terminal.Remoting
String Content { get; };
UInt32 TabIndex { get; };
}
[default_interface] runtimeclass RequestReceiveContentArgs {
RequestReceiveContentArgs(UInt64 src, UInt64 tgt, UInt32 tabIndex);

UInt64 SourceWindow { get; };
UInt64 TargetWindow { get; };
UInt32 TabIndex { get; };
};

interface IPeasant
{
Expand Down Expand Up @@ -82,7 +89,7 @@ namespace Microsoft.Terminal.Remoting
String GetWindowLayout();

void AttachContentToWindow(AttachRequest request);

void SendContent(RequestReceiveContentArgs args);

event Windows.Foundation.TypedEventHandler<Object, WindowActivatedArgs> WindowActivated;
event Windows.Foundation.TypedEventHandler<Object, CommandlineArgs> ExecuteCommandlineRequested;
Expand All @@ -98,6 +105,8 @@ namespace Microsoft.Terminal.Remoting
event Windows.Foundation.TypedEventHandler<Object, Object> QuitRequested;

event Windows.Foundation.TypedEventHandler<Object, AttachRequest> AttachRequested;

event Windows.Foundation.TypedEventHandler<Object, RequestReceiveContentArgs> SendContentRequested;
};

[default_interface] runtimeclass Peasant : IPeasant
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/Remoting/WindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,4 +422,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_monarch.RequestMoveContent(window, content, tabIndex);
}

winrt::fire_and_forget WindowManager::RequestSendContent(Remoting::RequestReceiveContentArgs args)
{
co_await winrt::resume_background();
_monarch.RequestSendContent(args);
}
}
1 change: 1 addition & 0 deletions src/cascadia/Remoting/WindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
bool DoesQuakeWindowExist();

winrt::fire_and_forget RequestMoveContent(winrt::hstring window, winrt::hstring content, uint32_t tabIndex);
winrt::fire_and_forget RequestSendContent(Remoting::RequestReceiveContentArgs args);

TYPED_EVENT(FindTargetWindowRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs);

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/Remoting/WindowManager.idl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace Microsoft.Terminal.Remoting
Boolean DoesQuakeWindowExist();

void RequestMoveContent(String window, String content, UInt32 tabIndex);
void RequestSendContent(RequestReceiveContentArgs args);

event Windows.Foundation.TypedEventHandler<Object, FindTargetWindowArgs> FindTargetWindowRequested;

Expand Down
13 changes: 13 additions & 0 deletions src/cascadia/TerminalApp/TabBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,19 @@ namespace winrt::TerminalApp::implementation
tab->_RequestFocusActiveControlHandlers();
}
});

// BODGY: When the tab is drag/dropped, the TabView gets a
// TabDragStarting. However, the way it is implemented[^1], the
// TabViewItem needs either an Item or a Content for the event to
// include the correct TabViewItem. Otherwise, it will just return the
// first TabViewItem in the TabView with the same Content as the dragged
// tab (which, if the Content is null, will be the _first_ tab).
//
// So here, we'll stick an empty border in, just so that every tab has a
// Content which is not equal to the others.
//
// [^1]: microsoft-ui-xaml/blob/92fbfcd55f05c92ac65569f5d284c5b36492091e/dev/TabView/TabView.cpp#L751-L758
TabViewItem().Content(winrt::WUX::Controls::Border{});
}

std::optional<winrt::Windows::UI::Color> TabBase::GetTabColor()
Expand Down
8 changes: 7 additions & 1 deletion src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,11 @@ namespace winrt::TerminalApp::implementation
_mruTabs.RemoveAt(mruIndex);
}

if (_stashedDraggedTab && *_stashedDraggedTab == tab)
{
_stashedDraggedTab = nullptr;
}

_tabs.RemoveAt(tabIndex);
_tabView.TabItems().RemoveAt(tabIndex);
_UpdateTabIndices();
Expand Down Expand Up @@ -703,7 +708,8 @@ namespace winrt::TerminalApp::implementation
winrt::TerminalApp::TabBase TerminalPage::_GetTabByTabViewItem(const Microsoft::UI::Xaml::Controls::TabViewItem& tabViewItem) const noexcept
{
uint32_t tabIndexFromControl{};
if (_tabView.TabItems().IndexOf(tabViewItem, tabIndexFromControl))
const auto items{ _tabView.TabItems() };
if (items.IndexOf(tabViewItem, tabIndexFromControl))
{
// If IndexOf returns true, we've actually got an index
return _tabs.GetAt(tabIndexFromControl);
Expand Down
Loading

0 comments on commit 9514c11

Please sign in to comment.