Skip to content

Commit

Permalink
Manually dismiss popups when the window moves, or the SUI scrolls (#1…
Browse files Browse the repository at this point in the history
…0922)

## Summary of the Pull Request

BODGY!

This solution was suggested in microsoft/microsoft-ui-xaml#4554 (comment).

When the window moves, or when a ScrollViewer scrolls, dismiss any popups that are visible. This happens automagically when an app is a real XAML app, but it doesn't work for XAML Islands.

## References
* upstream at microsoft/microsoft-ui-xaml#4554

## PR Checklist
* [x] Closes #9320
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

Unfortunately, we've got a bunch of scroll viewers in our SUI. So I did something bodgyx2 to make our life a little easier.

`DismissAllPopups` can be used to dismiss all popups for a particular UI element. However, we've got a bunch of pages with scroll viewers that may or may not have popups in them. Rather than define the same exact body for all their `ViewChanging` events, the `HasScrollViewer` struct will just do it for you!

Inside the `HasScrollViewer` stuct, we can't get at the `XamlRoot()` that our subclass implements. I mean, _we_ can, but when XAML does it's codegen, _XAML_ won't be able to figure it out.

Fortunately for us, we don't need to! The sender is a UIElement, so we can just get _their_ `XamlRoot()`.

So, you can fix this for any SUI page with just a simple 

```diff
-    <ScrollViewer>
+    <ScrollViewer ViewChanging="ViewChanging">
```

```diff
-    struct AddProfile : AddProfileT<AddProfile>
+    struct AddProfile : public HasScrollViewer<AddProfile>, AddProfileT<AddProfile>
```

## Validation Steps Performed

* the window doesn't close when you move it
* the popups _do_ close when you move the window
* the popups close when you scroll any SUI page
  • Loading branch information
zadjii-msft authored Aug 16, 2021
1 parent 5d36e5d commit 29be856
Show file tree
Hide file tree
Showing 24 changed files with 231 additions and 155 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Model::CascadiaSettings, Settings, nullptr)
};

struct Actions : ActionsT<Actions>
struct Actions : public HasScrollViewer<Actions>, ActionsT<Actions>
{
public:
Actions();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Actions.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel MaxWidth="600"
Spacing="8"
Style="{StaticResource SettingsStackStyle}">
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/AddProfile.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_CALLBACK(AddNew, AddNewArgs);
};

struct AddProfile : AddProfileT<AddProfile>
struct AddProfile : public HasScrollViewer<AddProfile>, AddProfileT<AddProfile>
{
public:
AddProfile();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/AddProfile.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Style="{StaticResource SettingsStackStyle}">
<Button x:Uid="AddProfile_AddNewButton"
AutomationProperties.AutomationId="AddProfile_AddNewButton"
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/ColorSchemes.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(winrt::hstring, LastSelectedScheme, L"");
};

struct ColorSchemes : ColorSchemesT<ColorSchemes>
struct ColorSchemes : public HasScrollViewer<ColorSchemes>, ColorSchemesT<ColorSchemes>
{
ColorSchemes();

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Margin="{StaticResource StandardIndentMargin}"
Spacing="24">

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/GlobalAppearance.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Model::GlobalAppSettings, Globals, nullptr)
};

struct GlobalAppearance : GlobalAppearanceT<GlobalAppearance>
struct GlobalAppearance : public HasScrollViewer<GlobalAppearance>, GlobalAppearanceT<GlobalAppearance>
{
public:
GlobalAppearance();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Style="{StaticResource SettingsStackStyle}">
<!-- Language -->
<local:SettingContainer x:Uid="Globals_Language"
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Interaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Model::GlobalAppSettings, Globals, nullptr)
};

struct Interaction : InteractionT<Interaction>
struct Interaction : public HasScrollViewer<Interaction>, InteractionT<Interaction>
{
Interaction();

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Interaction.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Style="{StaticResource SettingsStackStyle}">
<!-- Copy On Select -->
<local:SettingContainer x:Uid="Globals_CopyOnSelect"
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Launch.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Model::CascadiaSettings, Settings, nullptr)
};

struct Launch : LaunchT<Launch>
struct Launch : public HasScrollViewer<Launch>, LaunchT<Launch>
{
public:
Launch();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Launch.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel>
<StackPanel Style="{StaticResource SettingsStackStyle}">
<!-- Default Profile -->
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Profiles.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
Windows::Foundation::Collections::IMapView<hstring, Model::ColorScheme> _Schemes;
};

struct Profiles : ProfilesT<Profiles>
struct Profiles : public HasScrollViewer<Profiles>, ProfilesT<Profiles>
{
public:
Profiles();
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalSettingsEditor/Profiles.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
SelectionChanged="Pivot_SelectionChanged">
<!-- General Tab -->
<PivotItem x:Uid="Profile_General">
<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Style="{StaticResource PivotStackStyle}">

<!-- Name -->
Expand Down Expand Up @@ -225,7 +225,7 @@

<!-- Appearance Tab -->
<PivotItem x:Uid="Profile_Appearance">
<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel>
<!-- Control Preview -->
<Border x:Name="ControlPreview"
Expand Down Expand Up @@ -397,7 +397,7 @@

<!-- Advanced Tab -->
<PivotItem x:Uid="Profile_Advanced">
<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Style="{StaticResource PivotStackStyle}">
<!-- Suppress Application Title -->
<local:SettingContainer x:Uid="Profile_SuppressApplicationTitle"
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/ReadOnlyActions.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
TYPED_EVENT(OpenJson, Windows::Foundation::IInspectable, Model::SettingsTarget);
};

struct ReadOnlyActions : ReadOnlyActionsT<ReadOnlyActions>
struct ReadOnlyActions : public HasScrollViewer<ReadOnlyActions>, ReadOnlyActionsT<ReadOnlyActions>
{
public:
ReadOnlyActions();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/ReadOnlyActions.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Style="{StaticResource SettingsStackStyle}">
<TextBlock x:Uid="Globals_KeybindingsDisclaimer"
Style="{StaticResource DisclaimerStyle}" />
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Rendering.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
WINRT_PROPERTY(Model::GlobalAppSettings, Globals, nullptr)
};

struct Rendering : RenderingT<Rendering>
struct Rendering : public HasScrollViewer<Rendering>, RenderingT<Rendering>
{
Rendering();

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Rendering.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
</ResourceDictionary>
</Page.Resources>

<ScrollViewer>
<ScrollViewer ViewChanging="ViewChanging">
<StackPanel Style="{StaticResource SettingsStackStyle}">
<TextBlock x:Uid="Globals_RenderingDisclaimer"
Style="{StaticResource DisclaimerStyle}" />
Expand Down
38 changes: 38 additions & 0 deletions src/cascadia/TerminalSettingsEditor/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,41 @@ namespace winrt::Microsoft::Terminal::Settings
winrt::hstring GetSelectedItemTag(winrt::Windows::Foundation::IInspectable const& comboBoxAsInspectable);
winrt::hstring LocalizedNameForEnumName(const std::wstring_view sectionAndType, const std::wstring_view enumValue, const std::wstring_view propertyType);
}

// BODGY!
//
// The following function and struct are a workaround for GH#9320.
//
// DismissAllPopups can be used to dismiss all popups for a particular UI
// element. However, we've got a bunch of pages with scroll viewers that may or
// may not have popups in them. Rather than define the same exact body for all
// their ViewChanging events, the HasScrollViewer struct will just do it for
// you!
inline void DismissAllPopups(winrt::Windows::UI::Xaml::XamlRoot const& xamlRoot)
{
const auto popups{ winrt::Windows::UI::Xaml::Media::VisualTreeHelper::GetOpenPopupsForXamlRoot(xamlRoot) };
for (const auto& p : popups)
{
p.IsOpen(false);
}
}

template<typename T>
struct HasScrollViewer
{
// When the ScrollViewer scrolls, dismiss any popups we might have.
void ViewChanging(winrt::Windows::Foundation::IInspectable const& sender,
const winrt::Windows::UI::Xaml::Controls::ScrollViewerViewChangingEventArgs& /*e*/)
{
// Inside this struct, we can't get at the XamlRoot() that our subclass
// implements. I mean, _we_ can, but when XAML does it's code
// generation, _XAML_ won't be able to figure it out.
//
// Fortunately for us, we don't need to! The sender is a UIElement, so
// we can just get _their_ XamlRoot().
if (const auto& uielem{ sender.try_as<winrt::Windows::UI::Xaml::UIElement>() })
{
DismissAllPopups(uielem.XamlRoot());
}
}
};
27 changes: 27 additions & 0 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ AppHost::AppHost() noexcept :
std::placeholders::_2));
_window->MouseScrolled({ this, &AppHost::_WindowMouseWheeled });
_window->WindowActivated({ this, &AppHost::_WindowActivated });
_window->WindowMoved({ this, &AppHost::_WindowMoved });
_window->HotkeyPressed({ this, &AppHost::_GlobalHotkeyPressed });
_window->SetAlwaysOnTop(_logic.GetInitialAlwaysOnTop());
_window->MakeWindow();
Expand Down Expand Up @@ -1095,3 +1096,29 @@ winrt::fire_and_forget AppHost::_HideTrayIconRequested()
}
}
}

// Method Description:
// - BODGY workaround for GH#9320. When the window moves, dismiss all the popups
// in the UI tree. Xaml Islands unfortunately doesn't do this for us, see
// microsoft/microsoft-ui-xaml#4554
// Arguments:
// - <none>
// Return Value:
// - <none>
void AppHost::_WindowMoved()
{
if (_logic)
{
const auto root{ _logic.GetRoot() };

// This is basically DismissAllPopups which is also in
// TerminalSettingsEditor/Utils.h
// There isn't a good place that's shared between these two files, but
// it's only 5 LOC so whatever.
const auto popups{ Media::VisualTreeHelper::GetOpenPopupsForXamlRoot(root.XamlRoot()) };
for (const auto& p : popups)
{
p.IsOpen(false);
}
}
}
1 change: 1 addition & 0 deletions src/cascadia/WindowsTerminal/AppHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class AppHost
const winrt::Windows::Foundation::IInspectable& arg);
void _WindowMouseWheeled(const til::point coord, const int32_t delta);
winrt::fire_and_forget _WindowActivated();
void _WindowMoved();

void _DispatchCommandline(winrt::Windows::Foundation::IInspectable sender,
winrt::Microsoft::Terminal::Remoting::CommandlineArgs args);
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ long IslandWindow::_calculateTotalSize(const bool isWidth, const long clientSize
{
return _OnMoving(wparam, lparam);
}
case WM_MOVE:
{
_WindowMovedHandlers();
break;
}
case WM_CLOSE:
{
// If the user wants to close the app by clicking 'X' button,
Expand Down
Loading

0 comments on commit 29be856

Please sign in to comment.