Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Open
Nitin-100 wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Fix XAML popup positioning and light dismiss in ScrollView (#15557) #15564

Nitin-100 wants to merge 13 commits into from
+590 −153

Conversation

@Nitin-100
Copy link
Contributor

@Nitin-100 Nitin-100 commented Jan 21, 2026 •
edited
Loading

Fix XAML popup positioning and light dismiss in ScrollView (#15557)

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:

  1. Bug 1 (Position): After scrolling, opening the popup shows it at the wrong position (offset by the scroll amount)
  2. Bug 2 (Light Dismiss): Opening a popup and then scrolling leaves the popup floating at its original position instead of dismissing

Root Cause Analysis

Bug 1: Popup Position

  • ContentIslandComponentView uses ChildSiteLink.LocalToParentTransformMatrix for popup positioning
  • Issue 1: getClientRect() returns values in physical pixels (scaled by pointScaleFactor), but LocalToParentTransformMatrix expects logical pixels (DIPs)
  • Issue 2: Transform was updated asynchronously via UIDispatcher().Post(), causing race conditions where popup opened with stale transform values
  • Issue 3: Initial transform wasn't set before Connect(), causing wrong position even without scrolling

Bug 2: Light Dismiss on Scroll

  • XAML's native light dismiss behavior wasn't being triggered when scroll begins in React Native

Solution

Bug 1 Fix: Popup Position After Scroll

  1. Convert to DIPs: Divide getClientRect() values by pointScaleFactor before setting LocalToParentTransformMatrix
  2. Synchronous updates: Remove async UIDispatcher().Post() wrapper - update transform immediately
  3. Initial transform: Set LocalToParentTransformMatrix in OnMounted() before Connect()
  4. Scroll notification: ScrollViewComponentView fires LayoutMetricsChanged event when scroll position changes

Bug 2 Fix: Light Dismiss on Scroll (Optimized)

  • Added DismissPopupsRequest event to ContentIslandComponentView
  • Added ScrollBeginDrag event to ScrollViewComponentView
  • Optimized registration: During OnMounted(), ContentIslandComponentView walks up the tree once and registers for ScrollBeginDrag on all parent ScrollViewComponentView instances
  • When scroll begins, ScrollViewComponentView fires its ScrollBeginDrag event, which notifies only the registered ContentIslandComponentView instances
  • No tree walking on every scroll - registration happens once during mount
  • 3rd party XAML components subscribe to DismissPopupsRequest and dismiss their own popups
  • Core remains XAML-agnostic - no XAML-specific code in the framework

Files Changed

Core Fix (vnext/Microsoft.ReactNative/)

  • Fabric/Composition/ContentIslandComponentView.cpp - DIP conversion, sync updates, register for parent ScrollView events during mount
  • Fabric/Composition/ContentIslandComponentView.h - Store scroll subscriptions
  • Fabric/Composition/ScrollViewComponentView.cpp - Fire LayoutMetricsChanged on scroll, expose ScrollBeginDrag event
  • Fabric/Composition/ScrollViewComponentView.h - ScrollBeginDrag event
  • CompositionComponentView.idl - Added DismissPopupsRequest event to ContentIslandComponentView, ScrollBeginDrag event to ScrollViewComponentView

Testing

  1. Run the XamlPopupRepro sample in Playground
  2. Scroll down in the ScrollView
  3. Click a ComboBox to open dropdown - should appear at correct position (right at the button)
  4. Open a ComboBox dropdown, then scroll - dropdown should close automatically

##Screenshot

testingFix.mp4

Fix XAML popup positioning and light dismiss in ScrollView (microsoft…
4de944f 
…#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
@Nitin-100 Nitin-100 requested review from a team as code owners January 21, 2026 07:49
sundaramramaswamy
sundaramramaswamy requested changes Jan 23, 2026
View reviewed changes
Copy link
Contributor

@sundaramramaswamy sundaramramaswamy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address comments.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Jan 23, 2026
Fix XAML popup positioning and light dismiss in ScrollView (microsoft…
1df8e5d 
…#15557)

- Set LocalToParentTransformMatrix synchronously before Connect() to fix initial position
- Fire LayoutMetricsChanged on scroll to update position after scrolling
- Add DismissPopupsRequest event for 3P components to implement light dismiss
- Update ComboBox sample to use event-based approach (core remains XAML-agnostic)
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Jan 25, 2026
Nitin Chaudhary and others added 3 commits January 25, 2026 19:08
Fix XAML popup positioning - convert to DIPs and update synchronously
7917f1a 
Issue microsoft#15557: Fix popup positioning for XAML controls in ScrollView

Bug 1 (Position Fix):
- Convert getClientRect() physical pixels to DIPs by dividing by pointScaleFactor
- LocalToParentTransformMatrix expects logical pixels (DIPs), not physical pixels
- Update transform synchronously instead of async to avoid race conditions
- Set initial transform in OnMounted() before Connect()

Bug 2 (Light Dismiss Fix):
- Add DismissPopupsRequest event for 3P components to handle popup dismissal
- ScrollView fires event when scroll begins via DismissChildContentIslandPopups()
- Core remains XAML-agnostic - 3P components handle their own popups
@Nitin-100
Merge branch 'main' into nitinc/fix-xaml-popup-positioning-15557
6b10e7c
Apply clang-format
6d19597
@Nitin-100 Nitin-100 self-assigned this Jan 27, 2026
sundaramramaswamy
sundaramramaswamy requested changes Jan 27, 2026
View reviewed changes
Copy link
Contributor

@sundaramramaswamy sundaramramaswamy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert formatting changes.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Jan 27, 2026
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Jan 27, 2026
@Nitin-100 Nitin-100 force-pushed the nitinc/fix-xaml-popup-positioning-15557 branch from f0d7eb5 to e8cfe99 Compare January 27, 2026 14:27
acoates-ms
acoates-ms reviewed Jan 27, 2026
View reviewed changes
@acoates-ms
Copy link
Contributor

acoates-ms commented Jan 27, 2026 •
edited
Loading

Assuming LocalToParentTransformMatrix is pretty cheap, the LocalToParentTransformMatrix changes look good.

The light dismiss logic seems more wrong. This looks like we need additional hosting APIs from ContentIslands. ContentIslands should provide APIs to handle this kind of communication, and Xaml / RNW should talk to the new API. Xaml content islands should not have to know about RNW APIs - it breaks the ContentIsland encapsulation.

@Nitin-100 Nitin-100 requested a review from a team January 28, 2026 17:40
@Nitin-100
Copy link
Contributor Author

Nitin-100 commented Jan 30, 2026

@acoates-ms I have addressed all comments, please review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@acoates-ms acoates-ms acoates-ms left review comments

@protikbiswas100 protikbiswas100 protikbiswas100 left review comments

@sundaramramaswamy sundaramramaswamy sundaramramaswamy requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@Nitin-100 Nitin-100

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Pop-ups of Xaml controls need positioning and dismissal

4 participants

@Nitin-100 @acoates-ms @sundaramramaswamy @protikbiswas100