Skip to content

Commit

Permalink
Fix reordering tabs mysteriously shuffling the actual backing tabs (#…
Browse files Browse the repository at this point in the history
…15178)

TL;DR: we stopped getting `TabView.TabItemsChanged`. This meant that the
tab view would change its apparent order, but we wouldn't change the
backing tab order.

I'm fixing this by grabbing the index of the tab that starts the drag,
and the index of the tab view item at the end of the drag, and using
that to reorder our backing list.

Closes #15121

Upstream microsoft/microsoft-ui-xaml#8388

Regressed in #15078 - I'm pretty confident about this, since I've got a
1.18.931 build of the Terminal with tear-out, but not MUX 2.8.
  • Loading branch information
zadjii-msft authored Apr 17, 2023
1 parent 1d354d0 commit 9b960bc
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 36 deletions.
53 changes: 23 additions & 30 deletions src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -890,34 +890,6 @@ namespace winrt::TerminalApp::implementation
co_await _HandleCloseTabRequested(tab);
}
}
// Method Description:
// - Responds to changes in the TabView's item list by changing the
// tabview's visibility.
// - This method is also invoked when tabs are dragged / dropped as part of
// tab reordering and this method hands that case as well in concert with
// TabDragStarting and TabDragCompleted handlers that are set up in
// TerminalPage::Create()
// Arguments:
// - sender: the control that originated this event
// - eventArgs: the event's constituent arguments
void TerminalPage::_OnTabItemsChanged(const IInspectable& /*sender*/, const Windows::Foundation::Collections::IVectorChangedEventArgs& eventArgs)
{
if (_rearranging)
{
if (eventArgs.CollectionChange() == Windows::Foundation::Collections::CollectionChange::ItemRemoved)
{
_rearrangeFrom = eventArgs.Index();
}

if (eventArgs.CollectionChange() == Windows::Foundation::Collections::CollectionChange::ItemInserted)
{
_rearrangeTo = eventArgs.Index();
}
}

CommandPalette().Visibility(Visibility::Collapsed);
_UpdateTabView();
}

// Method Description:
// - Additional responses to clicking on a TabView's item. Currently, just remove tab with middle click
Expand Down Expand Up @@ -1079,16 +1051,37 @@ namespace winrt::TerminalApp::implementation
}

void TerminalPage::_TabDragStarted(const IInspectable& /*sender*/,
const IInspectable& /*eventArgs*/)
const winrt::MUX::Controls::TabViewTabDragStartingEventArgs& eventArgs)
{
_rearranging = true;
_rearrangeFrom = std::nullopt;
_rearrangeTo = std::nullopt;

// Start tracking the index of the tab that is being dragged. In
// `_TabDragCompleted`, we'll use this to determine how to reorder our
// internal tabs list.
const auto& draggedTabViewItem{ eventArgs.Tab() };
uint32_t tabIndexFromControl{};
const auto tabItems{ _tabView.TabItems() };
if (tabItems.IndexOf(draggedTabViewItem, tabIndexFromControl))
{
// If IndexOf returns true, we've actually got an index
_rearrangeFrom = tabIndexFromControl;
}
}

void TerminalPage::_TabDragCompleted(const IInspectable& /*sender*/,
const IInspectable& /*eventArgs*/)
const winrt::MUX::Controls::TabViewTabDragCompletedEventArgs& eventArgs)
{
const auto& draggedTabViewItem{ eventArgs.Tab() };

uint32_t tabIndexFromControl{};
const auto tabItems{ _tabView.TabItems() };
if (tabItems.IndexOf(draggedTabViewItem, tabIndexFromControl))
{
_rearrangeTo = tabIndexFromControl;
}

auto& from{ _rearrangeFrom };
auto& to{ _rearrangeTo };

Expand Down
7 changes: 4 additions & 3 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ namespace winrt::TerminalApp::implementation
});
_tabView.SelectionChanged({ this, &TerminalPage::_OnTabSelectionChanged });
_tabView.TabCloseRequested({ this, &TerminalPage::_OnTabCloseRequested });
_tabView.TabItemsChanged({ this, &TerminalPage::_OnTabItemsChanged });

_tabView.TabDragStarting({ this, &TerminalPage::_onTabDragStarting });
_tabView.TabStripDragOver({ this, &TerminalPage::_onTabStripDragOver });
Expand Down Expand Up @@ -4720,6 +4719,8 @@ namespace winrt::TerminalApp::implementation
co_await wil::resume_foreground(Dispatcher());
if (const auto& page{ weakThis.get() })
{
// `this` is safe to use
//
// First we need to get the position in the List to drop to
auto index = -1;

Expand All @@ -4739,8 +4740,8 @@ namespace winrt::TerminalApp::implementation
}
}

// `this` is safe to use
const auto request = winrt::make_self<RequestReceiveContentArgs>(src, _WindowProperties.WindowId(), index);
const auto myId{ _WindowProperties.WindowId() };
const auto request = winrt::make_self<RequestReceiveContentArgs>(src, myId, index);

// This will go up to the monarch, who will then dispatch the request
// back down to the source TerminalPage, who will then perform a
Expand Down
5 changes: 2 additions & 3 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,11 @@ namespace winrt::TerminalApp::implementation

fire_and_forget _LaunchSettings(const Microsoft::Terminal::Settings::Model::SettingsTarget target);

void _TabDragStarted(const IInspectable& sender, const IInspectable& eventArgs);
void _TabDragCompleted(const IInspectable& sender, const IInspectable& eventArgs);
void _TabDragStarted(const IInspectable& sender, const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragStartingEventArgs& eventArgs);
void _TabDragCompleted(const IInspectable& sender, const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragCompletedEventArgs& eventArgs);

void _OnTabClick(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
void _OnTabSelectionChanged(const IInspectable& sender, const Windows::UI::Xaml::Controls::SelectionChangedEventArgs& eventArgs);
void _OnTabItemsChanged(const IInspectable& sender, const Windows::Foundation::Collections::IVectorChangedEventArgs& eventArgs);
void _OnTabCloseRequested(const IInspectable& sender, const Microsoft::UI::Xaml::Controls::TabViewTabCloseRequestedEventArgs& eventArgs);
void _OnFirstLayout(const IInspectable& sender, const IInspectable& eventArgs);
void _UpdatedSelectedTab(const winrt::TerminalApp::TabBase& tab);
Expand Down

0 comments on commit 9b960bc

Please sign in to comment.