starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-07-08 18:16:28 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[PR #10922] [MERGED] Manually dismiss popups when the window moves, or the SUI scrolls #28308

New Issue
Open
opened 2026-01-31 09:27:41 +00:00 by claunia · 0 comments
claunia commented 2026-01-31 09:27:41 +00:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/10922
Author: @zadjii-msft
Created: 8/11/2021
Status: ✅ Merged
Merged: 8/16/2021
Merged by: @undefined

Base: main ← Head: dev/migrie/b/9320-interfacial-separation-2

📝 Commits (10+)

  • 41e4801 well I'll be danged, that worked
  • 98ebc9d move to a helper
  • efdbd83 now THIS is BODGY
  • b06e85e Move BODGYBODGEY to utils
  • b22d74e Ah poop. This might not compile
  • 8ab4f27 I'm shook that this works as well as it does
  • 2d11fa4 What the heck? If you do this, then moving the window will... close all tabs?
  • 29956c7 still no luck
  • ba5bd73 derp
  • 0077482 this is so bodgy

📊 Changes

24 files changed (+231 additions, -155 deletions)

View changed files

📝 src/cascadia/TerminalSettingsEditor/Actions.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Actions.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/AddProfile.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/AddProfile.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/ColorSchemes.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/GlobalAppearance.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Interaction.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Interaction.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Launch.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Launch.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Profiles.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Profiles.xaml (+3 -3)
📝 src/cascadia/TerminalSettingsEditor/ReadOnlyActions.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/ReadOnlyActions.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Rendering.h (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Rendering.xaml (+1 -1)
📝 src/cascadia/TerminalSettingsEditor/Utils.h (+38 -0)
📝 src/cascadia/WindowsTerminal/AppHost.cpp (+27 -0)

...and 4 more files

📄 Description

Summary of the Pull Request

BODGY!

This solution was suggested in https://github.com/microsoft/microsoft-ui-xaml/issues/4554#issuecomment-887815332.

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

PR Checklist

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

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

-    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

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/10922 **Author:** [@zadjii-msft](https://github.com/zadjii-msft) **Created:** 8/11/2021 **Status:** ✅ Merged **Merged:** 8/16/2021 **Merged by:** [@undefined](undefined) **Base:** `main` ← **Head:** `dev/migrie/b/9320-interfacial-separation-2` --- ### 📝 Commits (10+) - [`41e4801`](https://github.com/microsoft/terminal/commit/41e48012a207d615accbfc316b49d2e45e1b33d3) well I'll be danged, that worked - [`98ebc9d`](https://github.com/microsoft/terminal/commit/98ebc9d9dc5ac816ba986e93a45236f586cab61f) move to a helper - [`efdbd83`](https://github.com/microsoft/terminal/commit/efdbd83842ff1a254be7a97982f6f59800f24c51) now THIS is BODGY - [`b06e85e`](https://github.com/microsoft/terminal/commit/b06e85e5f6d7e5a9098fcd61fb1e87ae0d7e0bba) Move BODGYBODGEY to utils - [`b22d74e`](https://github.com/microsoft/terminal/commit/b22d74e4a0dcbc524ae022a04b151049b0e89255) Ah poop. This might not compile - [`8ab4f27`](https://github.com/microsoft/terminal/commit/8ab4f277aaecb504b040b8f073806011862f59ed) I'm shook that this works as well as it does - [`2d11fa4`](https://github.com/microsoft/terminal/commit/2d11fa42ad52b41d7b3b464b712c9b1d1411d518) What the heck? If you do this, then moving the window will... close all tabs? - [`29956c7`](https://github.com/microsoft/terminal/commit/29956c7cbfe977d78a8f38bf8fe2f74407cbcdb0) still no luck - [`ba5bd73`](https://github.com/microsoft/terminal/commit/ba5bd7345060117d98dc752c4dee5a5645299ad3) derp - [`0077482`](https://github.com/microsoft/terminal/commit/0077482bea180489de7133342036329a22d2d4bf) this is so bodgy ### 📊 Changes **24 files changed** (+231 additions, -155 deletions) <details> <summary>View changed files</summary> 📝 `src/cascadia/TerminalSettingsEditor/Actions.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Actions.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/AddProfile.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/AddProfile.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/ColorSchemes.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/ColorSchemes.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/GlobalAppearance.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Interaction.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Interaction.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Launch.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Launch.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Profiles.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Profiles.xaml` (+3 -3) 📝 `src/cascadia/TerminalSettingsEditor/ReadOnlyActions.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/ReadOnlyActions.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Rendering.h` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Rendering.xaml` (+1 -1) 📝 `src/cascadia/TerminalSettingsEditor/Utils.h` (+38 -0) 📝 `src/cascadia/WindowsTerminal/AppHost.cpp` (+27 -0) _...and 4 more files_ </details> ### 📄 Description ## Summary of the Pull Request BODGY! This solution was suggested in https://github.com/microsoft/microsoft-ui-xaml/issues/4554#issuecomment-887815332. 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 https://github.com/microsoft/microsoft-ui-xaml/issues/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 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:27:41 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#28308
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?