-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix XAML popup positioning and light dismiss in ScrollView (#15557) #15564
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
base: main
Are you sure you want to change the base?
Fix XAML popup positioning and light dismiss in ScrollView (#15557) #15564
Conversation
…#15557) - Add SetXamlRoot() API on ContentIslandComponentView for 3rd party XAML components - Add DismissPopups() using VisualTreeHelper.GetOpenPopupsForXamlRoot() - Fire LayoutMetricsChanged on scroll to update popup positions - Dismiss child ContentIsland popups when scroll begins - Add ComboBox sample component demonstrating the pattern - Add xamlPopupBug test sample for playground-composition
sundaramramaswamy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address comments.
| // Issue #15557: Register the XamlRoot for this ContentIsland to enable popup dismissal. | ||
| // When a parent ScrollView starts scrolling, DismissPopups() will be called which uses | ||
| // VisualTreeHelper.GetOpenPopupsForXamlRoot() to find and close all open XAML popups. | ||
| // 3rd party XAML components (ComboBox, DatePicker, etc.) should call this after creating | ||
| // their XamlIsland: islandView.SetXamlRoot(m_xamlIsland.Content().XamlRoot()); | ||
| void SetXamlRoot(Microsoft.UI.Xaml.XamlRoot xamlRoot); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ContentIsland is generic. Xaml 3P modules just ride on it but any other island (inherited from Microsoft.Ui.ContentIsland) can be hosted. The core "knowing" about Xaml is a problem. Instead of the core dismissing the popup (and there by knowing about Xaml) it should just tell the module to dismiss pop-up if any.
Try to see if you could emit an existing event to which the 3P component can listen and dismiss pop-ups.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please drop the re-formatting and only keep your change. Two reasons
- What changed isn't evident
- It screws up
git blameoutput
Description
This PR fixes Issue #15557 - "Pop-ups of Xaml controls need positioning and dismissal"
When XAML controls with popups (like ComboBox, DatePicker, TimePicker) are hosted inside a React Native ScrollView via
ContentIslandComponentView, two bugs occur:Root Cause
ContentIslandComponentViewusesChildSiteLink.LocalToParentTransformMatrixfor popup positioning, but wasn't being notified when scroll position changedSolution
Bug 1 Fix: Popup Position After Scroll
ScrollViewComponentViewnow firesLayoutMetricsChangedevent when scroll position changesContentIslandComponentViewlistens to this and callsParentLayoutChanged()LocalToParentTransformMatrixwith the correct scroll offset viagetClientRect()Bug 2 Fix: Light Dismiss on Scroll (Generic for ALL 3rd Party XAML Controls)
SetXamlRoot(XamlRoot)method toContentIslandComponentViewAPIXamlRootafter the element is loadedDismissPopups()usesVisualTreeHelper.GetOpenPopupsForXamlRoot()to find and close ALL open XAML popupsFiles Changed
Core Fix (vnext/Microsoft.ReactNative/)
CompositionComponentView.idl- AddedSetXamlRoot()method toContentIslandComponentViewFabric/Composition/ContentIslandComponentView.h/.cpp- ImplementedSetXamlRoot()andDismissPopups()Fabric/Composition/ScrollViewComponentView.h/.cpp- Added scroll position change notification and popup dismissal triggerSample Component (packages/sample-custom-component/)
ComboBoxcomponent as example of proper 3rd party implementationSetXamlRoot()for popup dismissalTest Samples
XamlPopupRepro.windows.tsx- Test case in RNTesterxamlPopupBug.tsx- Test case in Playground🔧 Guidance for 3rd Party XAML Component Developers
If you're building a custom XAML component that has popups (ComboBox, DatePicker, Flyouts, etc.), follow this pattern to ensure proper popup behavior inside React Native ScrollViews:
C++ Implementation Pattern
Why This Works
Position Fix: The framework automatically updates
LocalToParentTransformMatrixwhen scroll position changes, so your popup appears at the correct location.Light Dismiss: When you register your
XamlRoot, the framework usesVisualTreeHelper.GetOpenPopupsForXamlRoot()to find ALL open popups when scroll begins and closes them automatically.What else handled with this fix
Testing
XamlPopupReprosample in PlaygroundMicrosoft Reviewers: Open in CodeFlow