Repository navigation
Conversation
meta-cla
Bot
added
the
CLA Signed
|
@jasozh has exported this pull request. If you are a Meta employee, you can view the originating Diff in D123471013. |
cortinico
approved these changes
Oct 7, 2026
cortinico
left a comment
cortinico
left a comment
Contributor
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
Summary: Pull Request resolved: react#58902 Fixes the issue described in [react#48552](react#48552), where a Pressable component can lose both its background color and ripple effect when borderless is true. ### Root Cause Android sets `isProjected() = true` for a borderless RippleDrawable so the ripple can extend beyond the view's bounds. React Native adds the background and ripple as layers in a CompositeBackgroundDrawable, where `isProjected() = true` if any layer is projected. With projection, both the background and ripple are drawn on the nearest native ancestor with a background. Given "root view -> parent view -> Pressable", the ripple and background should now be drawn on the parent. However, since the parent doesn't form a stacking context, Fabric flattens its children. The Pressable and parent become siblings, and the Pressable's background and ripple are now drawn on a further ancestor and consequently become hidden. Workarounds like opacity, `overflow: hidden`, and `collapsable={false}` work by establishing a stacking context so the parent remains the Pressable's ancestor. ### This PR Modify CompositeBackgroundDrawable to wrap `feedbackUnderlay` in a DrawableWrapper where `isProjected() = false`. The background and ripple are drawn in-place, so they remain visible without a stacking context. The ripple still extends beyond the view's bounds since `clipChildren = false` in React Native. This is gated behind the `fixBorderlessRippleAndroid` flag which defaults to false. ### Limitations With projection, the background and borderless ripple are drawn underneath all siblings. Without projection, they are now drawn over previous siblings and underneath following siblings. This matches behavior already shown by React Native foreground ripples, and Jetpack Compose also disables projection in `RippleHostView.UnprojectedRipple` and exhibits the same behavior. Disabling ripple projection also causes some behavioral changes with clipping. The ripple is no longer clipped to the outline of the ancestor it was projected on, and it can now be clipped by `overflow: hidden` or ScrollView ancestors it used to draw beyond. These changes also align with foreground ripples and Jetpack Compose. One alternative which more closely preserves projected behavior is to modify Fabric's core renderer logic to maintain a parent stacking context if a borderless ripple is present. However, this can potentially change stacking behavior in non-obvious ways depending on where a ripple is defined, where the nearest ancestor with a background exists, and siblings in between. This impacts zIndex behavior and makes Android's stacking diverge from iOS, where ripples don't exist. Changelog: [Android][Fixed] - Fix Pressable and TouchableNativeFeedback losing their background color and ripple when using a borderless ripple Reviewed By: cortinico Differential Revision: D123471013
meta-codesync
Bot
changed the title
jasozh
force-pushed
the
export-D123471013
branch
from
October 7, 2026 22:36
2ae30df to
479266c
Compare
|
This pull request has been merged in ca3e76a. |
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 26, 2026
…on Android (#59425) Summary: On Android, `outline*`, `boxShadow` and the `borderRadius` they're drawn with are kept in the `CompositeBackgroundDrawable` that `BackgroundStyleApplicator` installs as a view's background. **Current behavior.** `ReactSwitch.setBackgroundColor` replaces the background with a new `RippleDrawable`, which throws away that composite and everything in it. Fabric applies a view's props in the order of its prop map, which depends on the set of prop keys, so whether a `Switch` keeps its outline depends on which props it has: - With `{width: 60, height: 40, backgroundColor: 'white', outlineWidth: 3, outlineColor: 'blue', outlineOffset: 8}`, the props are applied as `outlineColor`, `outlineOffset`, `backgroundColor`, `outlineWidth`. `backgroundColor` drops the color and offset, and `outlineWidth` then creates a new outline with the defaults, so the `Switch` mounts with a black outline and no offset. Updating `outlineOffset` later restores the offset, but the color stays black because `outlineColor` isn't sent again. - Any update to `backgroundColor` removes the outline and box shadow entirely, whatever order they were applied in. - `thumbColor` also sets the ripple's color. That color is lost when `backgroundColor` is applied after it, since the new ripple uses the background color, and it isn't applied at all when the ripple is inside a composite, since `setThumbColor` only checks whether the background itself is a `RippleDrawable`. **With this change.** - When the background is already a `CompositeBackgroundDrawable`, `setBackgroundColor` replaces only its original background layer with the ripple, and keeps the other layers. `CompositeBackgroundDrawable` gets a `withNewOriginalBackground`, like its other `withNew*` methods. - `ReactSwitch` keeps the last thumb color and builds the ripple with it (or with the background color once `thumbColor` is removed), and `setThumbColor` finds the ripple inside the composite. `ReactSwitch` is the only view in ReactAndroid that replaces its background in `setBackgroundColor`; the others go through `BackgroundStyleApplicator`. Moving the `Switch`'s background color to `BackgroundStyleApplicator.setBackgroundColor` and its ripple to the feedback underlay would remove the special case, but it would also move the ripple above the border and change how the background is drawn, so this keeps the existing ripple and only stops it from replacing the other layers. ## Changelog: [ANDROID] [FIXED] - Fix `Switch` losing its outline, `boxShadow` and `thumbColor` ripple color when `backgroundColor` is set Pull Request resolved: #59425 Test Plan: - **Unit tests.** New `ReactSwitchManagerTest` (Robolectric): outline props set before the background color are kept; updating the background color keeps the outline; the background color keeps a box shadow; `thumbColor` sets the color of a ripple inside the composite; a thumb color set before the background color is kept; a removed thumb color isn't reused for the next background color. `./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests "com.facebook.react.views.switchview.ReactSwitchManagerTest"`: 6 passed. On the current code the first 5 fail (black instead of blue, no outline, the composite replaced by a `RippleDrawable`, the ripple left white instead of green twice); the last passes there, since the current code doesn't store the thumb color, and guards the new stored color. - `./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest`: 832 tests, 0 failures. - `yarn format-check-kotlin` passes. - **Device:** OPPO K12x 5G (CPH2667, Android 16), RNTester debug build, New Architecture, with the `Switch` above next to a `View` with the same style. Logging the prop setters and `setBackgroundColor` (not part of this PR) showed, for the `Switch`: ``` setOutlineColor value=-16776961 bg=RippleDrawable setOutlineOffset value=8.0 bg=CompositeBackgroundDrawable ReactSwitch.setBackgroundColor color=ffffffff replacing CompositeBackgroundDrawable setOutlineWidth value=3.0 bg=RippleDrawable ``` - Before: the `Switch` mounts with a black outline and no offset; after setting `backgroundColor` to yellow, its outline is gone, and a `Switch` with `boxShadow: '0 0 0 6px red'` loses its shadow. - After: the `Switch` matches the `View` on mount, after updating `outlineOffset`, and after setting `backgroundColor`. - With `thumbColor: 'red'` and the outline, long-pressing the `Switch` shows no ripple before (it's white, like the background) and a red ripple after. - **Screenshots.** Each image shows, left to right: on mount, after updating `outlineOffset`, and after setting `backgroundColor` to yellow. In each row the `Switch` is on the left and a `View` with the same style on the right; the middle row (no `backgroundColor`) is the case not fixed here. Before: <img width="720" alt="Before" src="https://github.com/user-attachments/assets/bf96545a-d6a7-4b3d-8dc4-5788a48371a1" /> After: <img width="720" alt="After" src="https://github.com/user-attachments/assets/090a1036-9b3e-43d2-ac1e-8b5eba97fa3d" /> Long-pressing a `Switch` with `thumbColor: 'red'`, `backgroundColor: 'white'` and a blue outline, on `main` (left; the outline is also black there, from the prop order above) and with this change (right): <img width="720" alt="Ripple before and after" src="https://github.com/user-attachments/assets/cc8cef77-a213-43ba-ab3b-1740aeb85f6b" /> - **Not run:** other devices and Android versions, and the old architecture, where outline and box shadow aren't supported. Clearing the stored ripple color when `thumbColor` is removed is covered by the unit test only, not checked on a device. Not fixed here: a `Switch` with an outline and no `backgroundColor` draws no outline. Its default background is a projected ripple, so the whole composite is projected onto an ancestor and hidden behind flattened content. That's the same problem #58902 fixed for the feedback underlay behind `fixBorderlessRippleAndroid`, and it would need the same treatment for the original background. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: javache Differential Revision: D127266607 Pulled By: Abbondanzo fbshipit-source-id: bfb0c98060102fc1e79c9a89f4a5d9acb78306a0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Fixes the issue described in #48552, where a Pressable component can lose both its background color and ripple effect when borderless is true.
Root Cause
Android sets
isProjected() = truefor a borderless RippleDrawable so the ripple can extend beyond the view's bounds. React Native adds the background and ripple as layers in a CompositeBackgroundDrawable, whereisProjected() = trueif any layer is projected.With projection, both the background and ripple are drawn on the nearest native ancestor with a background. Given "root view -> parent view -> Pressable", the ripple and background should now be drawn on the parent. However, since the parent doesn't form a stacking context, Fabric flattens its children. The Pressable and parent become siblings, and the Pressable's background and ripple are now drawn on a further ancestor and consequently become hidden.
Workarounds like opacity,
overflow: hidden, andcollapsable={false}work by establishing a stacking context so the parent remains the Pressable's ancestor.This PR
Modify CompositeBackgroundDrawable to wrap
feedbackUnderlayin a DrawableWrapper whereisProjected() = false. The background and ripple are drawn in-place, so they remain visible without a stacking context. The ripple still extends beyond the view's bounds sinceclipChildren = falsein React Native. This is gated behind thefixBorderlessRippleAndroidflag which defaults to false.Limitations
With projection, the background and borderless ripple are drawn underneath all siblings. Without projection, they are now drawn over previous siblings and underneath following siblings. This matches behavior already shown by React Native foreground ripples, and Jetpack Compose also disables projection in
RippleHostView.UnprojectedRippleand exhibits the same behavior.Disabling ripple projection also causes some behavioral changes with clipping. The ripple is no longer clipped to the outline of the ancestor it was projected on, and it can now be clipped by
overflow: hiddenor ScrollView ancestors it used to draw beyond. These changes also align with foreground ripples and Jetpack Compose.One alternative which more closely preserves projected behavior is to modify Fabric's core renderer logic to maintain a parent stacking context if a borderless ripple is present. However, this can potentially change stacking behavior in non-obvious ways depending on where a ripple is defined, where the nearest ancestor with a background exists, and siblings in between. This impacts zIndex behavior and makes Android's stacking diverge from iOS, where ripples don't exist.
Changelog: [Android][Fixed] - Fix Pressable and TouchableNativeFeedback losing their background color and ripple when using a borderless ripple
Reviewed By: cortinico
Differential Revision: D123471013