[0.83] fix(scrollview): honor programmatic scrollTo when scrollEnabled={false} - #16304
Conversation
ScrollViewComponentView::scrollTo() early-returned whenever scrollEnabled was false, so scrollTo (and scrollToIndex / scrollToOffset, which route through the "scrollTo" command) were dropped on a non-scrollable ScrollView / FlatList. On iOS and Android scrollEnabled only blocks user pan gestures; programmatic scroll offsets still apply. Remove the early-return; user-gesture scrolling remains gated by m_scrollVisual.ScrollEnabled (set from scrollEnabled in updateProps). Note: StartBringIntoView() and scrollToEnd()/scrollToStart() keep an analogous scrollEnabled gate; those also serve keyboard/focus paths and are left for separate discussion so keyboard scroll semantics aren't changed here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
@microsoft-github-policy-service agree [company="Facilitron"] |
|
@microsoft-github-policy-service agree company="Facilitron" |
|
Validated end-to-end on a patched framework source build. We compiled this fix into Microsoft.ReactNative from source (yarn-patch, RNW 0.83.2, ARM64 Debug; patched dll verified as the loaded module via GetDeviceCaps/GDI32 import markers vs the stock NuGet dll as negative control) and re-ran the same probe in our production app: |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Andrew Coates (@acoates-ms) — a batch CI request, since this PR is in the best shape of the set and you're already familiar with it from #16303. Ten Windows PRs from me are open and none has ever had Azure CI run — each shows only
If it's easier to batch, Context on where these come from: all of them are running in production in a shipping RNW 0.83.2 new-arch app (Facilitron FIT), compiled from source with Happy to split, rebase, or narrow any of these if that makes review easier — and if the |
There was a problem hiding this comment.
Pull request overview
This PR aligns React Native Windows Fabric ScrollView behavior with iOS/Android by ensuring imperative scrolling APIs (scrollTo and the commands that route through it, like scrollToIndex/scrollToOffset) still work when scrollEnabled={false}—while keeping user-gesture scrolling disabled via the existing ScrollEnabled gating.
Changes:
- Removed the
scrollEnabledearly-return inScrollViewComponentView::scrollTo, so programmatic scroll requests are no longer dropped. - Added an explanatory comment documenting the intended cross-platform semantics and why this is safe in RNW.
- Added a change file to record the patch update for
react-native-windows.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vnext/Microsoft.ReactNative/Fabric/Composition/ScrollViewComponentView.cpp | Always honors programmatic scrollTo even when scrollEnabled is false; user gestures remain disabled via m_scrollVisual.ScrollEnabled. |
| change/react-native-windows-fix-scrollview-programmatic.json | Patch change entry describing the behavior fix. |
…e} (#16336) scrollEnabled={false} must only disable user scroll gestures, matching iOS and Android where setContentOffset / scrollToOffset still work when scrolling is disabled. The scrollTo command (and scrollToIndex / scrollToOffset, which route through it) previously hit a scrollEnabled early-return and was silently dropped. User-gesture input is gated separately via m_scrollVisual.ScrollEnabled (set from scrollEnabled in updateProps), so honoring a programmatic scroll here does not re-enable user scrolling. main-branch twin of #16304 (0.83-stable).
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
* fix(scrollview): honor programmatic scrollTo when scrollEnabled={false} (#16336)
scrollEnabled={false} must only disable user scroll gestures, matching iOS and
Android where setContentOffset / scrollToOffset still work when scrolling is
disabled. The scrollTo command (and scrollToIndex / scrollToOffset, which route
through it) previously hit a scrollEnabled early-return and was silently
dropped. User-gesture input is gated separately via m_scrollVisual.ScrollEnabled
(set from scrollEnabled in updateProps), so honoring a programmatic scroll here
does not re-enable user scrolling.
main-branch twin of #16304 (0.83-stable).
* fix(textinput): correct placeholder layout constraints (px vs DIP) and no-op NaN fontSize guard (#16317)
* fix(textinput): correct placeholder layout constraints (px vs DIP) and no-op NaN fontSize guard
Forward-port of #16303 (0.83-stable) to main.
CreatePlaceholderLayout fed m_imgWidth/m_imgHeight - which are physical
pixels (frame * pointScaleFactor) - into LayoutConstraints, which are
expressed in DIPs. The placeholder was laid out in a box pointScaleFactor
times too large, so it measured and positioned at a different height than
the typed text. Divide by pointScaleFactor.
The NaN fontSize guard was also a no-op: it evaluated
defaultTextAttributes().fontSize as a discarded expression statement
instead of assigning it, so a placeholder with no fontSize never picked
up the default.
* add beachball change file
* Update release type to prerelease
Change type from 'patch' to 'prerelease' for react-native-windows.
---------
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
* fix(pointer): label a touch/pen contact as the primary button (#16338)
onPointerPressed maps ActiveTouch.button exclusively from PointerUpdateKind, a
mouse-only concept. A touch or pen contact matches no case, falls through to
default: button = -1, and the derived W3C buttons bitmask becomes 0. The
pointerdown delivered to JS therefore claims no button is pressed, so
pointer-event-driven press handling discards finger contacts while identical
mouse clicks work.
Per W3C pointer-events a touch/pen contact IS the primary button: button 0,
buttons 1. Set that after the switch when the mouse mapping left it negative.
main counterpart of the button-labeling change in #16333 (0.83-stable), tracking
#16332. Ports only that change: main already covers the tag == -1 release leak
and the stale-pointer-reuse leak via #16048 (dispatching a synthesized touch
Cancel), and cancels capture loss per pointer. #16333's cancel-all loop,
IsPrimary purge and stale-touch backstop are deliberately not ported - they do
not exist on main and their necessity there has not been assessed.
* fix(pointer): null-check the capturing component view before notifying OnPointerCaptureLost (#16337)
CapturePointer and releasePointerCapture look the capturing component up by its
cached m_pointerCapturingComponentTag and dereference the result unguarded. That
tag can outlive the component it names: when list/ScrollView virtualization
recycles the capturing row mid-pan, componentViewDescriptorWithTag returns a
descriptor whose .view is null, so winrt::get_self(...)->OnPointerCaptureLost()
dereferences null and terminates the process with 0xc0000005.
Null-check targetComponentView at both sites. Skipping the notify loses no state
transition: CapturePointer overwrites the stale tag immediately below, and
releasePointerCapture clears it via the existing m_capturedPointers.empty()
branch.
main twin of #16334 (0.83-stable).
* Fix modifying outline property
* delete createRnwApp tests in 0.81 branch
* Change files
---------
Co-authored-by: Collin Schneide <27441618+FaithfulAudio@users.noreply.github.com>


Problem
On a
ScrollView/FlatListwithscrollEnabled={false}, imperativescrollTo/scrollToOffset/scrollToIndexdo nothing on RNW. On iOS/Android,scrollEnabledblocks only user gestures — programmatic scrolls still apply.Root cause
ScrollViewComponentView::scrollTo()early-returns whenscrollEnabledis false.scrollToIndex/scrollToOffsetroute through thescrollTocommand, so all imperative scrolls are dropped.Fix
Remove the early-return. User-gesture scrolling remains gated by
m_scrollVisual.ScrollEnabled(...)(set fromscrollEnabledinupdateProps), so this only re-enables programmatic scrolls — matching iOS/Android semantics.Validation
Bug reproduced in a production RNW 0.83.2 new-arch app (Facilitron FIT, Windows 11 ARM64, Debug, 250% scale): a
scrollEnabled={false}ScrollView with 800-DIP content in a 110-DIP viewport ignoresscrollTo({y:500, animated:false})— the call fires, no scroll event is emitted, content stays at Row 0. The probe renders its own status line:Expected after-state per the diff:
TryUpdatePositionapplies the offset andonScrollreports y=500. Needs upstream CI + the same probe on a framework source build.Intentionally not touched:
StartBringIntoView()andscrollToEnd()/scrollToStart()carry an analogous gate but also serve keyboard/focus paths — changing those would alter keyboard-scroll semantics, so they're flagged for maintainer discussion instead. Authored against0.83-stable; happy to re-cut ontomain.Microsoft Reviewers: Open in CodeFlow