From f17b6c7d0da3c068c794ae119f451d0b848f28e5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 28 May 2020 16:06:17 -0500 Subject: [PATCH] Enable tab renaming at runtime from the UI (#5775) ## Summary of the Pull Request Adds support for setting, from the UI, a runtime override for the tab title text. The user can use this to effectively "rename" a tab. If set, the tab will _always_ use the runtime override string. If the user has multiple panes with different titles in a pane, then the tab's override text will be used _regardless_ of which pane was focused when the tab was renamed. The override text can be removed by just deleting the entire contents of the box. Then, the tab will revert to using the terminal's usual title. ## References * Wouldn't be possible without the context menu from #3789 * Focus doesn't return to the active terminal after hitting enter/esc, but that's tracked by #5750 ## PR Checklist * [x] Closes #1079 * [x] I work here * [ ] Tests added/passed * [ ] Requires documentation to be updated ## TODO * [x] `Tab::SetTabText` might be able to be greatly simplified/removed? * [x] I'm _pretty sure_ if they set an override title, we won't bubble that up to set the window title. * [x] I'm unsure how this behaves when the terminal's title changes _while_ the TextBox is visible. I don't think it should change the current contents of the box, but it might currently. * [ ] **for discussion**: If the user doesn't actually change the text of the tab, then we probably shouldn't set the override text, right? - EX: if they open the box and the text is "cmd", and immediately hit enter, then run `title foo`, should the text change to "foo" or stay "cmd"? ## Detailed Description of the Pull Request / Additional comments ![image](https://user-images.githubusercontent.com/18356694/81230615-713f9180-8fb7-11ea-8945-6681eec02a4f.png) ![image](https://user-images.githubusercontent.com/18356694/81230640-7ac8f980-8fb7-11ea-9e6b-22f0e0ed128a.png) ![image](https://user-images.githubusercontent.com/18356694/81230665-86b4bb80-8fb7-11ea-90f0-16d4ffb60d89.png) ![image](https://user-images.githubusercontent.com/18356694/81230686-9207e700-8fb7-11ea-94a9-f3f5a59be139.png) ![image](https://user-images.githubusercontent.com/18356694/81230732-a350f380-8fb7-11ea-9901-6dd4f36154f1.png) ![image](https://user-images.githubusercontent.com/18356694/81230746-a8ae3e00-8fb7-11ea-94fa-d2578f9241a7.png) ![image](https://user-images.githubusercontent.com/18356694/81230787-bc59a480-8fb7-11ea-8edf-2bd7fad343fc.png) ![image](https://user-images.githubusercontent.com/18356694/81230851-dc896380-8fb7-11ea-98c1-918b943543e4.png) --- .../Resources/en-US/Resources.resw | 3 + src/cascadia/TerminalApp/Tab.cpp | 185 ++++++++++++++++-- src/cascadia/TerminalApp/Tab.h | 9 +- src/cascadia/TerminalApp/TerminalPage.cpp | 22 ++- 4 files changed, 193 insertions(+), 26 deletions(-) diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 2d4519aec39..bdc3cf861e8 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -189,6 +189,9 @@ Reset + + Rename Tab + Found a profile with an invalid "backgroundImage". Defaulting that profile to have no background image. Make sure that when setting a "backgroundImage", the value is a valid file path to an image. {Locked="\"backgroundImage\""} diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index e30c2232639..22b92d1e271 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -14,6 +14,7 @@ using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; +using namespace winrt::Windows::System; namespace winrt { @@ -221,28 +222,33 @@ namespace winrt::TerminalApp::implementation // - the title string of the last focused terminal control in our tree. winrt::hstring Tab::GetActiveTitle() const { + if (!_runtimeTabText.empty()) + { + return _runtimeTabText; + } const auto lastFocusedControl = GetActiveTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; } // Method Description: - // - Set the text on the TabViewItem for this tab. + // - Set the text on the TabViewItem for this tab, and bubbles the new title + // value up to anyone listening for changes to our title. Callers can + // listen for the title change with a PropertyChanged even handler. // Arguments: - // - text: The new text string to use as the Header for our TabViewItem + // - // Return Value: // - - winrt::fire_and_forget Tab::SetTabText(const winrt::hstring text) + winrt::fire_and_forget Tab::_UpdateTitle() { - // Copy the hstring, so we don't capture a dead reference - winrt::hstring textCopy{ text }; auto weakThis{ get_weak() }; - co_await winrt::resume_foreground(_tabViewItem.Dispatcher()); - if (auto tab{ weakThis.get() }) { - Title(text); - _tabViewItem.Header(winrt::box_value(text)); + // Bubble our current tab text to anyone who's listening for changes. + Title(GetActiveTitle()); + + // Update the UI to reflect the changed + _UpdateTabHeader(); } } @@ -391,7 +397,7 @@ namespace winrt::TerminalApp::implementation { // The title of the control changed, but not necessarily the title of the tab. // Set the tab's text to the active panes' text. - tab->SetTabText(tab->GetActiveTitle()); + tab->_UpdateTitle(); } }); @@ -426,7 +432,7 @@ namespace winrt::TerminalApp::implementation _activePane->SetActive(); // Update our own title text to match the newly-active pane. - SetTabText(GetActiveTitle()); + _UpdateTitle(); // Raise our own ActivePaneChanged event. _ActivePaneChangedHandlers(); @@ -513,15 +519,164 @@ namespace winrt::TerminalApp::implementation } }); + Controls::MenuFlyoutItem renameTabMenuItem; + { + // "Rename Tab" + Controls::FontIcon renameTabSymbol; + renameTabSymbol.FontFamily(Media::FontFamily{ L"Segoe MDL2 Assets" }); + renameTabSymbol.Glyph(L"\xE932"); // Label + + renameTabMenuItem.Click([weakThis](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_inRename = true; + tab->_UpdateTabHeader(); + } + }); + renameTabMenuItem.Text(RS_(L"RenameTabText")); + renameTabMenuItem.Icon(renameTabSymbol); + } + // Build the menu Controls::MenuFlyout newTabFlyout; Controls::MenuFlyoutSeparator menuSeparator; newTabFlyout.Items().Append(chooseColorMenuItem); + newTabFlyout.Items().Append(renameTabMenuItem); newTabFlyout.Items().Append(menuSeparator); newTabFlyout.Items().Append(closeTabMenuItem); _tabViewItem.ContextFlyout(newTabFlyout); } + // Method Description: + // - This will update the contents of our TabViewItem for our current state. + // - If we're not in a rename, we'll set the Header of the TabViewItem to + // simply our current tab text (either the runtime tab text or the + // active terminal's text). + // - If we're in a rename, then we'll set the Header to a TextBox with the + // current tab text. The user can then use that TextBox to set a string + // to use as an override for the tab's text. + // Arguments: + // - + // Return Value: + // - + void Tab::_UpdateTabHeader() + { + winrt::hstring tabText{ GetActiveTitle() }; + + if (!_inRename) + { + // If we're not currently in the process of renaming the tab, then just set the tab's text to whatever our active title is. + _tabViewItem.Header(winrt::box_value(tabText)); + } + else + { + _ConstructTabRenameBox(tabText); + } + } + + // Method Description: + // - Create a new TextBox to use as the control for renaming the tab text. + // If the text box is already created, then this will do nothing, and + // leave the current box unmodified. + // Arguments: + // - tabText: This should be the text to initialize the rename text box with. + // Return Value: + // - + void Tab::_ConstructTabRenameBox(const winrt::hstring& tabText) + { + if (_tabViewItem.Header().try_as()) + { + return; + } + + Controls::TextBox tabTextBox; + tabTextBox.Text(tabText); + + // The TextBox has a MinHeight already set by default, which is + // larger than we want. Get rid of it. + tabTextBox.MinHeight(0); + // Also get rid of the internal padding on the text box, between the + // border and the text content, on the top and bottom. This will + // help the box fit within the bounds of the tab. + Thickness internalPadding = ThicknessHelper::FromLengths(4, 0, 4, 0); + tabTextBox.Padding(internalPadding); + + // Make the margin (0, -8, 0, -8), to counteract the padding that + // the TabViewItem has. + // + // This is maybe a bit fragile, as the actual value might not be exactly + // (0, 8, 0, 8), but using TabViewItemHeaderPadding to look up the real + // value at runtime didn't work. So this is good enough for now. + Thickness negativeMargins = ThicknessHelper::FromLengths(0, -8, 0, -8); + tabTextBox.Margin(negativeMargins); + + // Set up some event handlers on the text box. We need three of them: + // * A LostFocus event, so when the TextBox loses focus, we'll + // remove it and return to just the text on the tab. + // * A KeyUp event, to be able to submit the tab text on Enter or + // dismiss the text box on Escape + // * A LayoutUpdated event, so that we can auto-focus the text box + // when it's added to the tree. + auto weakThis{ get_weak() }; + + // When the text box loses focus, update the tab title of our tab. + // - If there are any contents in the box, we'll use that value as + // the new "runtime text", which will override any text set by the + // application. + // - If the text box is empty, we'll reset the "runtime text", and + // return to using the active terminal's title. + tabTextBox.LostFocus([weakThis](const IInspectable& sender, auto&&) { + auto tab{ weakThis.get() }; + auto textBox{ sender.try_as() }; + if (tab && textBox) + { + tab->_runtimeTabText = textBox.Text(); + tab->_inRename = false; + tab->_UpdateTitle(); + } + }); + + // NOTE: (Preview)KeyDown does not work here. If you use that, we'll + // remove the TextBox from the UI tree, then the following KeyUp + // will bubble to the NewTabButton, which we don't want to have + // happen. + tabTextBox.KeyUp([weakThis](const IInspectable& sender, Input::KeyRoutedEventArgs const& e) { + auto tab{ weakThis.get() }; + auto textBox{ sender.try_as() }; + if (tab && textBox) + { + switch (e.OriginalKey()) + { + case VirtualKey::Enter: + tab->_runtimeTabText = textBox.Text(); + [[fallthrough]]; + case VirtualKey::Escape: + e.Handled(true); + textBox.Text(tab->_runtimeTabText); + tab->_inRename = false; + tab->_UpdateTitle(); + break; + } + } + }); + + // As soon as the text box is added to the UI tree, focus it. We can't focus it till it's in the tree. + _tabRenameBoxLayoutUpdatedRevoker = tabTextBox.LayoutUpdated(winrt::auto_revoke, [this](auto&&, auto&&) { + // Curiously, the sender for this event is null, so we have to + // get the TextBox from the Tab's Header(). + auto textBox{ _tabViewItem.Header().try_as() }; + if (textBox) + { + textBox.SelectAll(); + textBox.Focus(FocusState::Programmatic); + } + // Only let this succeed once. + _tabRenameBoxLayoutUpdatedRevoker.revoke(); + }); + + _tabViewItem.Header(tabTextBox); + } + // Method Description: // Returns the tab color, if any // Arguments: @@ -649,13 +804,13 @@ namespace winrt::TerminalApp::implementation { if (_focused) { - winrt::Windows::UI::Xaml::VisualStateManager::GoToState(_tabViewItem, L"Normal", true); - winrt::Windows::UI::Xaml::VisualStateManager::GoToState(_tabViewItem, L"Selected", true); + VisualStateManager::GoToState(_tabViewItem, L"Normal", true); + VisualStateManager::GoToState(_tabViewItem, L"Selected", true); } else { - winrt::Windows::UI::Xaml::VisualStateManager::GoToState(_tabViewItem, L"Selected", true); - winrt::Windows::UI::Xaml::VisualStateManager::GoToState(_tabViewItem, L"Normal", true); + VisualStateManager::GoToState(_tabViewItem, L"Selected", true); + VisualStateManager::GoToState(_tabViewItem, L"Normal", true); } } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index b1e0435984b..830fa65803a 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -47,7 +47,6 @@ namespace winrt::TerminalApp::implementation void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); winrt::hstring GetActiveTitle() const; - winrt::fire_and_forget SetTabText(const winrt::hstring text); void Shutdown(); void ClosePane(); @@ -73,6 +72,10 @@ namespace winrt::TerminalApp::implementation bool _focused{ false }; winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem{ nullptr }; + winrt::hstring _runtimeTabText{}; + bool _inRename{ false }; + winrt::Windows::UI::Xaml::Controls::TextBox::LayoutUpdated_revoker _tabRenameBoxLayoutUpdatedRevoker; + void _MakeTabViewItem(); void _Focus(); @@ -89,6 +92,10 @@ namespace winrt::TerminalApp::implementation int _GetLeafPaneCount() const noexcept; void _UpdateActivePane(std::shared_ptr pane); + void _UpdateTabHeader(); + winrt::fire_and_forget _UpdateTitle(); + void _ConstructTabRenameBox(const winrt::hstring& tabText); + friend class ::TerminalAppLocalTests::TabTests; }; } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 2d074ba9a0f..750d97a09b8 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -944,23 +944,25 @@ namespace winrt::TerminalApp::implementation // Bind Tab events to the TermControl and the Tab's Pane hostingTab.Initialize(term); - // 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. - term.TitleChanged([weakTab{ hostingTab.get_weak() }, weakThis{ get_weak() }](auto newTitle) { + auto weakTab{ hostingTab.get_weak() }; + auto weakThis{ get_weak() }; + // PropertyChanged is the generic mechanism by which the Tab + // communicates changes to any of its observable properties, including + // the Title + hostingTab.PropertyChanged([weakTab, weakThis](auto&&, const WUX::Data::PropertyChangedEventArgs& args) { auto page{ weakThis.get() }; auto tab{ weakTab.get() }; - if (page && tab) { - // 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. - page->_UpdateTitle(*tab); + if (args.PropertyName() == L"Title") + { + page->_UpdateTitle(*tab); + } } }); // react on color changed events - hostingTab.ColorSelected([weakTab{ hostingTab.get_weak() }, weakThis{ get_weak() }](auto&& color) { + hostingTab.ColorSelected([weakTab, weakThis](auto&& color) { auto page{ weakThis.get() }; auto tab{ weakTab.get() }; @@ -970,7 +972,7 @@ namespace winrt::TerminalApp::implementation } }); - hostingTab.ColorCleared([weakTab{ hostingTab.get_weak() }, weakThis{ get_weak() }]() { + hostingTab.ColorCleared([weakTab, weakThis]() { auto page{ weakThis.get() }; auto tab{ weakTab.get() };