Don't always focus pane content on Tapped, if the pane is already foc…

…used (#17174) You'll never believe this. Clicking on the dropdown button on a ComboBox doesn't set `e.Tapped = true`. It bubbles up, and lands in our `Pane`'s `Border`'s tapped handler. And in there, we yeet focus to the first content. We end up stealing focus from the combobox, and then the combobox doesn't actually open its dropdown. So yea we can just fix that. Easy enough. Closes #17062 --------- Co-authored-by: Dustin L. Howett <[email protected]> Co-authored-by: Leonard Hecker <[email protected]>
microsoft · May 2, 2024 · c2b8f99 · c2b8f99
1 parent 015055c
commit c2b8f99
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 18 deletions.
diff --git a/scratch/ScratchIslandApp/SampleApp/MySettings.h b/scratch/ScratchIslandApp/SampleApp/MySettings.h
@@ -11,7 +11,7 @@ Licensed under the MIT license.
 #include <conattrs.hpp>
 #include "MySettings.g.h"
 
-using IFontFeatureMap = winrt::Windows::Foundation::Collections::IMap<winrt::hstring, uint32_t>;
+using IFontFeatureMap = winrt::Windows::Foundation::Collections::IMap<winrt::hstring, float>;
 using IFontAxesMap = winrt::Windows::Foundation::Collections::IMap<winrt::hstring, float>;
 
 namespace winrt::SampleApp::implementation

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
@@ -47,14 +47,8 @@ Pane::Pane(const IPaneContent& content, const bool lastFocused) :
  // 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.
- _borderFirst.Tapped([this](auto&, auto& e) {
- _FocusFirstChild();
- e.Handled(true);
- });
- _borderSecond.Tapped([this](auto&, auto& e) {
- _FocusFirstChild();
- e.Handled(true);
- });
+ _borderFirst.Tapped({ this, &Pane::_borderTappedHandler });
+ _borderSecond.Tapped({ this, &Pane::_borderTappedHandler });
 }
 
 Pane::Pane(std::shared_ptr<Pane> first,
@@ -88,14 +82,8 @@ Pane::Pane(std::shared_ptr<Pane> first,
  // 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.
- _borderFirst.Tapped([this](auto&, auto& e) {
- _FocusFirstChild();
- e.Handled(true);
- });
- _borderSecond.Tapped([this](auto&, auto& e) {
- _FocusFirstChild();
- e.Handled(true);
- });
+ _borderFirst.Tapped({ this, &Pane::_borderTappedHandler });
+ _borderSecond.Tapped({ this, &Pane::_borderTappedHandler });
 }
 
 // Extract the terminal settings from the current (leaf) pane's control
@@ -1237,6 +1225,14 @@ void Pane::UpdateVisuals()
 // - <none>
 void Pane::_Focus()
 {
+ // Don't focus our content if we're already focused. This prevents a bug
+ // where tapping on the arrow in a ComboBox will land in our Tapped handler,
+ // and if we steal focus from the ComboBox, it won't open. See GH#17062
+ if (WasLastFocused())
+ {
+ return;
+ }
+
  GotFocus.raise(shared_from_this(), FocusState::Programmatic);
  if (const auto& lastContent{ GetLastFocusedContent() })
  {
@@ -3021,3 +3017,9 @@ winrt::Windows::UI::Xaml::Media::SolidColorBrush Pane::_ComputeBorderColor()
 
  return _themeResources.unfocusedBorderBrush;
 }
+
+void Pane::_borderTappedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::Windows::UI::Xaml::Input::TappedRoutedEventArgs& e)
+{
+ _FocusFirstChild();
+ e.Handled(true);
+}
diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
@@ -314,6 +314,8 @@ class Pane : public std::enable_shared_from_this<Pane>
 
  SplitState _convertAutomaticOrDirectionalSplitState(const winrt::Microsoft::Terminal::Settings::Model::SplitDirection& splitType) const;
 
+ void _borderTappedHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Input::TappedRoutedEventArgs& e);
+
  // Function Description:
  // - Returns true if the given direction can be used with the given split
  // type.

diff --git a/src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml b/src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml
@@ -31,7 +31,8 @@
  <local:SettingContainer x:Uid="Globals_Language"
  Visibility="{x:Bind ViewModel.LanguageSelectorAvailable}">
  <ComboBox ItemsSource="{x:Bind ViewModel.LanguageList}"
- SelectedItem="{x:Bind ViewModel.CurrentLanguage, Mode=TwoWay}">
+ SelectedItem="{x:Bind ViewModel.CurrentLanguage, Mode=TwoWay}"
+ Style="{StaticResource ComboBoxSettingStyle}">
  <ComboBox.ItemTemplate>
  <DataTemplate x:DataType="x:String">
  <TextBlock Text="{x:Bind local:GlobalAppearanceViewModel.LanguageDisplayConverter((x:String))}" />