From 250e3d4059100c4dbde01e495a332c72c7ef134c Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 11 Nov 2019 14:26:45 -0600 Subject: [PATCH 01/14] this is janky but is close for some reason? --- src/cascadia/TerminalApp/Pane.cpp | 76 ++++++++++++++++------- src/cascadia/TerminalApp/Pane.h | 12 +++- src/cascadia/TerminalApp/Tab.cpp | 55 ++++++++++++---- src/cascadia/TerminalApp/Tab.h | 3 +- src/cascadia/TerminalApp/TerminalPage.cpp | 4 +- src/cascadia/inc/cppwinrt_utils.h | 14 ++--- 6 files changed, 118 insertions(+), 46 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 4c0b62cae95..e68541c002b 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -62,6 +62,16 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus s_focusedBorderBrush = SolidColorBrush{ Colors::Transparent() }; } } + + // _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, { this, &Pane::_GotFocusHandler }); + // _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, [this](auto&&, const winrt::Windows::UI::Xaml::RoutedEventArgs& args) { + _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, [this](auto&&, auto&&) { + // _GotFocusHandler(shared_from_this(), args); + if (pfnGotFocus) + { + pfnGotFocus(shared_from_this()); + } + }); } // Method Description: @@ -403,6 +413,23 @@ TermControl Pane::GetFocusedTerminalControl() return lastFocused ? lastFocused->_control : nullptr; } +void Pane::ClearActive() +{ + _lastFocused = false; + if (!_IsLeaf()) + { + _firstChild->ClearActive(); + _secondChild->ClearActive(); + } + UpdateFocus(); +} + +void Pane::SetActive() +{ + _lastFocused = true; + UpdateFocus(); +} + // Method Description: // - Returns nullopt if no children of this pane were the last control to be // focused, or the GUID of the profile of the last control to be focused (if @@ -469,22 +496,23 @@ bool Pane::_HasFocusedChild() const noexcept // - void Pane::UpdateFocus() { - if (_IsLeaf()) - { - const auto controlFocused = _control && - _control.FocusState() != FocusState::Unfocused; - - _lastFocused = controlFocused; - - _border.BorderBrush(_lastFocused ? s_focusedBorderBrush : nullptr); - } - else - { - _lastFocused = false; - - _firstChild->UpdateFocus(); - _secondChild->UpdateFocus(); - } + _border.BorderBrush(_lastFocused ? s_focusedBorderBrush : nullptr); + // // TODO: Do we need this? + // if (_IsLeaf()) + // { + // const auto controlFocused = _control && + // _control.FocusState() != FocusState::Unfocused; + + // _lastFocused = controlFocused; + + // } + // else + // { + // _lastFocused = false; + + // _firstChild->UpdateFocus(); + // _secondChild->UpdateFocus(); + // } } // Method Description: @@ -893,23 +921,23 @@ bool Pane::CanSplit(SplitState splitType) // - control: A TermControl to use in the new pane. // Return Value: // - -void Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) +std::pair, std::shared_ptr> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) { if (!_IsLeaf()) { if (_firstChild->_HasFocusedChild()) { - _firstChild->Split(splitType, profile, control); + return _firstChild->Split(splitType, profile, control); } else if (_secondChild->_HasFocusedChild()) { - _secondChild->Split(splitType, profile, control); + return _secondChild->Split(splitType, profile, control); } - return; + return { nullptr, nullptr }; } - _Split(splitType, profile, control); + return _Split(splitType, profile, control); } // Method Description: @@ -953,7 +981,7 @@ bool Pane::_CanSplit(SplitState splitType) // - control: A TermControl to use in the new pane. // Return Value: // - -void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) +std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) { // Lock the create/close lock so that another operation won't concurrently // modify our tree @@ -992,6 +1020,10 @@ void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& _SetupChildCloseHandlers(); _lastFocused = false; + + pfnGotFocus = nullptr; + + return { _firstChild, _secondChild }; } // Method Description: diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index f22bdf9bb9c..fe004ff115f 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -53,6 +53,8 @@ class Pane : public std::enable_shared_from_this bool WasLastFocused() const noexcept; void UpdateFocus(); + void ClearActive(); + void SetActive(); void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); void ResizeContent(const winrt::Windows::Foundation::Size& newSize); @@ -60,12 +62,16 @@ class Pane : public std::enable_shared_from_this bool NavigateFocus(const winrt::TerminalApp::Direction& direction); bool CanSplit(SplitState splitType); - void Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + std::pair, std::shared_ptr> Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void Close(); + std::function)> pfnGotFocus{ nullptr }; + DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); + // TYPED_EVENT(GotFocus, std::shared_ptr, winrt::Windows::UI::Xaml::RoutedEventArgs); + private: winrt::Windows::UI::Xaml::Controls::Grid _root{}; winrt::Windows::UI::Xaml::Controls::Border _border{}; @@ -84,6 +90,8 @@ class Pane : public std::enable_shared_from_this winrt::event_token _firstClosedToken{ 0 }; winrt::event_token _secondClosedToken{ 0 }; + winrt::Windows::UI::Xaml::UIElement::GotFocus_revoker _gotFocusRevoker; + std::shared_mutex _createCloseLock{}; Borders _borders{ Borders::None }; @@ -93,7 +101,7 @@ class Pane : public std::enable_shared_from_this void _SetupChildCloseHandlers(); bool _CanSplit(SplitState splitType); - void _Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + std::pair, std::shared_ptr> _Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void _CreateRowColDefinitions(const winrt::Windows::Foundation::Size& rootSize); void _CreateSplitContent(); void _ApplySplitDefinitions(); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index f0e2efa88c2..dc4b9242bc8 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -23,6 +23,17 @@ Tab::Tab(const GUID& profile, const TermControl& control) _closedHandlers(); }); + _activePane = _rootPane; + + // TODO: Add an event handler to this pane's GotFocus event. + // When that pane gains focus, we'll mark it as the new active pane. + _rootPane->pfnGotFocus = ([this](std::shared_ptr sender) { + // _rootPane->GotFocus([this](std::shared_ptr sender, auto&&) { + _rootPane->ClearActive(); + _activePane = sender; + _activePane->SetActive(); + }); + _MakeTabViewItem(); } @@ -47,9 +58,12 @@ UIElement Tab::GetRootElement() // Return Value: // - nullptr if no children were marked `_lastFocused`, else the TermControl // that was last focused. -TermControl Tab::GetFocusedTerminalControl() +TermControl Tab::GetFocusedTerminalControl() const { - return _rootPane->GetFocusedTerminalControl(); + // TODO: Probably just GetTerminalControl() + return _activePane->GetFocusedTerminalControl(); + + // return _rootPane->GetFocusedTerminalControl(); } winrt::MUX::Controls::TabViewItem Tab::GetTabViewItem() @@ -99,7 +113,7 @@ void Tab::SetFocused(const bool focused) // focused, else the GUID of the profile of the last control to be focused std::optional Tab::GetFocusedProfile() const noexcept { - return _rootPane->GetFocusedProfile(); + return _activePane->GetFocusedProfile(); } // Method Description: @@ -124,7 +138,7 @@ void Tab::_Focus() { _focused = true; - auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); + auto lastFocusedControl = GetFocusedTerminalControl(); if (lastFocusedControl) { lastFocusedControl.Focus(FocusState::Programmatic); @@ -142,7 +156,8 @@ void Tab::_Focus() // - void Tab::UpdateFocus() { - _rootPane->UpdateFocus(); + // _rootPane->UpdateFocus(); + // Maybe root->ClearActive, active->SetActive } void Tab::UpdateIcon(const winrt::hstring iconPath) @@ -169,7 +184,8 @@ void Tab::UpdateIcon(const winrt::hstring iconPath) // - the title string of the last focused terminal control in our tree. winrt::hstring Tab::GetFocusedTitle() const { - const auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); + // const auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); + const auto lastFocusedControl = GetFocusedTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; } @@ -213,7 +229,7 @@ void Tab::Scroll(const int delta) // - True if the focused pane can be split. False otherwise. bool Tab::CanSplitPane(Pane::SplitState splitType) { - return _rootPane->CanSplit(splitType); + return _activePane->CanSplit(splitType); } // Method Description: @@ -227,7 +243,22 @@ bool Tab::CanSplitPane(Pane::SplitState splitType) // - void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl& control) { - _rootPane->Split(splitType, profile, control); + auto [firstNewPane, secondNewPane] = _rootPane->Split(splitType, profile, control); + + // TODO: Add an event handler to this pane's GotFocus event. + // When that pane gains focus, we'll mark it as the new active pane. + firstNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { + // firstNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { + _rootPane->ClearActive(); + _activePane = sender; + _activePane->SetActive(); + }); + secondNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { + // secondNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { + _rootPane->ClearActive(); + _activePane = sender; + _activePane->SetActive(); + }); } // Method Description: @@ -239,7 +270,7 @@ void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl // - void Tab::ResizeContent(const winrt::Windows::Foundation::Size& newSize) { - _rootPane->ResizeContent(newSize); + _activePane->ResizeContent(newSize); } // Method Description: @@ -251,7 +282,7 @@ void Tab::ResizeContent(const winrt::Windows::Foundation::Size& newSize) // - void Tab::ResizePane(const winrt::TerminalApp::Direction& direction) { - _rootPane->ResizePane(direction); + _activePane->ResizePane(direction); } // Method Description: @@ -263,7 +294,7 @@ void Tab::ResizePane(const winrt::TerminalApp::Direction& direction) // - void Tab::NavigateFocus(const winrt::TerminalApp::Direction& direction) { - _rootPane->NavigateFocus(direction); + _activePane->NavigateFocus(direction); } // Method Description: @@ -276,7 +307,7 @@ void Tab::NavigateFocus(const winrt::TerminalApp::Direction& direction) // - void Tab::ClosePane() { - auto focused = _rootPane->GetFocusedPane(); + auto focused = _activePane->GetFocusedPane(); focused->Close(); } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 8af4423e6b8..b5805ceaa9f 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -12,7 +12,7 @@ class Tab winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); winrt::Windows::UI::Xaml::UIElement GetRootElement(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl() const; std::optional GetFocusedProfile() const noexcept; bool IsFocused() const noexcept; @@ -40,6 +40,7 @@ class Tab private: std::shared_ptr _rootPane{ nullptr }; + std::shared_ptr _activePane{ nullptr }; winrt::hstring _lastIconPath{}; bool _focused{ false }; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index f17a19a5b7f..3d436145d5b 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -743,8 +743,8 @@ namespace winrt::TerminalApp::implementation { return; } - // Update the focus of the tab's panes - tab->UpdateFocus(); + // // Update the focus of the tab's panes + // tab->UpdateFocus(); // Possibly update the title of the tab, window to match the newly // focused pane. diff --git a/src/cascadia/inc/cppwinrt_utils.h b/src/cascadia/inc/cppwinrt_utils.h index c51a023ef7b..9329c1cdef8 100644 --- a/src/cascadia/inc/cppwinrt_utils.h +++ b/src/cascadia/inc/cppwinrt_utils.h @@ -63,13 +63,13 @@ private: // signatures and define them both for you, because they don't really vary from // event to event. // Use this in a classes header if you have a Windows.Foundation.TypedEventHandler -#define TYPED_EVENT(name, sender, args) \ -public: \ - winrt::event_token name(Windows::Foundation::TypedEventHandler const& handler) { return _##name##Handlers.add(handler); } \ - void name(winrt::event_token const& token) noexcept { _##name##Handlers.remove(token); } \ - \ -private: \ - winrt::event> _##name##Handlers; +#define TYPED_EVENT(name, sender, args) \ +public: \ + winrt::event_token name(winrt::Windows::Foundation::TypedEventHandler const& handler) { return _##name##Handlers.add(handler); } \ + void name(winrt::event_token const& token) noexcept { _##name##Handlers.remove(token); } \ + \ +private: \ + winrt::event> _##name##Handlers; // This is a helper macro for both declaring the signature and body of an event // which is exposed by one class, but actually handled entirely by one of the From 3125f624f77d39ec6ea9a3093dd675a9efeb2242 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 11 Nov 2019 17:08:29 -0600 Subject: [PATCH 02/14] This is _almost_ right to solve #1205 If I want to double up and also fix #522 (which I do), then I need to also * when a tab GetsFocus, send the focus instead to the Pane * When the border is clicked on, focus that pane's control And like a lot of cleanup, because this is horrifying --- src/cascadia/TerminalApp/Pane.cpp | 18 ++++--- src/cascadia/TerminalApp/Pane.h | 5 +- src/cascadia/TerminalApp/Tab.cpp | 64 +++++++++++++++++------ src/cascadia/TerminalApp/Tab.h | 1 + src/cascadia/TerminalApp/TerminalPage.cpp | 55 +++++++++++-------- 5 files changed, 95 insertions(+), 48 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index e68541c002b..77e879cb31d 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -921,23 +921,24 @@ bool Pane::CanSplit(SplitState splitType) // - control: A TermControl to use in the new pane. // Return Value: // - -std::pair, std::shared_ptr> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) +void Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) +// std::pair, std::shared_ptr> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) { if (!_IsLeaf()) { if (_firstChild->_HasFocusedChild()) { - return _firstChild->Split(splitType, profile, control); + _firstChild->Split(splitType, profile, control); } else if (_secondChild->_HasFocusedChild()) { - return _secondChild->Split(splitType, profile, control); + _secondChild->Split(splitType, profile, control); } - return { nullptr, nullptr }; + return; } - return _Split(splitType, profile, control); + _Split(splitType, profile, control); } // Method Description: @@ -981,7 +982,8 @@ bool Pane::_CanSplit(SplitState splitType) // - control: A TermControl to use in the new pane. // Return Value: // - -std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) +void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) +// std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) { // Lock the create/close lock so that another operation won't concurrently // modify our tree @@ -1021,9 +1023,9 @@ std::pair, std::shared_ptr> Pane::_Split(SplitState _lastFocused = false; + _firstChild->pfnGotFocus = pfnGotFocus; + _secondChild->pfnGotFocus = pfnGotFocus; pfnGotFocus = nullptr; - - return { _firstChild, _secondChild }; } // Method Description: diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index fe004ff115f..f93e7b6e228 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -62,7 +62,8 @@ class Pane : public std::enable_shared_from_this bool NavigateFocus(const winrt::TerminalApp::Direction& direction); bool CanSplit(SplitState splitType); - std::pair, std::shared_ptr> Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + // std::pair, std::shared_ptr> Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void Close(); @@ -101,7 +102,7 @@ class Pane : public std::enable_shared_from_this void _SetupChildCloseHandlers(); bool _CanSplit(SplitState splitType); - std::pair, std::shared_ptr> _Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void _Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void _CreateRowColDefinitions(const winrt::Windows::Foundation::Size& rootSize); void _CreateSplitContent(); void _ApplySplitDefinitions(); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index dc4b9242bc8..ab75a52e4b6 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -32,6 +32,35 @@ Tab::Tab(const GUID& profile, const TermControl& control) _rootPane->ClearActive(); _activePane = sender; _activePane->SetActive(); + + // // void TerminalPage::_UpdateTabIcon(std::shared_ptr tab) + // { + // const auto lastFocusedProfileOpt = this->GetFocusedProfile(); + // if (lastFocusedProfileOpt.has_value()) + // { + // const auto lastFocusedProfile = lastFocusedProfileOpt.value(); + // const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile); + // if (matchingProfile) + // { + // tab->UpdateIcon(matchingProfile->GetExpandedIconPath()); + // } + // else + // { + // tab->UpdateIcon({}); + // } + // } + // } + if (pfnFocusChanged) + { + pfnFocusChanged(); + auto newTabTitle = this->GetFocusedTitle(); + this->SetTabText(newTabTitle); + } + }); + + control.TitleChanged([this](auto newTitle) { + auto newTabTitle = this->GetFocusedTitle(); + this->SetTabText(newTabTitle); }); _MakeTabViewItem(); @@ -243,21 +272,26 @@ bool Tab::CanSplitPane(Pane::SplitState splitType) // - void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl& control) { - auto [firstNewPane, secondNewPane] = _rootPane->Split(splitType, profile, control); - - // TODO: Add an event handler to this pane's GotFocus event. - // When that pane gains focus, we'll mark it as the new active pane. - firstNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { - // firstNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { - _rootPane->ClearActive(); - _activePane = sender; - _activePane->SetActive(); - }); - secondNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { - // secondNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { - _rootPane->ClearActive(); - _activePane = sender; - _activePane->SetActive(); + _activePane->Split(splitType, profile, control); + // auto [firstNewPane, secondNewPane] = _activePane->Split(splitType, profile, control); + + // // TODO: Add an event handler to this pane's GotFocus event. + // // When that pane gains focus, we'll mark it as the new active pane. + // firstNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { + // // firstNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { + // _rootPane->ClearActive(); + // _activePane = sender; + // _activePane->SetActive(); + // }); + // secondNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { + // // secondNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { + // _rootPane->ClearActive(); + // _activePane = sender; + // _activePane->SetActive(); + // }); + control.TitleChanged([this](auto newTitle) { + auto newTabTitle = this->GetFocusedTitle(); + this->SetTabText(newTabTitle); }); } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index b5805ceaa9f..f8dc9f88d03 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -36,6 +36,7 @@ class Tab void ClosePane(); + std::function pfnFocusChanged{ nullptr }; DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); private: diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 3d436145d5b..2a981730780 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -591,7 +591,7 @@ namespace winrt::TerminalApp::implementation void TerminalPage::_UpdateTitle(std::shared_ptr tab) { auto newTabTitle = tab->GetFocusedTitle(); - tab->SetTabText(newTabTitle); + // tab->SetTabText(newTabTitle); if (_settings->GlobalSettings().GetShowTitleInTitlebar() && tab->IsFocused()) @@ -725,34 +725,43 @@ namespace winrt::TerminalApp::implementation // Don't capture a strong ref to the tab. If the tab is removed as this // is called, we don't really care anymore about handling the event. std::weak_ptr weakTabPtr = hostingTab; - term.TitleChanged([this, weakTabPtr](auto newTitle) { + // term.TitleChanged([this, weakTabPtr](auto newTitle) { + // auto tab = weakTabPtr.lock(); + // if (!tab) + // { + // return; + // } + // // The title of the control changed, but not necessarily the title + // // of the tab. Get the title of the focused pane of the tab, and set + // // the tab's text to the focused panes' text. + // _UpdateTitle(tab); + // }); + + hostingTab->pfnFocusChanged = ([this, weakTabPtr]() { auto tab = weakTabPtr.lock(); - if (!tab) - { - return; - } - // The title of the control changed, but not necessarily the title - // of the tab. Get the title of the focused pane of the tab, and set - // the tab's text to the focused panes' text. - _UpdateTitle(tab); - }); - - term.GotFocus([this, weakTabPtr](auto&&, auto&&) { - auto tab = weakTabPtr.lock(); - if (!tab) - { - return; - } - // // Update the focus of the tab's panes - // tab->UpdateFocus(); + // Possibly update the icon of the tab. + _UpdateTabIcon(tab); // Possibly update the title of the tab, window to match the newly // focused pane. _UpdateTitle(tab); - - // Possibly update the icon of the tab. - _UpdateTabIcon(tab); }); + // term.GotFocus([this, weakTabPtr](auto&&, auto&&) { + // auto tab = weakTabPtr.lock(); + // if (!tab) + // { + // return; + // } + // // // Update the focus of the tab's panes + // // tab->UpdateFocus(); + + // // Possibly update the title of the tab, window to match the newly + // // focused pane. + // _UpdateTitle(tab); + + // // Possibly update the icon of the tab. + // _UpdateTabIcon(tab); + // }); } // Method Description: From 59294845475e2e8e6c79b348fa1fdaef1584eb06 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 09:37:10 -0600 Subject: [PATCH 03/14] hey this autorevoker is really nice --- src/cascadia/TerminalApp/Pane.cpp | 19 ++++++++++--------- src/cascadia/TerminalApp/Pane.h | 21 ++++++++++++++------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 77e879cb31d..3a654fd972c 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -63,15 +63,7 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus } } - // _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, { this, &Pane::_GotFocusHandler }); - // _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, [this](auto&&, const winrt::Windows::UI::Xaml::RoutedEventArgs& args) { - _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, [this](auto&&, auto&&) { - // _GotFocusHandler(shared_from_this(), args); - if (pfnGotFocus) - { - pfnGotFocus(shared_from_this()); - } - }); + _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, { this, &Pane::_ControlGotFocusHandler }); } // Method Description: @@ -1087,4 +1079,13 @@ Size Pane::_GetMinSize() const } } +void Pane::_ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable const& /* sender */, + RoutedEventArgs const& /* args */) +{ + if (pfnGotFocus) + { + pfnGotFocus(shared_from_this()); + } +} + DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index f93e7b6e228..865e95c3196 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -43,7 +43,9 @@ class Pane : public std::enable_shared_from_this Horizontal = 2 }; - Pane(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control, const bool lastFocused = false); + Pane(const GUID& profile, + const winrt::Microsoft::Terminal::TerminalControl::TermControl& control, + const bool lastFocused = false); std::shared_ptr GetFocusedPane(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); @@ -56,14 +58,16 @@ class Pane : public std::enable_shared_from_this void ClearActive(); void SetActive(); - void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); + void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, + const GUID& profile); void ResizeContent(const winrt::Windows::Foundation::Size& newSize); bool ResizePane(const winrt::TerminalApp::Direction& direction); bool NavigateFocus(const winrt::TerminalApp::Direction& direction); bool CanSplit(SplitState splitType); - void Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); - // std::pair, std::shared_ptr> Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void Split(SplitState splitType, + const GUID& profile, + const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void Close(); @@ -71,8 +75,6 @@ class Pane : public std::enable_shared_from_this DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); - // TYPED_EVENT(GotFocus, std::shared_ptr, winrt::Windows::UI::Xaml::RoutedEventArgs); - private: winrt::Windows::UI::Xaml::Controls::Grid _root{}; winrt::Windows::UI::Xaml::Controls::Border _border{}; @@ -102,7 +104,10 @@ class Pane : public std::enable_shared_from_this void _SetupChildCloseHandlers(); bool _CanSplit(SplitState splitType); - void _Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void _Split(SplitState splitType, + const GUID& profile, + const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void _CreateRowColDefinitions(const winrt::Windows::Foundation::Size& rootSize); void _CreateSplitContent(); void _ApplySplitDefinitions(); @@ -119,6 +124,8 @@ class Pane : public std::enable_shared_from_this std::pair _GetPaneSizes(const float& fullSize); winrt::Windows::Foundation::Size _GetMinSize() const; + void _ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable const& sender, + winrt::Windows::UI::Xaml::RoutedEventArgs const& e); // Function Description: // - Returns true if the given direction can be used with the given split From aa701568d37cf65434665dc986ab73fc14736576 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 09:52:48 -0600 Subject: [PATCH 04/14] Encapsulate Pane::pfnGotFocus --- src/cascadia/TerminalApp/Pane.cpp | 17 +++++++----- src/cascadia/TerminalApp/Pane.h | 4 ++- src/cascadia/TerminalApp/Tab.cpp | 43 +++++++++++-------------------- src/cascadia/TerminalApp/Tab.h | 2 ++ 4 files changed, 30 insertions(+), 36 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 3a654fd972c..64040c66670 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -914,7 +914,6 @@ bool Pane::CanSplit(SplitState splitType) // Return Value: // - void Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) -// std::pair, std::shared_ptr> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) { if (!_IsLeaf()) { @@ -975,7 +974,6 @@ bool Pane::_CanSplit(SplitState splitType) // Return Value: // - void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) -// std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) { // Lock the create/close lock so that another operation won't concurrently // modify our tree @@ -1015,9 +1013,9 @@ void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& _lastFocused = false; - _firstChild->pfnGotFocus = pfnGotFocus; - _secondChild->pfnGotFocus = pfnGotFocus; - pfnGotFocus = nullptr; + _firstChild->_pfnGotFocus = _pfnGotFocus; + _secondChild->_pfnGotFocus = _pfnGotFocus; + _pfnGotFocus = nullptr; } // Method Description: @@ -1082,10 +1080,15 @@ Size Pane::_GetMinSize() const void Pane::_ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable const& /* sender */, RoutedEventArgs const& /* args */) { - if (pfnGotFocus) + if (_pfnGotFocus) { - pfnGotFocus(shared_from_this()); + _pfnGotFocus(shared_from_this()); } } +void Pane::SetGotFocusCallback(std::function)> pfnGotFocus) +{ + _pfnGotFocus = pfnGotFocus; +} + DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 865e95c3196..e083abe45d6 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -71,7 +71,7 @@ class Pane : public std::enable_shared_from_this void Close(); - std::function)> pfnGotFocus{ nullptr }; + void SetGotFocusCallback(std::function)> pfnGotFocus); DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); @@ -99,6 +99,8 @@ class Pane : public std::enable_shared_from_this Borders _borders{ Borders::None }; + std::function)> _pfnGotFocus{ nullptr }; + bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index ab75a52e4b6..0baed9e8492 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -25,10 +25,10 @@ Tab::Tab(const GUID& profile, const TermControl& control) _activePane = _rootPane; - // TODO: Add an event handler to this pane's GotFocus event. - // When that pane gains focus, we'll mark it as the new active pane. - _rootPane->pfnGotFocus = ([this](std::shared_ptr sender) { - // _rootPane->GotFocus([this](std::shared_ptr sender, auto&&) { + // Add an event handler to this pane's GotFocus event. When that pane gains + // focus, we'll mark it as the new active pane. This pane will propogate + // this function down as it is split, so only leaves will trigger this. + _rootPane->SetGotFocusCallback([this](std::shared_ptr sender) { _rootPane->ClearActive(); _activePane = sender; _activePane->SetActive(); @@ -58,10 +58,7 @@ Tab::Tab(const GUID& profile, const TermControl& control) } }); - control.TitleChanged([this](auto newTitle) { - auto newTabTitle = this->GetFocusedTitle(); - this->SetTabText(newTabTitle); - }); + _AttachEventHandersToControl(control); _MakeTabViewItem(); } @@ -273,26 +270,8 @@ bool Tab::CanSplitPane(Pane::SplitState splitType) void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl& control) { _activePane->Split(splitType, profile, control); - // auto [firstNewPane, secondNewPane] = _activePane->Split(splitType, profile, control); - - // // TODO: Add an event handler to this pane's GotFocus event. - // // When that pane gains focus, we'll mark it as the new active pane. - // firstNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { - // // firstNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { - // _rootPane->ClearActive(); - // _activePane = sender; - // _activePane->SetActive(); - // }); - // secondNewPane->pfnGotFocus = ([this](std::shared_ptr sender) { - // // secondNewPane->GotFocus([this](std::shared_ptr sender, auto&&) { - // _rootPane->ClearActive(); - // _activePane = sender; - // _activePane->SetActive(); - // }); - control.TitleChanged([this](auto newTitle) { - auto newTabTitle = this->GetFocusedTitle(); - this->SetTabText(newTabTitle); - }); + + _AttachEventHandersToControl(control); } // Method Description: @@ -345,4 +324,12 @@ void Tab::ClosePane() focused->Close(); } +void Tab::_AttachEventHandersToControl(const TermControl& control) +{ + control.TitleChanged([this](auto newTitle) { + auto newTabTitle = this->GetFocusedTitle(); + this->SetTabText(newTabTitle); + }); +} + DEFINE_EVENT(Tab, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index f8dc9f88d03..a13ed717634 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -49,4 +49,6 @@ class Tab void _MakeTabViewItem(); void _Focus(); + + void _AttachEventHandersToControl(const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); }; From 30fc8811e6019a974f8021d31b6ddf5f7fa5ff61 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 10:01:23 -0600 Subject: [PATCH 05/14] Propogate the events back up on close --- src/cascadia/TerminalApp/Pane.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 64040c66670..c68a8517eb5 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -596,6 +596,12 @@ void Pane::_CloseChild(const bool closeFirst) // Add our new event handler before revoking the old one. _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); + // Take the got focus callback back into ourselves, and clear it from + // our (abandoned) children. + _pfnGotFocus = remainingChild->_pfnGotFocus; + _firstChild->_pfnGotFocus = nullptr; + _secondChild->_pfnGotFocus = nullptr; + // Revoke the old event handlers. Remove both the handlers for the panes // themselves closing, and remove their handlers for their controls // closing. At this point, if the remaining child's control is closed, @@ -664,6 +670,12 @@ void Pane::_CloseChild(const bool closeFirst) _firstChild = remainingChild->_firstChild; _secondChild = remainingChild->_secondChild; + // Remove the got focus callback from the closed child. + // Our new children should both still have their old callback values. + // As we aren't a leaf, ours should definitely be null. + closedChild->_pfnGotFocus = nullptr; + _pfnGotFocus = nullptr; + // Set up new close handlers on the children _SetupChildCloseHandlers(); From 9cfebf1686c6b914e65abd7aefece2345b50339a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 10:28:28 -0600 Subject: [PATCH 06/14] Encapsulate Tab::pfnFocusChanged, and clean up TerminalPage a bit --- .../TerminalApp/AppActionHandlers.cpp | 2 +- src/cascadia/TerminalApp/Pane.cpp | 10 +-- src/cascadia/TerminalApp/Pane.h | 3 +- src/cascadia/TerminalApp/Tab.cpp | 31 ++++---- src/cascadia/TerminalApp/Tab.h | 7 +- src/cascadia/TerminalApp/TerminalPage.cpp | 76 +++++++------------ src/cascadia/TerminalApp/TerminalPage.h | 2 +- 7 files changed, 56 insertions(+), 75 deletions(-) diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 2a28e0134ff..0671fe5a047 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -193,7 +193,7 @@ namespace winrt::TerminalApp::implementation { if (const auto& realArgs = args.ActionArgs().try_as()) { - const auto termControl = _GetFocusedControl(); + const auto termControl = _GetActiveControl(); termControl.AdjustFontSize(realArgs.Delta()); args.Handled(true); } diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index c68a8517eb5..d6869041593 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -389,20 +389,16 @@ std::shared_ptr Pane::GetFocusedPane() } // Method Description: -// - Returns nullptr if no children of this pane were the last control to be -// focused, or the TermControl that _was_ the last control to be focused (if -// there was one). -// - This control might not currently be focused, if the tab itself is not -// currently focused. +// - TODO // Arguments: // - // Return Value: // - nullptr if no children were marked `_lastFocused`, else the TermControl // that was last focused. -TermControl Pane::GetFocusedTerminalControl() +TermControl Pane::GetTerminalControl() { auto lastFocused = GetFocusedPane(); - return lastFocused ? lastFocused->_control : nullptr; + return _IsLeaf() ? _control : nullptr; } void Pane::ClearActive() diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index e083abe45d6..5e7a32a44e9 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -48,7 +48,8 @@ class Pane : public std::enable_shared_from_this const bool lastFocused = false); std::shared_ptr GetFocusedPane(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + // winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); std::optional GetFocusedProfile(); winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 0baed9e8492..cf2469549e5 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -50,9 +50,9 @@ Tab::Tab(const GUID& profile, const TermControl& control) // } // } // } - if (pfnFocusChanged) + if (_pfnActivePaneChanged) { - pfnFocusChanged(); + _pfnActivePaneChanged(); auto newTabTitle = this->GetFocusedTitle(); this->SetTabText(newTabTitle); } @@ -84,12 +84,9 @@ UIElement Tab::GetRootElement() // Return Value: // - nullptr if no children were marked `_lastFocused`, else the TermControl // that was last focused. -TermControl Tab::GetFocusedTerminalControl() const +TermControl Tab::GetActiveTerminalControl() const { - // TODO: Probably just GetTerminalControl() - return _activePane->GetFocusedTerminalControl(); - - // return _rootPane->GetFocusedTerminalControl(); + return _activePane->GetTerminalControl(); } winrt::MUX::Controls::TabViewItem Tab::GetTabViewItem() @@ -164,7 +161,7 @@ void Tab::_Focus() { _focused = true; - auto lastFocusedControl = GetFocusedTerminalControl(); + auto lastFocusedControl = GetActiveTerminalControl(); if (lastFocusedControl) { lastFocusedControl.Focus(FocusState::Programmatic); @@ -210,8 +207,8 @@ void Tab::UpdateIcon(const winrt::hstring iconPath) // - the title string of the last focused terminal control in our tree. winrt::hstring Tab::GetFocusedTitle() const { - // const auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); - const auto lastFocusedControl = GetFocusedTerminalControl(); + // const auto lastFocusedControl = _rootPane->GetActiveTerminalControl(); + const auto lastFocusedControl = GetActiveTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; } @@ -240,7 +237,7 @@ void Tab::SetTabText(const winrt::hstring& text) // - void Tab::Scroll(const int delta) { - auto control = GetFocusedTerminalControl(); + auto control = GetActiveTerminalControl(); control.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [control, delta]() { const auto currentOffset = control.GetScrollOffset(); control.KeyboardScrollViewport(currentOffset + delta); @@ -327,9 +324,17 @@ void Tab::ClosePane() void Tab::_AttachEventHandersToControl(const TermControl& control) { control.TitleChanged([this](auto newTitle) { - auto newTabTitle = this->GetFocusedTitle(); - this->SetTabText(newTabTitle); + // The title of the control changed, but not necessarily the title + // of the tab. Get the title of the active pane of the tab, and set + // the tab's text to the active panes' text. + auto newTabTitle = GetFocusedTitle(); + SetTabText(newTabTitle); }); } +void Tab::SetActivePaneChangedCallback(std::function pfnActivePaneChanged) +{ + _pfnActivePaneChanged = pfnActivePaneChanged; +} + DEFINE_EVENT(Tab, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index a13ed717634..1565787c70c 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -12,7 +12,7 @@ class Tab winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); winrt::Windows::UI::Xaml::UIElement GetRootElement(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl() const; + winrt::Microsoft::Terminal::TerminalControl::TermControl GetActiveTerminalControl() const; std::optional GetFocusedProfile() const noexcept; bool IsFocused() const noexcept; @@ -36,7 +36,8 @@ class Tab void ClosePane(); - std::function pfnFocusChanged{ nullptr }; + void SetActivePaneChangedCallback(std::function pfnActivePaneChanged); + DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); private: @@ -47,6 +48,8 @@ class Tab bool _focused{ false }; winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem{ nullptr }; + std::function _pfnActivePaneChanged{ nullptr }; + void _MakeTabViewItem(); void _Focus(); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 2a981730780..7c680f8b631 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -416,15 +416,31 @@ namespace winrt::TerminalApp::implementation // Add the new tab to the list of our tabs. auto newTab = _tabs.emplace_back(std::make_shared(profileGuid, term)); - const auto* const profile = _settings->FindProfile(profileGuid); - // Hookup our event handlers to the new terminal _RegisterTerminalEvents(term, newTab); + // Don't capture a strong ref to the tab. If the tab is removed as this + // is called, we don't really care anymore about handling the event. + std::weak_ptr weakTabPtr = newTab; + // When the tab's active pane changes, we'll want to lookup a new icon + // for it, and possibly propogate the title up to the window. + newTab->SetActivePaneChangedCallback([this, weakTabPtr]() { + if (auto tab = weakTabPtr.lock()) + { + // Possibly update the icon of the tab. + _UpdateTabIcon(tab); + + // Possibly update the title of the tab, window to match the newly + // focused pane. + _UpdateTitle(tab); + } + }); + auto tabViewItem = newTab->GetTabViewItem(); _tabView.TabItems().Append(tabViewItem); - // Set this profile's tab to the icon the user specified + // Set this tab's icon to the icon from the user's profile + const auto* const profile = _settings->FindProfile(profileGuid); if (profile != nullptr && profile->HasIcon()) { newTab->UpdateIcon(profile->GetExpandedIconPath()); @@ -707,6 +723,7 @@ namespace winrt::TerminalApp::implementation // handle. This includes: // * the Copy and Paste events, for setting and retrieving clipboard data // on the right thread + // TODO: This description is outdated // * the TitleChanged event, for changing the text of the tab // * the GotFocus event, for changing the title/icon in the tab when a new // control is focused @@ -721,47 +738,6 @@ namespace winrt::TerminalApp::implementation // Add an event handler when the terminal wants to paste data from the Clipboard. term.PasteFromClipboard({ this, &TerminalPage::_PasteFromClipboardHandler }); - - // Don't capture a strong ref to the tab. If the tab is removed as this - // is called, we don't really care anymore about handling the event. - std::weak_ptr weakTabPtr = hostingTab; - // term.TitleChanged([this, weakTabPtr](auto newTitle) { - // auto tab = weakTabPtr.lock(); - // if (!tab) - // { - // return; - // } - // // The title of the control changed, but not necessarily the title - // // of the tab. Get the title of the focused pane of the tab, and set - // // the tab's text to the focused panes' text. - // _UpdateTitle(tab); - // }); - - hostingTab->pfnFocusChanged = ([this, weakTabPtr]() { - auto tab = weakTabPtr.lock(); - // Possibly update the icon of the tab. - _UpdateTabIcon(tab); - - // Possibly update the title of the tab, window to match the newly - // focused pane. - _UpdateTitle(tab); - }); - // term.GotFocus([this, weakTabPtr](auto&&, auto&&) { - // auto tab = weakTabPtr.lock(); - // if (!tab) - // { - // return; - // } - // // // Update the focus of the tab's panes - // // tab->UpdateFocus(); - - // // Possibly update the title of the tab, window to match the newly - // // focused pane. - // _UpdateTitle(tab); - - // // Possibly update the icon of the tab. - // _UpdateTabIcon(tab); - // }); } // Method Description: @@ -806,11 +782,11 @@ namespace winrt::TerminalApp::implementation _tabs[focusedTabIndex]->NavigateFocus(direction); } - winrt::Microsoft::Terminal::TerminalControl::TermControl TerminalPage::_GetFocusedControl() + winrt::Microsoft::Terminal::TerminalControl::TermControl TerminalPage::_GetActiveControl() { int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; - return focusedTab->GetFocusedTerminalControl(); + return focusedTab->GetActiveTerminalControl(); } // Method Description: @@ -987,7 +963,7 @@ namespace winrt::TerminalApp::implementation { delta = std::clamp(delta, -1, 1); const auto focusedTabIndex = _GetFocusedTabIndex(); - const auto control = _GetFocusedControl(); + const auto control = _GetActiveControl(); const auto termHeight = control.GetViewHeight(); _tabs[focusedTabIndex]->Scroll(termHeight * delta); } @@ -1008,7 +984,7 @@ namespace winrt::TerminalApp::implementation { try { - if (auto focusedControl{ _GetFocusedControl() }) + if (auto focusedControl{ _GetActiveControl() }) { return focusedControl.Title(); } @@ -1164,7 +1140,7 @@ namespace winrt::TerminalApp::implementation // - true iff we we able to copy text (if a selection was active) bool TerminalPage::_CopyText(const bool trimTrailingWhitespace) { - const auto control = _GetFocusedControl(); + const auto control = _GetActiveControl(); return control.CopySelectionToClipboard(trimTrailingWhitespace); } @@ -1172,7 +1148,7 @@ namespace winrt::TerminalApp::implementation // - Paste text from the Windows Clipboard to the focused terminal void TerminalPage::_PasteText() { - const auto control = _GetFocusedControl(); + const auto control = _GetActiveControl(); control.PasteTextFromClipboard(); } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index ee7a9b2b17e..9222b3e7820 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -94,7 +94,7 @@ namespace winrt::TerminalApp::implementation bool _SelectTab(const int tabIndex); void _MoveFocus(const Direction& direction); - winrt::Microsoft::Terminal::TerminalControl::TermControl _GetFocusedControl(); + winrt::Microsoft::Terminal::TerminalControl::TermControl _GetActiveControl(); int _GetFocusedTabIndex() const; void _SetFocusedTabIndex(int tabIndex); void _CloseFocusedTab(); From b61e6fe38dbced9ee41408943d425c0a3d42e92b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 11:11:00 -0600 Subject: [PATCH 07/14] Mostly just code cleanup, commenting --- src/cascadia/TerminalApp/Pane.cpp | 107 ++++++++++++---------- src/cascadia/TerminalApp/Pane.h | 7 +- src/cascadia/TerminalApp/Tab.cpp | 19 ++++ src/cascadia/TerminalApp/TerminalPage.cpp | 11 +-- 4 files changed, 84 insertions(+), 60 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index d6869041593..0546b7ca3e2 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -21,7 +21,7 @@ winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_focusedBorderBrush = { Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocused) : _control{ control }, - _lastFocused{ lastFocused }, + _lastActive{ lastFocused }, _profile{ profile } { _root.Children().Append(_border); @@ -189,8 +189,8 @@ bool Pane::ResizePane(const Direction& direction) // If it is, and the requested resize direction matches our separator, then // we're the pane that needs to adjust its separator. // If our separator is the wrong direction, then we can't handle it. - const bool firstIsFocused = _firstChild->_IsLeaf() && _firstChild->_lastFocused; - const bool secondIsFocused = _secondChild->_IsLeaf() && _secondChild->_lastFocused; + const bool firstIsFocused = _firstChild->_IsLeaf() && _firstChild->_lastActive; + const bool secondIsFocused = _secondChild->_IsLeaf() && _secondChild->_lastActive; if (firstIsFocused || secondIsFocused) { return _Resize(direction); @@ -248,7 +248,7 @@ bool Pane::_NavigateFocus(const Direction& direction) // Transfer focus to our child, and update the focus of our tree. newlyFocusedChild->_FocusFirstChild(); - UpdateFocus(); + UpdateVisuals(); return true; } @@ -278,8 +278,8 @@ bool Pane::NavigateFocus(const Direction& direction) // Check if either our first or second child is the currently focused leaf. // If it is, and the requested move direction matches our separator, then // we're the pane that needs to handle this focus move. - const bool firstIsFocused = _firstChild->_IsLeaf() && _firstChild->_lastFocused; - const bool secondIsFocused = _secondChild->_IsLeaf() && _secondChild->_lastFocused; + const bool firstIsFocused = _firstChild->_IsLeaf() && _firstChild->_lastActive; + const bool secondIsFocused = _secondChild->_IsLeaf() && _secondChild->_lastActive; if (firstIsFocused || secondIsFocused) { return _NavigateFocus(direction); @@ -372,50 +372,63 @@ Controls::Grid Pane::GetRootElement() // not currently focused. // Return Value: // - nullptr if we're a leaf and unfocused, or no children were marked -// `_lastFocused`, else returns this -std::shared_ptr Pane::GetFocusedPane() +// `_lastActive`, else returns this +std::shared_ptr Pane::GetActivePane() { if (_IsLeaf()) { - return _lastFocused ? shared_from_this() : nullptr; + return _lastActive ? shared_from_this() : nullptr; } - auto firstFocused = _firstChild->GetFocusedPane(); + auto firstFocused = _firstChild->GetActivePane(); if (firstFocused != nullptr) { return firstFocused; } - return _secondChild->GetFocusedPane(); + return _secondChild->GetActivePane(); } // Method Description: -// - TODO +// - Gets the TermControl of this pane. If this Pane is not a leaf, this will return nullptr. // Arguments: // - // Return Value: -// - nullptr if no children were marked `_lastFocused`, else the TermControl -// that was last focused. +// - nullptr if this Pane is a parent, otherwise the TermControl of this Pane. TermControl Pane::GetTerminalControl() { - auto lastFocused = GetFocusedPane(); return _IsLeaf() ? _control : nullptr; } +// Method Description: +// - Recursively remove the "Active" state from this Pane and all it's children. +// - Updates our visuals to match our new state, including highlighting our borders. +// Arguments: +// - +// Return Value: +// - void Pane::ClearActive() { - _lastFocused = false; + _lastActive = false; if (!_IsLeaf()) { _firstChild->ClearActive(); _secondChild->ClearActive(); } - UpdateFocus(); + UpdateVisuals(); } +// Method Description: +// - Sets the "Active" state on this Pane. Only one Pane in a tree of Panes +// should be "active", and that pane should be a leaf. +// - Updates our visuals to match our new state, including highlighting our borders. +// Arguments: +// - +// Return Value: +// - void Pane::SetActive() { - _lastFocused = true; - UpdateFocus(); + _lastActive = true; + UpdateVisuals(); } // Method Description: @@ -429,7 +442,7 @@ void Pane::SetActive() // focused, else the GUID of the profile of the last control to be focused std::optional Pane::GetFocusedProfile() { - auto lastFocused = GetFocusedPane(); + auto lastFocused = GetActivePane(); return lastFocused ? lastFocused->_profile : std::nullopt; } @@ -441,7 +454,7 @@ std::optional Pane::GetFocusedProfile() // - true iff we were the last pane focused in this tree of panes. bool Pane::WasLastFocused() const noexcept { - return _lastFocused; + return _lastActive; } // Method Description: @@ -468,39 +481,21 @@ bool Pane::_HasFocusedChild() const noexcept // We're intentionally making this one giant expression, so the compiler // will skip the following lookups if one of the lookups before it returns // true - return (_control && _control.FocusState() != FocusState::Unfocused) || + return (_control && _lastActive) || (_firstChild && _firstChild->_HasFocusedChild()) || (_secondChild && _secondChild->_HasFocusedChild()); } // Method Description: -// - Update the focus state of this pane, and all its descendants. -// * If this is a leaf node, and our control is actively focused, we'll mark -// ourselves as the _lastFocused. -// * If we're not a leaf, we'll recurse on our children to check them. +// - Update the focus state of this pane. We'll make sure to colorize our +// borders depending on if we are the active pane or not. // Arguments: // - // Return Value: // - -void Pane::UpdateFocus() +void Pane::UpdateVisuals() { - _border.BorderBrush(_lastFocused ? s_focusedBorderBrush : nullptr); - // // TODO: Do we need this? - // if (_IsLeaf()) - // { - // const auto controlFocused = _control && - // _control.FocusState() != FocusState::Unfocused; - - // _lastFocused = controlFocused; - - // } - // else - // { - // _lastFocused = false; - - // _firstChild->UpdateFocus(); - // _secondChild->UpdateFocus(); - // } + _border.BorderBrush(_lastActive ? s_focusedBorderBrush : nullptr); } // Method Description: @@ -609,7 +604,7 @@ void Pane::_CloseChild(const bool closeFirst) // If either of our children was focused, we want to take that focus from // them. - _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; + _lastActive = _firstChild->_lastActive || _secondChild->_lastActive; // Remove all the ui elements of our children. This'll make sure we can // re-attach the TermControl to our Grid. @@ -628,7 +623,7 @@ void Pane::_CloseChild(const bool closeFirst) _root.Children().Append(_border); _border.Child(_control); - if (_lastFocused) + if (_lastActive) { _control.Focus(FocusState::Programmatic); } @@ -724,7 +719,7 @@ void Pane::_CloseChild(const bool closeFirst) _secondChild->_UpdateBorders(); // If the closed child was focused, transfer the focus to it's first sibling. - if (closedChild->_lastFocused) + if (closedChild->_lastActive) { _FocusFirstChild(); } @@ -1019,7 +1014,7 @@ void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& // Register event handlers on our children to handle their Close events _SetupChildCloseHandlers(); - _lastFocused = false; + _lastActive = false; _firstChild->_pfnGotFocus = _pfnGotFocus; _secondChild->_pfnGotFocus = _pfnGotFocus; @@ -1085,6 +1080,14 @@ Size Pane::_GetMinSize() const } } +// Event Description: +// - Called when our control gains focus. We'll use this to trigger our GotFocus +// callback. The tab that's hosting us should have registered a callback which +// can be used to mark us as active. +// Arguments: +// - +// Return Value: +// - void Pane::_ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable const& /* sender */, RoutedEventArgs const& /* args */) { @@ -1094,6 +1097,14 @@ void Pane::_ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable cons } } +// Method Description: +// - Register a callback function to be called when this Pane's control gains +// focused. This is used by the Tab hosting us so it can track which Pane in +// the tree was the last one to be focused. +// Arguments: +// - pfnGotFocus: A function that should be called when this pane's control gets focus. +// Return Value: +// - void Pane::SetGotFocusCallback(std::function)> pfnGotFocus) { _pfnGotFocus = pfnGotFocus; diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 5e7a32a44e9..adb3807b33d 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -47,15 +47,14 @@ class Pane : public std::enable_shared_from_this const winrt::Microsoft::Terminal::TerminalControl::TermControl& control, const bool lastFocused = false); - std::shared_ptr GetFocusedPane(); - // winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + std::shared_ptr GetActivePane(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); std::optional GetFocusedProfile(); winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); bool WasLastFocused() const noexcept; - void UpdateFocus(); + void UpdateVisuals(); void ClearActive(); void SetActive(); @@ -88,7 +87,7 @@ class Pane : public std::enable_shared_from_this std::optional _firstPercent{ std::nullopt }; std::optional _secondPercent{ std::nullopt }; - bool _lastFocused{ false }; + bool _lastActive{ false }; std::optional _profile{ std::nullopt }; winrt::event_token _connectionClosedToken{ 0 }; winrt::event_token _firstClosedToken{ 0 }; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index cf2469549e5..0d17e7b474e 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -321,6 +321,16 @@ void Tab::ClosePane() focused->Close(); } +// Method Description: +// - Register any event handlers that we may need with the given TermControl. +// This should be called on each and every TermControl that we add to the tree +// of Panes in this tab. We'll add events too: +// * notify us when the control's title changed, so we can update our own +// title (if necessary) +// Arguments: +// - control: the TermControl to add events to. +// Return Value: +// - void Tab::_AttachEventHandersToControl(const TermControl& control) { control.TitleChanged([this](auto newTitle) { @@ -332,6 +342,15 @@ void Tab::_AttachEventHandersToControl(const TermControl& control) }); } +// Method Description: +// - Register a callback function to be called when this Tab's active pane +// changes. The App will use this to lookup the appropriate icon for this Tab +// (based on the newly-active pane) and also to possibly propogate the new +// Pane's title to the window. +// Arguments: +// - pfnGotFocus: A function that should be called when this Tab's active pane changes +// Return Value: +// - void Tab::SetActivePaneChangedCallback(std::function pfnActivePaneChanged) { _pfnActivePaneChanged = pfnActivePaneChanged; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 7c680f8b631..4c83ca6416b 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -599,15 +599,14 @@ namespace winrt::TerminalApp::implementation } // Method Description: - // - Get the title of the currently focused terminal control, and set it's - // tab's text to that text. If this tab is the focused tab, then also - // bubble this title to any listeners of our TitleChanged event. + // - Get the title of the currently focused terminal control. If this tab is + // the focused tab, then also bubble this title to any listeners of our + // TitleChanged event. // Arguments: // - tab: the Tab to update the title for. void TerminalPage::_UpdateTitle(std::shared_ptr tab) { auto newTabTitle = tab->GetFocusedTitle(); - // tab->SetTabText(newTabTitle); if (_settings->GlobalSettings().GetShowTitleInTitlebar() && tab->IsFocused()) @@ -723,10 +722,6 @@ namespace winrt::TerminalApp::implementation // handle. This includes: // * the Copy and Paste events, for setting and retrieving clipboard data // on the right thread - // TODO: This description is outdated - // * the TitleChanged event, for changing the text of the tab - // * the GotFocus event, for changing the title/icon in the tab when a new - // control is focused // Arguments: // - term: The newly created TermControl to connect the events for // - hostingTab: The Tab that's hosting this TermControl instance From e358e0dcba13509b4b60ffb4638b92b4d720f57b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 11:31:10 -0600 Subject: [PATCH 08/14] This works to hittest on the borders If the border is `Transparent`, then it can't hittest for Tapped events, and it'll fall through (to someone) THis at least works, but looks garish --- src/cascadia/TerminalApp/Pane.cpp | 45 ++++++++++++++++++++++++++++--- src/cascadia/TerminalApp/Pane.h | 1 + src/cascadia/TerminalApp/Tab.cpp | 3 +-- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 0546b7ca3e2..542de502837 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -18,6 +18,7 @@ static const int CombinedPaneBorderSize = 2 * PaneBorderSize; static const float Half = 0.50f; winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_focusedBorderBrush = { nullptr }; +winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::s_unfocusedBorderBrush = { nullptr }; Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocused) : _control{ control }, @@ -54,16 +55,54 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus // If SystemAccentColor is _not_ a Color for some reason, use // Transparent as the color, so we don't do this process again on // the next pane (by leaving s_focusedBorderBrush nullptr) - auto actualColor = winrt::unbox_value_or(colorFromResources, Colors::Transparent()); + auto actualColor = winrt::unbox_value_or(colorFromResources, Colors::Black()); s_focusedBorderBrush = SolidColorBrush(actualColor); } else { - s_focusedBorderBrush = SolidColorBrush{ Colors::Transparent() }; + // DON'T use Transparent here - if it's "Transparent", then it won't + // be able to hittest for clicks, and then clicking on the border + // will eat focus. + s_focusedBorderBrush = SolidColorBrush{ Colors::Black() }; } } + //////////////////////////////////////////////////////////////////////////// + if (s_unfocusedBorderBrush == nullptr) + { + const auto accentColorKey = winrt::box_value(L"SystemAccentColorDark3"); + if (res.HasKey(accentColorKey)) + { + const auto colorFromResources = res.Lookup(accentColorKey); + // If SystemAccentColor is _not_ a Color for some reason, use + // Transparent as the color, so we don't do this process again on + // the next pane (by leaving s_unfocusedBorderBrush nullptr) + auto actualColor = winrt::unbox_value_or(colorFromResources, Colors::Black()); + s_unfocusedBorderBrush = SolidColorBrush(actualColor); + } + else + { + // DON'T use Transparent here - if it's "Transparent", then it won't + // be able to hittest for clicks, and then clicking on the border + // will eat focus. + s_unfocusedBorderBrush = SolidColorBrush{ Colors::Black() }; + } + } + //////////////////////////////////////////////////////////////////////////// + _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, { this, &Pane::_ControlGotFocusHandler }); + + _border.Tapped([this](auto&, auto& e) { + _FocusFirstChild(); + e.Handled(true); + }); + // _border.IsTabStop(true); // Apparently not a property + + // // This does nothing?: + // _border.GotFocus([this](auto&&, auto&&) { + // // _control.Focus(FocusState::Programmatic); + // _FocusFirstChild(); + // }); } // Method Description: @@ -495,7 +534,7 @@ bool Pane::_HasFocusedChild() const noexcept // - void Pane::UpdateVisuals() { - _border.BorderBrush(_lastActive ? s_focusedBorderBrush : nullptr); + _border.BorderBrush(_lastActive ? s_focusedBorderBrush : s_unfocusedBorderBrush); } // Method Description: diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index adb3807b33d..ebe6151e5e4 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -80,6 +80,7 @@ class Pane : public std::enable_shared_from_this winrt::Windows::UI::Xaml::Controls::Border _border{}; winrt::Microsoft::Terminal::TerminalControl::TermControl _control{ nullptr }; static winrt::Windows::UI::Xaml::Media::SolidColorBrush s_focusedBorderBrush; + static winrt::Windows::UI::Xaml::Media::SolidColorBrush s_unfocusedBorderBrush; std::shared_ptr _firstChild{ nullptr }; std::shared_ptr _secondChild{ nullptr }; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 0d17e7b474e..e52f636f464 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -317,8 +317,7 @@ void Tab::NavigateFocus(const winrt::TerminalApp::Direction& direction) // - void Tab::ClosePane() { - auto focused = _activePane->GetFocusedPane(); - focused->Close(); + _activePane->Close(); } // Method Description: From b40e1905bff93f37ae0c3e36a68252be170e40ce Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 11:52:28 -0600 Subject: [PATCH 09/14] Match the pane border to the TabViewHeader --- src/cascadia/TerminalApp/Pane.cpp | 104 ++++++++++++++++-------------- src/cascadia/TerminalApp/Pane.h | 2 + 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 542de502837..1e4ff64d554 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -32,7 +32,7 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus // Set the background of the pane to match that of the theme's default grid // background. This way, we'll match the small underline under the tabs, and - // the UI will be consistent on bot light and dark modes. + // the UI will be consistent on both light and dark modes. const auto res = Application::Current().Resources(); const auto key = winrt::box_value(L"BackgroundGridThemeStyle"); if (res.HasKey(key)) @@ -46,63 +46,25 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus } } - if (s_focusedBorderBrush == nullptr) + // On the first Pane's creation, lookup resources we'll use to theme the + // Pane, including the brushed to use for the focused/unfocused border + // color. + if (s_focusedBorderBrush == nullptr || s_unfocusedBorderBrush == nullptr) { - const auto accentColorKey = winrt::box_value(L"SystemAccentColor"); - if (res.HasKey(accentColorKey)) - { - const auto colorFromResources = res.Lookup(accentColorKey); - // If SystemAccentColor is _not_ a Color for some reason, use - // Transparent as the color, so we don't do this process again on - // the next pane (by leaving s_focusedBorderBrush nullptr) - auto actualColor = winrt::unbox_value_or(colorFromResources, Colors::Black()); - s_focusedBorderBrush = SolidColorBrush(actualColor); - } - else - { - // DON'T use Transparent here - if it's "Transparent", then it won't - // be able to hittest for clicks, and then clicking on the border - // will eat focus. - s_focusedBorderBrush = SolidColorBrush{ Colors::Black() }; - } + _SetupResources(); } - //////////////////////////////////////////////////////////////////////////// - if (s_unfocusedBorderBrush == nullptr) - { - const auto accentColorKey = winrt::box_value(L"SystemAccentColorDark3"); - if (res.HasKey(accentColorKey)) - { - const auto colorFromResources = res.Lookup(accentColorKey); - // If SystemAccentColor is _not_ a Color for some reason, use - // Transparent as the color, so we don't do this process again on - // the next pane (by leaving s_unfocusedBorderBrush nullptr) - auto actualColor = winrt::unbox_value_or(colorFromResources, Colors::Black()); - s_unfocusedBorderBrush = SolidColorBrush(actualColor); - } - else - { - // DON'T use Transparent here - if it's "Transparent", then it won't - // be able to hittest for clicks, and then clicking on the border - // will eat focus. - s_unfocusedBorderBrush = SolidColorBrush{ Colors::Black() }; - } - } - //////////////////////////////////////////////////////////////////////////// - + // Register an event with the control to have it inform us when it gains focus. _gotFocusRevoker = control.GotFocus(winrt::auto_revoke, { this, &Pane::_ControlGotFocusHandler }); + // When our border is tapped, make sure to transfer focus to our control. + // LOAD-BEARING: This will NOT work if the border's BorderBrush is set to + // Colors::Transparent! The border won't get Tapped events, and they'll fall + // through to something else. _border.Tapped([this](auto&, auto& e) { _FocusFirstChild(); e.Handled(true); }); - // _border.IsTabStop(true); // Apparently not a property - - // // This does nothing?: - // _border.GotFocus([this](auto&&, auto&&) { - // // _control.Focus(FocusState::Programmatic); - // _FocusFirstChild(); - // }); } // Method Description: @@ -1149,4 +1111,48 @@ void Pane::SetGotFocusCallback(std::function)> pfnGot _pfnGotFocus = pfnGotFocus; } +// Function Description: +// - Attempts to load some XAML resources that the Pane will need. This includes: +// * The Color we'll use for active Panes's borders - SystemAccentColor +// * The Brush we'll use for inactive Panes - TabViewBackground (to match the +// color of the titlebar) +// Arguments: +// - +// Return Value: +// - +void Pane::_SetupResources() +{ + const auto accentColorKey = winrt::box_value(L"SystemAccentColor"); + if (res.HasKey(accentColorKey)) + { + const auto colorFromResources = res.Lookup(accentColorKey); + // If SystemAccentColor is _not_ a Color for some reason, use + // Transparent as the color, so we don't do this process again on + // the next pane (by leaving s_focusedBorderBrush nullptr) + auto actualColor = winrt::unbox_value_or(colorFromResources, Colors::Black()); + s_focusedBorderBrush = SolidColorBrush(actualColor); + } + else + { + // DON'T use Transparent here - if it's "Transparent", then it won't + // be able to hittest for clicks, and then clicking on the border + // will eat focus. + s_focusedBorderBrush = SolidColorBrush{ Colors::Black() }; + } + + const auto accentColorKey = winrt::box_value(L"TabViewBackground"); + if (res.HasKey(accentColorKey)) + { + winrt::Windows::Foundation::IInspectable obj = res.Lookup(accentColorKey); + s_unfocusedBorderBrush = obj.try_as(); + } + else + { + // DON'T use Transparent here - if it's "Transparent", then it won't + // be able to hittest for clicks, and then clicking on the border + // will eat focus. + s_unfocusedBorderBrush = SolidColorBrush{ Colors::Black() }; + } +} + DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index ebe6151e5e4..fae9ab85b54 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -163,4 +163,6 @@ class Pane : public std::enable_shared_from_this } return false; } + + static void _SetupResources(); }; From b585950312830c19dbcb2d638c8d1ee31bc0afe4 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 12 Nov 2019 12:01:24 -0600 Subject: [PATCH 10/14] Fix a bit of dead code and a bad copy-pasta --- src/cascadia/TerminalApp/Pane.cpp | 21 +++----------------- src/cascadia/TerminalApp/Tab.cpp | 33 ------------------------------- src/cascadia/TerminalApp/Tab.h | 1 - 3 files changed, 3 insertions(+), 52 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 1e4ff64d554..93ead95291a 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -30,22 +30,6 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); - // Set the background of the pane to match that of the theme's default grid - // background. This way, we'll match the small underline under the tabs, and - // the UI will be consistent on both light and dark modes. - const auto res = Application::Current().Resources(); - const auto key = winrt::box_value(L"BackgroundGridThemeStyle"); - if (res.HasKey(key)) - { - const auto g = res.Lookup(key); - const auto style = g.try_as(); - // try_as fails by returning nullptr - if (style) - { - _root.Style(style); - } - } - // On the first Pane's creation, lookup resources we'll use to theme the // Pane, including the brushed to use for the focused/unfocused border // color. @@ -1122,6 +1106,7 @@ void Pane::SetGotFocusCallback(std::function)> pfnGot // - void Pane::_SetupResources() { + const auto res = Application::Current().Resources(); const auto accentColorKey = winrt::box_value(L"SystemAccentColor"); if (res.HasKey(accentColorKey)) { @@ -1140,10 +1125,10 @@ void Pane::_SetupResources() s_focusedBorderBrush = SolidColorBrush{ Colors::Black() }; } - const auto accentColorKey = winrt::box_value(L"TabViewBackground"); + const auto tabViewBackgroundKey = winrt::box_value(L"TabViewBackground"); if (res.HasKey(accentColorKey)) { - winrt::Windows::Foundation::IInspectable obj = res.Lookup(accentColorKey); + winrt::Windows::Foundation::IInspectable obj = res.Lookup(tabViewBackgroundKey); s_unfocusedBorderBrush = obj.try_as(); } else diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index e52f636f464..5141a9eb8e3 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -33,23 +33,6 @@ Tab::Tab(const GUID& profile, const TermControl& control) _activePane = sender; _activePane->SetActive(); - // // void TerminalPage::_UpdateTabIcon(std::shared_ptr tab) - // { - // const auto lastFocusedProfileOpt = this->GetFocusedProfile(); - // if (lastFocusedProfileOpt.has_value()) - // { - // const auto lastFocusedProfile = lastFocusedProfileOpt.value(); - // const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile); - // if (matchingProfile) - // { - // tab->UpdateIcon(matchingProfile->GetExpandedIconPath()); - // } - // else - // { - // tab->UpdateIcon({}); - // } - // } - // } if (_pfnActivePaneChanged) { _pfnActivePaneChanged(); @@ -168,21 +151,6 @@ void Tab::_Focus() } } -// Method Description: -// - Update the focus state of this tab's tree of panes. If one of the controls -// under this tab is focused, then it will be marked as the last focused. If -// there are no focused panes, then there will not be a last focused control -// when this returns. -// Arguments: -// - -// Return Value: -// - -void Tab::UpdateFocus() -{ - // _rootPane->UpdateFocus(); - // Maybe root->ClearActive, active->SetActive -} - void Tab::UpdateIcon(const winrt::hstring iconPath) { // Don't reload our icon if it hasn't changed. @@ -207,7 +175,6 @@ void Tab::UpdateIcon(const winrt::hstring iconPath) // - the title string of the last focused terminal control in our tree. winrt::hstring Tab::GetFocusedTitle() const { - // const auto lastFocusedControl = _rootPane->GetActiveTerminalControl(); const auto lastFocusedControl = GetActiveTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 1565787c70c..e43fc1f36fa 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -23,7 +23,6 @@ class Tab bool CanSplitPane(Pane::SplitState splitType); void SplitPane(Pane::SplitState splitType, const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control); - void UpdateFocus(); void UpdateIcon(const winrt::hstring iconPath); void ResizeContent(const winrt::Windows::Foundation::Size& newSize); From 58808b0d22acbf243e18011b2558088e39c1dbf2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 13 Nov 2019 12:20:54 -0600 Subject: [PATCH 11/14] This _works_ to use a winrt event, but it's dirty --- src/cascadia/TerminalApp/Pane.cpp | 72 ++++++++++++---------- src/cascadia/TerminalApp/Pane.h | 19 +++--- src/cascadia/TerminalApp/Tab.cpp | 74 ++++++++++++++++------- src/cascadia/TerminalApp/Tab.h | 5 +- src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- 5 files changed, 107 insertions(+), 65 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 93ead95291a..6ecdedc60c2 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -574,9 +574,9 @@ void Pane::_CloseChild(const bool closeFirst) // Take the got focus callback back into ourselves, and clear it from // our (abandoned) children. - _pfnGotFocus = remainingChild->_pfnGotFocus; - _firstChild->_pfnGotFocus = nullptr; - _secondChild->_pfnGotFocus = nullptr; + // _GotFocusHandlers = remainingChild->_GotFocusHandlers; + // _firstChild->_GotFocusHandlers = nullptr; + // _secondChild->_GotFocusHandlers = nullptr; // Revoke the old event handlers. Remove both the handlers for the panes // themselves closing, and remove their handlers for their controls @@ -608,13 +608,15 @@ void Pane::_CloseChild(const bool closeFirst) _root.Children().Append(_border); _border.Child(_control); + _splitState = SplitState::None; + _gotFocusRevoker = _control.GotFocus(winrt::auto_revoke, { this, &Pane::_ControlGotFocusHandler }); + if (_lastActive) { _control.Focus(FocusState::Programmatic); + _GotFocusHandlers(shared_from_this()); } - _splitState = SplitState::None; - _UpdateBorders(); // Release our children. @@ -649,8 +651,8 @@ void Pane::_CloseChild(const bool closeFirst) // Remove the got focus callback from the closed child. // Our new children should both still have their old callback values. // As we aren't a leaf, ours should definitely be null. - closedChild->_pfnGotFocus = nullptr; - _pfnGotFocus = nullptr; + // closedChild->_GotFocusHandlers = nullptr; + // _GotFocusHandlers = nullptr; // Set up new close handlers on the children _SetupChildCloseHandlers(); @@ -901,23 +903,23 @@ bool Pane::CanSplit(SplitState splitType) // - control: A TermControl to use in the new pane. // Return Value: // - -void Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) +std::pair, std::shared_ptr> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) { if (!_IsLeaf()) { if (_firstChild->_HasFocusedChild()) { - _firstChild->Split(splitType, profile, control); + return _firstChild->Split(splitType, profile, control); } else if (_secondChild->_HasFocusedChild()) { - _secondChild->Split(splitType, profile, control); + return _secondChild->Split(splitType, profile, control); } - return; + return { nullptr, nullptr }; } - _Split(splitType, profile, control); + return _Split(splitType, profile, control); } // Method Description: @@ -961,7 +963,7 @@ bool Pane::_CanSplit(SplitState splitType) // - control: A TermControl to use in the new pane. // Return Value: // - -void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) +std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) { // Lock the create/close lock so that another operation won't concurrently // modify our tree @@ -971,6 +973,9 @@ void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& _control.ConnectionClosed(_connectionClosedToken); _connectionClosedToken.value = 0; + // _control.GotFocus(_gotFocusRevoker); + _gotFocusRevoker.revoke(); + _splitState = splitType; _firstPercent = { Half }; @@ -1001,9 +1006,10 @@ void Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& _lastActive = false; - _firstChild->_pfnGotFocus = _pfnGotFocus; - _secondChild->_pfnGotFocus = _pfnGotFocus; - _pfnGotFocus = nullptr; + // _firstChild->_GotFocusHandlers = _GotFocusHandlers; + // _secondChild->_GotFocusHandlers = _GotFocusHandlers; + // _GotFocusHandlers = nullptr; + return { _firstChild, _secondChild }; } // Method Description: @@ -1076,24 +1082,25 @@ Size Pane::_GetMinSize() const void Pane::_ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable const& /* sender */, RoutedEventArgs const& /* args */) { - if (_pfnGotFocus) - { - _pfnGotFocus(shared_from_this()); - } + // if (_pfnGotFocus) + // { + // _pfnGotFocus(shared_from_this()); + // } + _GotFocusHandlers(shared_from_this()); } -// Method Description: -// - Register a callback function to be called when this Pane's control gains -// focused. This is used by the Tab hosting us so it can track which Pane in -// the tree was the last one to be focused. -// Arguments: -// - pfnGotFocus: A function that should be called when this pane's control gets focus. -// Return Value: -// - -void Pane::SetGotFocusCallback(std::function)> pfnGotFocus) -{ - _pfnGotFocus = pfnGotFocus; -} +// // Method Description: +// // - Register a callback function to be called when this Pane's control gains +// // focused. This is used by the Tab hosting us so it can track which Pane in +// // the tree was the last one to be focused. +// // Arguments: +// // - pfnGotFocus: A function that should be called when this pane's control gets focus. +// // Return Value: +// // - +// void Pane::SetGotFocusCallback(std::function)> pfnGotFocus) +// { +// _pfnGotFocus = std::move(pfnGotFocus); +// } // Function Description: // - Attempts to load some XAML resources that the Pane will need. This includes: @@ -1141,3 +1148,4 @@ void Pane::_SetupResources() } DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs); +DEFINE_EVENT(Pane, GotFocus, _GotFocusHandlers, winrt::delegate>); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index fae9ab85b54..ad85623c920 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -65,16 +65,19 @@ class Pane : public std::enable_shared_from_this bool NavigateFocus(const winrt::TerminalApp::Direction& direction); bool CanSplit(SplitState splitType); - void Split(SplitState splitType, - const GUID& profile, - const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + std::pair, std::shared_ptr> Split(SplitState splitType, + const GUID& profile, + const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void Close(); - void SetGotFocusCallback(std::function)> pfnGotFocus); + // void SetGotFocusCallback(std::function)> pfnGotFocus); DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); + DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate>); + // winrt::event>> m_accountIsInDebitEvent; + private: winrt::Windows::UI::Xaml::Controls::Grid _root{}; winrt::Windows::UI::Xaml::Controls::Border _border{}; @@ -100,16 +103,16 @@ class Pane : public std::enable_shared_from_this Borders _borders{ Borders::None }; - std::function)> _pfnGotFocus{ nullptr }; + // std::function)> _pfnGotFocus{ nullptr }; bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); bool _CanSplit(SplitState splitType); - void _Split(SplitState splitType, - const GUID& profile, - const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + std::pair, std::shared_ptr> _Split(SplitState splitType, + const GUID& profile, + const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void _CreateRowColDefinitions(const winrt::Windows::Foundation::Size& rootSize); void _CreateSplitContent(); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 5141a9eb8e3..b02dd800b45 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -25,23 +25,9 @@ Tab::Tab(const GUID& profile, const TermControl& control) _activePane = _rootPane; - // Add an event handler to this pane's GotFocus event. When that pane gains - // focus, we'll mark it as the new active pane. This pane will propogate - // this function down as it is split, so only leaves will trigger this. - _rootPane->SetGotFocusCallback([this](std::shared_ptr sender) { - _rootPane->ClearActive(); - _activePane = sender; - _activePane->SetActive(); - - if (_pfnActivePaneChanged) - { - _pfnActivePaneChanged(); - auto newTabTitle = this->GetFocusedTitle(); - this->SetTabText(newTabTitle); - } - }); + _AttachEventHandlersToPane(_rootPane); - _AttachEventHandersToControl(control); + _AttachEventHandlersToControl(control); _MakeTabViewItem(); } @@ -173,7 +159,7 @@ void Tab::UpdateIcon(const winrt::hstring iconPath) // - // Return Value: // - the title string of the last focused terminal control in our tree. -winrt::hstring Tab::GetFocusedTitle() const +winrt::hstring Tab::GetActiveTitle() const { const auto lastFocusedControl = GetActiveTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; @@ -233,9 +219,28 @@ bool Tab::CanSplitPane(Pane::SplitState splitType) // - void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl& control) { - _activePane->Split(splitType, profile, control); + auto [first, second] = _activePane->Split(splitType, profile, control); - _AttachEventHandersToControl(control); + _AttachEventHandlersToControl(control); + + _AttachEventHandlersToPane(first); + _AttachEventHandlersToPane(second); + + // Add an event handler to this pane's GotFocus event. When that pane gains + // focus, we'll mark it as the new active pane. This pane will propogate + // this function down as it is split, so only leaves will trigger this. + second->GotFocus([this](std::shared_ptr sender) { + _rootPane->ClearActive(); + _activePane = sender; + _activePane->SetActive(); + + auto newTabTitle = this->GetActiveTitle(); + this->SetTabText(newTabTitle); + if (_pfnActivePaneChanged) + { + _pfnActivePaneChanged(); + } + }); } // Method Description: @@ -297,13 +302,13 @@ void Tab::ClosePane() // - control: the TermControl to add events to. // Return Value: // - -void Tab::_AttachEventHandersToControl(const TermControl& control) +void Tab::_AttachEventHandlersToControl(const TermControl& control) { control.TitleChanged([this](auto newTitle) { // The title of the control changed, but not necessarily the title // of the tab. Get the title of the active pane of the tab, and set // the tab's text to the active panes' text. - auto newTabTitle = GetFocusedTitle(); + auto newTabTitle = GetActiveTitle(); SetTabText(newTabTitle); }); } @@ -319,7 +324,32 @@ void Tab::_AttachEventHandersToControl(const TermControl& control) // - void Tab::SetActivePaneChangedCallback(std::function pfnActivePaneChanged) { - _pfnActivePaneChanged = pfnActivePaneChanged; + _pfnActivePaneChanged = std::move(pfnActivePaneChanged); +} + +void Tab::_AttachEventHandlersToPane(std::shared_ptr pane) +{ + // Add an event handler to this pane's GotFocus event. When that pane gains + // focus, we'll mark it as the new active pane. This pane will propogate + // this function down as it is split, so only leaves will trigger this. + pane->GotFocus([this](std::shared_ptr sender) { + if (sender == _activePane) + { + return; + } + + _rootPane->ClearActive(); + _activePane = sender; + _activePane->SetActive(); + + auto newTabTitle = GetActiveTitle(); + std::wstring tabText{ newTabTitle }; + SetTabText(newTabTitle); + if (_pfnActivePaneChanged) + { + _pfnActivePaneChanged(); + } + }); } DEFINE_EVENT(Tab, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index e43fc1f36fa..1b989be5852 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -30,7 +30,7 @@ class Tab void NavigateFocus(const winrt::TerminalApp::Direction& direction); void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); - winrt::hstring GetFocusedTitle() const; + winrt::hstring GetActiveTitle() const; void SetTabText(const winrt::hstring& text); void ClosePane(); @@ -52,5 +52,6 @@ class Tab void _MakeTabViewItem(); void _Focus(); - void _AttachEventHandersToControl(const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void _AttachEventHandlersToControl(const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void _AttachEventHandlersToPane(std::shared_ptr pane); }; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 4c83ca6416b..519c67479ab 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -606,7 +606,7 @@ namespace winrt::TerminalApp::implementation // - tab: the Tab to update the title for. void TerminalPage::_UpdateTitle(std::shared_ptr tab) { - auto newTabTitle = tab->GetFocusedTitle(); + auto newTabTitle = tab->GetActiveTitle(); if (_settings->GlobalSettings().GetShowTitleInTitlebar() && tab->IsFocused()) From 805df516056a2dd136e7fe3a035806ea8de0b1ac Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 13 Nov 2019 14:33:17 -0600 Subject: [PATCH 12/14] Clean up everything from the winrt::event debacle. --- src/cascadia/TerminalApp/Pane.cpp | 49 ++++++++----------------------- src/cascadia/TerminalApp/Pane.h | 6 ---- src/cascadia/TerminalApp/Tab.cpp | 30 ++++++++++++++----- 3 files changed, 36 insertions(+), 49 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 6ecdedc60c2..a0d3307898b 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -572,12 +572,6 @@ void Pane::_CloseChild(const bool closeFirst) // Add our new event handler before revoking the old one. _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); - // Take the got focus callback back into ourselves, and clear it from - // our (abandoned) children. - // _GotFocusHandlers = remainingChild->_GotFocusHandlers; - // _firstChild->_GotFocusHandlers = nullptr; - // _secondChild->_GotFocusHandlers = nullptr; - // Revoke the old event handlers. Remove both the handlers for the panes // themselves closing, and remove their handlers for their controls // closing. At this point, if the remaining child's control is closed, @@ -608,13 +602,20 @@ void Pane::_CloseChild(const bool closeFirst) _root.Children().Append(_border); _border.Child(_control); + // Make sure to set our _splitState before focusing the control. If you + // fail to do this, when the tab handles the GotFocus event and asks us + // what our active control is, we won't technically be a "leaf", and + // GetTerminalControl will return null. _splitState = SplitState::None; + + // re-attach our handler for the control's GotFocus event. _gotFocusRevoker = _control.GotFocus(winrt::auto_revoke, { this, &Pane::_ControlGotFocusHandler }); + // If we're inheriting the "last active" state from one of our children, + // focus our control now. This should trigger our own GotFocus event. if (_lastActive) { _control.Focus(FocusState::Programmatic); - _GotFocusHandlers(shared_from_this()); } _UpdateBorders(); @@ -648,12 +649,6 @@ void Pane::_CloseChild(const bool closeFirst) _firstChild = remainingChild->_firstChild; _secondChild = remainingChild->_secondChild; - // Remove the got focus callback from the closed child. - // Our new children should both still have their old callback values. - // As we aren't a leaf, ours should definitely be null. - // closedChild->_GotFocusHandlers = nullptr; - // _GotFocusHandlers = nullptr; - // Set up new close handlers on the children _SetupChildCloseHandlers(); @@ -902,7 +897,7 @@ bool Pane::CanSplit(SplitState splitType) // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. // Return Value: -// - +// - The two newly created Panes std::pair, std::shared_ptr> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) { if (!_IsLeaf()) @@ -962,7 +957,7 @@ bool Pane::_CanSplit(SplitState splitType) // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. // Return Value: -// - +// - The two newly created Panes std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) { // Lock the create/close lock so that another operation won't concurrently @@ -973,7 +968,9 @@ std::pair, std::shared_ptr> Pane::_Split(SplitState _control.ConnectionClosed(_connectionClosedToken); _connectionClosedToken.value = 0; - // _control.GotFocus(_gotFocusRevoker); + // Remove our old GotFocus handler from the control. We don't what the + // control telling us that it's now focused, we want it telling its new + // parent. _gotFocusRevoker.revoke(); _splitState = splitType; @@ -1006,9 +1003,6 @@ std::pair, std::shared_ptr> Pane::_Split(SplitState _lastActive = false; - // _firstChild->_GotFocusHandlers = _GotFocusHandlers; - // _secondChild->_GotFocusHandlers = _GotFocusHandlers; - // _GotFocusHandlers = nullptr; return { _firstChild, _secondChild }; } @@ -1082,26 +1076,9 @@ Size Pane::_GetMinSize() const void Pane::_ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable const& /* sender */, RoutedEventArgs const& /* args */) { - // if (_pfnGotFocus) - // { - // _pfnGotFocus(shared_from_this()); - // } _GotFocusHandlers(shared_from_this()); } -// // Method Description: -// // - Register a callback function to be called when this Pane's control gains -// // focused. This is used by the Tab hosting us so it can track which Pane in -// // the tree was the last one to be focused. -// // Arguments: -// // - pfnGotFocus: A function that should be called when this pane's control gets focus. -// // Return Value: -// // - -// void Pane::SetGotFocusCallback(std::function)> pfnGotFocus) -// { -// _pfnGotFocus = std::move(pfnGotFocus); -// } - // Function Description: // - Attempts to load some XAML resources that the Pane will need. This includes: // * The Color we'll use for active Panes's borders - SystemAccentColor diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index ad85623c920..233216e27f1 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -71,12 +71,8 @@ class Pane : public std::enable_shared_from_this void Close(); - // void SetGotFocusCallback(std::function)> pfnGotFocus); - DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); - DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate>); - // winrt::event>> m_accountIsInDebitEvent; private: winrt::Windows::UI::Xaml::Controls::Grid _root{}; @@ -103,8 +99,6 @@ class Pane : public std::enable_shared_from_this Borders _borders{ Borders::None }; - // std::function)> _pfnGotFocus{ nullptr }; - bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index b02dd800b45..541db7df5fb 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -252,7 +252,9 @@ void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl // - void Tab::ResizeContent(const winrt::Windows::Foundation::Size& newSize) { - _activePane->ResizeContent(newSize); + // NOTE: This _must_ be called on the root pane, so that it can propogate + // throughout the entire tree. + _rootPane->ResizeContent(newSize); } // Method Description: @@ -264,7 +266,9 @@ void Tab::ResizeContent(const winrt::Windows::Foundation::Size& newSize) // - void Tab::ResizePane(const winrt::TerminalApp::Direction& direction) { - _activePane->ResizePane(direction); + // NOTE: This _must_ be called on the root pane, so that it can propogate + // throughout the entire tree. + _rootPane->ResizePane(direction); } // Method Description: @@ -276,7 +280,9 @@ void Tab::ResizePane(const winrt::TerminalApp::Direction& direction) // - void Tab::NavigateFocus(const winrt::TerminalApp::Direction& direction) { - _activePane->NavigateFocus(direction); + // NOTE: This _must_ be called on the root pane, so that it can propogate + // throughout the entire tree. + _rootPane->NavigateFocus(direction); } // Method Description: @@ -327,24 +333,34 @@ void Tab::SetActivePaneChangedCallback(std::function pfnActivePaneChange _pfnActivePaneChanged = std::move(pfnActivePaneChanged); } +// Method Description: +// - Add an event handler to this pane's GotFocus event. When that pane gains +// focus, we'll mark it as the new active pane. We'll also query the title of +// that pane when it's focused to set our own text, and finally, we'll trigger +// our own ActivePaneChanged event. +// Arguments: +// - +// Return Value: +// - void Tab::_AttachEventHandlersToPane(std::shared_ptr pane) { - // Add an event handler to this pane's GotFocus event. When that pane gains - // focus, we'll mark it as the new active pane. This pane will propogate - // this function down as it is split, so only leaves will trigger this. pane->GotFocus([this](std::shared_ptr sender) { + // Do nothing if it's the same pane as before. if (sender == _activePane) { return; } - + // Clear the active state of the entire tree, and mark only the sender as active. _rootPane->ClearActive(); _activePane = sender; _activePane->SetActive(); + // Update our own title text to match the newly-active pane. auto newTabTitle = GetActiveTitle(); std::wstring tabText{ newTabTitle }; SetTabText(newTabTitle); + + // Raise our own ActivePaneChanged event. if (_pfnActivePaneChanged) { _pfnActivePaneChanged(); From 14e944a94d23594e1938c75f076e2a557bd79f88 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 18 Nov 2019 11:21:59 -0600 Subject: [PATCH 13/14] This is dead code that shouldn't have been there --- src/cascadia/TerminalApp/Tab.cpp | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 541db7df5fb..70a6124a9fc 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -223,24 +223,10 @@ void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl _AttachEventHandlersToControl(control); + // Add a event handlers to the new panes' GotFocus event. When the pane + // gains focus, we'll mark it as the new active pane. _AttachEventHandlersToPane(first); _AttachEventHandlersToPane(second); - - // Add an event handler to this pane's GotFocus event. When that pane gains - // focus, we'll mark it as the new active pane. This pane will propogate - // this function down as it is split, so only leaves will trigger this. - second->GotFocus([this](std::shared_ptr sender) { - _rootPane->ClearActive(); - _activePane = sender; - _activePane->SetActive(); - - auto newTabTitle = this->GetActiveTitle(); - this->SetTabText(newTabTitle); - if (_pfnActivePaneChanged) - { - _pfnActivePaneChanged(); - } - }); } // Method Description: @@ -356,9 +342,7 @@ void Tab::_AttachEventHandlersToPane(std::shared_ptr pane) _activePane->SetActive(); // Update our own title text to match the newly-active pane. - auto newTabTitle = GetActiveTitle(); - std::wstring tabText{ newTabTitle }; - SetTabText(newTabTitle); + SetTabText(GetActiveTitle()); // Raise our own ActivePaneChanged event. if (_pfnActivePaneChanged) From 4a6891d2f024439538775769f85caa9208a9ee2f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 18 Nov 2019 12:13:44 -0600 Subject: [PATCH 14/14] Turn Tab's callback into a winrt::event as well --- src/cascadia/TerminalApp/Tab.cpp | 20 ++------------------ src/cascadia/TerminalApp/Tab.h | 5 +---- src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 70a6124a9fc..93ce7a6a50f 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -305,20 +305,6 @@ void Tab::_AttachEventHandlersToControl(const TermControl& control) }); } -// Method Description: -// - Register a callback function to be called when this Tab's active pane -// changes. The App will use this to lookup the appropriate icon for this Tab -// (based on the newly-active pane) and also to possibly propogate the new -// Pane's title to the window. -// Arguments: -// - pfnGotFocus: A function that should be called when this Tab's active pane changes -// Return Value: -// - -void Tab::SetActivePaneChangedCallback(std::function pfnActivePaneChanged) -{ - _pfnActivePaneChanged = std::move(pfnActivePaneChanged); -} - // Method Description: // - Add an event handler to this pane's GotFocus event. When that pane gains // focus, we'll mark it as the new active pane. We'll also query the title of @@ -345,11 +331,9 @@ void Tab::_AttachEventHandlersToPane(std::shared_ptr pane) SetTabText(GetActiveTitle()); // Raise our own ActivePaneChanged event. - if (_pfnActivePaneChanged) - { - _pfnActivePaneChanged(); - } + _ActivePaneChangedHandlers(); }); } DEFINE_EVENT(Tab, Closed, _closedHandlers, ConnectionClosedEventArgs); +DEFINE_EVENT(Tab, ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 1b989be5852..fffb13926b8 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -35,9 +35,8 @@ class Tab void ClosePane(); - void SetActivePaneChangedCallback(std::function pfnActivePaneChanged); - DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); + DECLARE_EVENT(ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>); private: std::shared_ptr _rootPane{ nullptr }; @@ -47,8 +46,6 @@ class Tab bool _focused{ false }; winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem{ nullptr }; - std::function _pfnActivePaneChanged{ nullptr }; - void _MakeTabViewItem(); void _Focus(); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 56a77021c62..5032ca7c241 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -424,7 +424,7 @@ namespace winrt::TerminalApp::implementation std::weak_ptr weakTabPtr = newTab; // When the tab's active pane changes, we'll want to lookup a new icon // for it, and possibly propogate the title up to the window. - newTab->SetActivePaneChangedCallback([this, weakTabPtr]() { + newTab->ActivePaneChanged([this, weakTabPtr]() { if (auto tab = weakTabPtr.lock()) { // Possibly update the icon of the tab.