diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index dd01c2a2e22..a82575656a9 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -844,18 +844,54 @@ namespace winrt::TerminalApp::implementation } _UpdateTeachingTipTheme(WindowRenamer().try_as()); - WindowRenamer().IsOpen(true); - // PAIN: We can't immediately focus the textbox in the TeachingTip. It's - // not technically focusable until it is opened. However, it doesn't - // provide an event to tell us when it is opened. That's tracked in - // microsoft/microsoft-ui-xaml#1607. So for now, the user _needs_ to - // click on the text box manually. + // BODGY: GH#12021 + // + // TeachingTip doesn't provide an Opened event. + // (microsoft/microsoft-ui-xaml#1607). But we want to focus the renamer + // text box when it's opened. We can't do that immediately, the TextBox + // technically isn't in the visual tree yet. We have to wait for it to + // get added some time after we call IsOpen. How do we do that reliably? + // Usually, for this kind of thing, we'd just use a one-off + // LayoutUpdated event, as a notification that the TextBox was added to + // the tree. HOWEVER: + // * The _first_ time this is fired, when the box is _first_ opened, + // tossing focus doesn't work on the first LayoutUpdated. It does + // work on the second LayoutUpdated. Okay, so we'll wait for two + // LayoutUpdated events, and focus on the second. + // * On subsequent opens: We only ever get a single LayoutUpdated. + // Period. But, you can successfully focus it on that LayoutUpdated. + // + // So, we'll keep track of how many LayoutUpdated's we've _ever_ gotten. + // If we've had at least 2, then we can focus the text box. // // We're also not using a ContentDialog for this, because in Xaml // Islands a text box in a ContentDialog won't receive _any_ keypresses. // Fun! // WindowRenamerTextBox().Focus(FocusState::Programmatic); + _renamerLayoutUpdatedRevoker.revoke(); + _renamerLayoutUpdatedRevoker = WindowRenamerTextBox().LayoutUpdated(winrt::auto_revoke, [weakThis = get_weak()](auto&&, auto&&) { + if (auto self{ weakThis.get() }) + { + auto& count{ self->_renamerLayoutCount }; + + // Don't just always increment this, we don't want to deal with overflow situations + if (count < 2) + { + count++; + } + + if (count >= 2) + { + self->_renamerLayoutUpdatedRevoker.revoke(); + self->WindowRenamerTextBox().Focus(FocusState::Programmatic); + } + } + }); + // Make sure to mark that enter was not pressed in the renamer quite + // yet. More details in TerminalPage::_WindowRenamerKeyDown. + _renamerPressedEnter = false; + WindowRenamer().IsOpen(true); args.Handled(true); } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index a137bf1637b..851aa47a746 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -3621,6 +3621,26 @@ namespace winrt::TerminalApp::implementation // of thing with co_return winrt::make(false). } + // Method Description: + // - Used to track if the user pressed enter with the renamer open. If we + // immediately focus it after hitting Enter on the command palette, then + // the Enter keydown will dismiss the command palette and open the + // renamer, and then the enter keyup will go to the renamer. So we need to + // make sure both a down and up go to the renamer. + // Arguments: + // - e: the KeyRoutedEventArgs describing the key that was released + // Return Value: + // - + void TerminalPage::_WindowRenamerKeyDown(const IInspectable& /*sender*/, + winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e) + { + const auto key = e.OriginalKey(); + if (key == Windows::System::VirtualKey::Enter) + { + _renamerPressedEnter = true; + } + } + // Method Description: // - Manually handle Enter and Escape for committing and dismissing a window // rename. This is highly similar to the TabHeaderControl's KeyUp handler. @@ -3631,16 +3651,18 @@ namespace winrt::TerminalApp::implementation void TerminalPage::_WindowRenamerKeyUp(const IInspectable& sender, winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e) { - if (e.OriginalKey() == Windows::System::VirtualKey::Enter) + const auto key = e.OriginalKey(); + if (key == Windows::System::VirtualKey::Enter && _renamerPressedEnter) { // User is done making changes, close the rename box _WindowRenamerActionClick(sender, nullptr); } - else if (e.OriginalKey() == Windows::System::VirtualKey::Escape) + else if (key == Windows::System::VirtualKey::Escape) { // User wants to discard the changes they made WindowRenamerTextBox().Text(WindowName()); WindowRenamer().IsOpen(false); + _renamerPressedEnter = false; } } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 2e48d9db883..1a72fe40c12 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -215,6 +215,10 @@ namespace winrt::TerminalApp::implementation std::shared_ptr _windowIdToast{ nullptr }; std::shared_ptr _windowRenameFailedToast{ nullptr }; + winrt::Windows::UI::Xaml::Controls::TextBox::LayoutUpdated_revoker _renamerLayoutUpdatedRevoker; + int _renamerLayoutCount{ 0 }; + bool _renamerPressedEnter{ false }; + winrt::Windows::Foundation::IAsyncOperation _ShowDialogHelper(const std::wstring_view& name); void _ShowAboutDialog(); @@ -414,6 +418,7 @@ namespace winrt::TerminalApp::implementation void _WindowRenamerActionClick(const IInspectable& sender, const IInspectable& eventArgs); void _RequestWindowRename(const winrt::hstring& newName); + void _WindowRenamerKeyDown(const IInspectable& sender, winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e); void _WindowRenamerKeyUp(const IInspectable& sender, winrt::Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e); void _UpdateTeachingTipTheme(winrt::Windows::UI::Xaml::FrameworkElement element); diff --git a/src/cascadia/TerminalApp/TerminalPage.xaml b/src/cascadia/TerminalApp/TerminalPage.xaml index 5ca8ed25858..2f6e8f66960 100644 --- a/src/cascadia/TerminalApp/TerminalPage.xaml +++ b/src/cascadia/TerminalApp/TerminalPage.xaml @@ -193,6 +193,7 @@ IsLightDismissEnabled="True">