From 7e4f37a5f35de8235a1ebb71ac4bcecad752a75c Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 13 Apr 2023 14:17:22 -0500 Subject: [PATCH 1/4] this has lots of dead code, but it fixes it --- src/cascadia/TerminalApp/TabManagement.cpp | 43 ++++++++++++++++++++- src/cascadia/TerminalApp/TerminalPage.cpp | 19 ++++++++- src/cascadia/TerminalApp/TerminalPage.h | 4 +- src/cascadia/TerminalApp/TerminalWindow.cpp | 12 +++++- 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index 457cf34c1ca..f21d7fbce19 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -1079,19 +1079,58 @@ namespace winrt::TerminalApp::implementation } void TerminalPage::_TabDragStarted(const IInspectable& /*sender*/, - const IInspectable& /*eventArgs*/) + const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragStartingEventArgs& eventArgs) { _rearranging = true; _rearrangeFrom = std::nullopt; _rearrangeTo = std::nullopt; + + const auto& draggedTvi{ eventArgs.Tab() }; + uint32_t tabIndexFromControl{}; + const auto tabItems{ _tabView.TabItems() }; + if (tabItems.IndexOf(draggedTvi, tabIndexFromControl)) + { + // If IndexOf returns true, we've actually got an index + _rearrangeFrom = tabIndexFromControl; + } } void TerminalPage::_TabDragCompleted(const IInspectable& /*sender*/, - const IInspectable& /*eventArgs*/) + const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragCompletedEventArgs& eventArgs) { + // FIRST + // * Get the `from` index. Get this by finding the TabBase of the + // TabViewItem that's been dropped, and finding the index of it in + // _tabs. + // + const auto& draggedTvi{ eventArgs.Tab() }; + + uint32_t tabIndexFromControl{}; + // winrt::TerminalApp::TabBase tabBase{nullptr}; + const auto tabItems{ _tabView.TabItems() }; + if (tabItems.IndexOf(draggedTvi, tabIndexFromControl)) + { + // If IndexOf returns true, we've actually got an index + // tabBase = _tabs.GetAt(tabIndexFromControl); + _rearrangeTo = tabIndexFromControl; + } + // if (tabBase ==nullptr) + // {return;} + + // winrt::com_ptr tabImpl; + // tabImpl.copy_from(winrt::get_self(tabBase)); + // if (tabImpl ==nullptr) + // { + // return; + // } + auto& from{ _rearrangeFrom }; auto& to{ _rearrangeTo }; + // SECOND + // * get the `to` index. To do that, get the index of the dropped + // TabViewItem in the TabView's items + if (from.has_value() && to.has_value() && to != from) { auto& tabs{ _tabs }; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 240ee8eca8b..2a7ae1687bf 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -4739,13 +4739,30 @@ namespace winrt::TerminalApp::implementation } } + const auto otherId{ winrt::unbox_value_or(windowIdObj, 0) }; + const auto myId{ _WindowProperties.WindowId() }; + otherId; + // if (otherId == myId) + // { + // // dragged from us to ourselves + + // uint32_t tabIndexFromTab{}; + // if (_tabs.IndexOf(*_stashed.draggedTab, tabIndexFromTab)) + // { + // _rearrangeFrom = tabIndexFromTab; + // _rearrangeTo = index; + // } + // } + // else + // { // `this` is safe to use - const auto request = winrt::make_self(src, _WindowProperties.WindowId(), index); + const auto request = winrt::make_self(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 // RequestMoveContent to move their tab to us. _RequestReceiveContentHandlers(*this, *request); + // } } } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 83b80bb633c..d3a8107f1e0 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -412,8 +412,8 @@ 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); diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp index 66d8a53f597..f8a906b55a6 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.cpp +++ b/src/cascadia/TerminalApp/TerminalWindow.cpp @@ -740,9 +740,19 @@ namespace winrt::TerminalApp::implementation { // If // * the position has been specified on the commandline, + // * we're re-opening from a persisted layout, // * We're opening the window as a part of tear out (and _contentBounds were set) // then don't center on launch - return !_contentBounds && _settings.GlobalSettings().CenterOnLaunch() && !_appArgs.GetPosition().has_value(); + bool hadPersistedPosition = false; + if (const auto layout = LoadPersistedLayout()) + { + hadPersistedPosition = (bool)layout.InitialPosition(); + } + + return !_contentBounds && + !hadPersistedPosition && + _settings.GlobalSettings().CenterOnLaunch() && + !_appArgs.GetPosition().has_value(); } // Method Description: From 293967da958bb0188002f7433edb987b91dc8e5d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 13 Apr 2023 15:41:23 -0500 Subject: [PATCH 2/4] last cleanup --- src/cascadia/TerminalApp/TabManagement.cpp | 56 ++-------------------- src/cascadia/TerminalApp/TerminalPage.cpp | 20 +------- src/cascadia/TerminalApp/TerminalPage.h | 1 - 3 files changed, 7 insertions(+), 70 deletions(-) diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index f21d7fbce19..b35080242bf 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/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,12 +1051,15 @@ namespace winrt::TerminalApp::implementation } void TerminalPage::_TabDragStarted(const IInspectable& /*sender*/, - const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragStartingEventArgs& 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& draggedTvi{ eventArgs.Tab() }; uint32_t tabIndexFromControl{}; const auto tabItems{ _tabView.TabItems() }; @@ -1096,41 +1071,20 @@ namespace winrt::TerminalApp::implementation } void TerminalPage::_TabDragCompleted(const IInspectable& /*sender*/, - const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragCompletedEventArgs& eventArgs) + const winrt::MUX::Controls::TabViewTabDragCompletedEventArgs& eventArgs) { - // FIRST - // * Get the `from` index. Get this by finding the TabBase of the - // TabViewItem that's been dropped, and finding the index of it in - // _tabs. - // const auto& draggedTvi{ eventArgs.Tab() }; uint32_t tabIndexFromControl{}; - // winrt::TerminalApp::TabBase tabBase{nullptr}; const auto tabItems{ _tabView.TabItems() }; if (tabItems.IndexOf(draggedTvi, tabIndexFromControl)) { - // If IndexOf returns true, we've actually got an index - // tabBase = _tabs.GetAt(tabIndexFromControl); _rearrangeTo = tabIndexFromControl; } - // if (tabBase ==nullptr) - // {return;} - - // winrt::com_ptr tabImpl; - // tabImpl.copy_from(winrt::get_self(tabBase)); - // if (tabImpl ==nullptr) - // { - // return; - // } auto& from{ _rearrangeFrom }; auto& to{ _rearrangeTo }; - // SECOND - // * get the `to` index. To do that, get the index of the dropped - // TabViewItem in the TabView's items - if (from.has_value() && to.has_value() && to != from) { auto& tabs{ _tabs }; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 2a7ae1687bf..fbd790e5617 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/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,30 +4740,13 @@ namespace winrt::TerminalApp::implementation } } - const auto otherId{ winrt::unbox_value_or(windowIdObj, 0) }; const auto myId{ _WindowProperties.WindowId() }; - otherId; - // if (otherId == myId) - // { - // // dragged from us to ourselves - - // uint32_t tabIndexFromTab{}; - // if (_tabs.IndexOf(*_stashed.draggedTab, tabIndexFromTab)) - // { - // _rearrangeFrom = tabIndexFromTab; - // _rearrangeTo = index; - // } - // } - // else - // { - // `this` is safe to use const auto request = winrt::make_self(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 // RequestMoveContent to move their tab to us. _RequestReceiveContentHandlers(*this, *request); - // } } } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index d3a8107f1e0..a775902396e 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -417,7 +417,6 @@ namespace winrt::TerminalApp::implementation 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); From 931338321bc861e801d91f41794605c150075b15 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 13 Apr 2023 15:43:05 -0500 Subject: [PATCH 3/4] this snuck in --- src/cascadia/TerminalApp/TerminalWindow.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp index f8a906b55a6..66d8a53f597 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.cpp +++ b/src/cascadia/TerminalApp/TerminalWindow.cpp @@ -740,19 +740,9 @@ namespace winrt::TerminalApp::implementation { // If // * the position has been specified on the commandline, - // * we're re-opening from a persisted layout, // * We're opening the window as a part of tear out (and _contentBounds were set) // then don't center on launch - bool hadPersistedPosition = false; - if (const auto layout = LoadPersistedLayout()) - { - hadPersistedPosition = (bool)layout.InitialPosition(); - } - - return !_contentBounds && - !hadPersistedPosition && - _settings.GlobalSettings().CenterOnLaunch() && - !_appArgs.GetPosition().has_value(); + return !_contentBounds && _settings.GlobalSettings().CenterOnLaunch() && !_appArgs.GetPosition().has_value(); } // Method Description: From f8f8a1084dc5580acb731e20fe132b466bdfa34d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 14 Apr 2023 13:06:30 -0500 Subject: [PATCH 4/4] I guess that's not a word, you're right --- src/cascadia/TerminalApp/TabManagement.cpp | 8 ++++---- src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index b35080242bf..51918082420 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -1060,10 +1060,10 @@ namespace winrt::TerminalApp::implementation // 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& draggedTvi{ eventArgs.Tab() }; + const auto& draggedTabViewItem{ eventArgs.Tab() }; uint32_t tabIndexFromControl{}; const auto tabItems{ _tabView.TabItems() }; - if (tabItems.IndexOf(draggedTvi, tabIndexFromControl)) + if (tabItems.IndexOf(draggedTabViewItem, tabIndexFromControl)) { // If IndexOf returns true, we've actually got an index _rearrangeFrom = tabIndexFromControl; @@ -1073,11 +1073,11 @@ namespace winrt::TerminalApp::implementation void TerminalPage::_TabDragCompleted(const IInspectable& /*sender*/, const winrt::MUX::Controls::TabViewTabDragCompletedEventArgs& eventArgs) { - const auto& draggedTvi{ eventArgs.Tab() }; + const auto& draggedTabViewItem{ eventArgs.Tab() }; uint32_t tabIndexFromControl{}; const auto tabItems{ _tabView.TabItems() }; - if (tabItems.IndexOf(draggedTvi, tabIndexFromControl)) + if (tabItems.IndexOf(draggedTabViewItem, tabIndexFromControl)) { _rearrangeTo = tabIndexFromControl; } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index fbd790e5617..fa2612c6c24 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -4720,7 +4720,7 @@ namespace winrt::TerminalApp::implementation 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;