Fix reordering tabs mysteriously shuffling the actual backing tabs

src/cascadia/TerminalApp/TabManagement.cpp

@@ -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
@@ -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 };
 

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -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 });
@@ -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;
 
@@ -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

src/cascadia/TerminalApp/TerminalPage.h

@@ -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);