Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manually dismiss popups when the window moves, or the SUI scrolls #10922

Merged
16 commits merged into from
Aug 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
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()
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
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