Repository navigation
Conversation
…on Android ReactSwitch.setBackgroundColor replaced the view's background with a new RippleDrawable. Outline and boxShadow are drawn by the CompositeBackgroundDrawable that BackgroundStyleApplicator installs as the background, so it was discarded whenever backgroundColor was applied after them. Fabric applies props in map iteration order, so depending on the keys a Switch's outlineColor/outlineOffset could be dropped on mount (with a later outlineWidth recreating the outline with its black, zero-offset defaults), and any backgroundColor update removed the outline and box shadow entirely. Swap the ripple into the composite's original background instead, and let setThumbColor find the ripple inside the composite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…is set setBackgroundColor built the new ripple with the background color, dropping a ripple color set by an earlier thumbColor. Also mutate the ripple before recoloring it, since the theme's ripple may share its state with other drawables loaded from the same resource. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
setThumbColor only stored non-null colors, so after thumbColor was removed the next background color still built its ripple with the old thumb color. Also drop the mutate() call: each RippleDrawable created from a resource copies its RippleState, and setColor only writes to that copy, so recoloring it can't affect other drawables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Visual regression1 visual difference(s) at ios: no capture android
|
|
@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D127266607. |
This branch has not been deployed
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:
On Android,
outline*,boxShadowand theborderRadiusthey're drawn with are kept in theCompositeBackgroundDrawablethatBackgroundStyleApplicatorinstalls as a view's background.Current behavior.
ReactSwitch.setBackgroundColorreplaces the background with a newRippleDrawable, 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 aSwitchkeeps its outline depends on which props it has:{width: 60, height: 40, backgroundColor: 'white', outlineWidth: 3, outlineColor: 'blue', outlineOffset: 8}, the props are applied asoutlineColor,outlineOffset,backgroundColor,outlineWidth.backgroundColordrops the color and offset, andoutlineWidththen creates a new outline with the defaults, so theSwitchmounts with a black outline and no offset. UpdatingoutlineOffsetlater restores the offset, but the color stays black becauseoutlineColorisn't sent again.backgroundColorremoves the outline and box shadow entirely, whatever order they were applied in.thumbColoralso sets the ripple's color. That color is lost whenbackgroundColoris 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, sincesetThumbColoronly checks whether the background itself is aRippleDrawable.With this change.
CompositeBackgroundDrawable,setBackgroundColorreplaces only its original background layer with the ripple, and keeps the other layers.CompositeBackgroundDrawablegets awithNewOriginalBackground, like its otherwithNew*methods.ReactSwitchkeeps the last thumb color and builds the ripple with it (or with the background color oncethumbColoris removed), andsetThumbColorfinds the ripple inside the composite.ReactSwitchis the only view in ReactAndroid that replaces its background insetBackgroundColor; the others go throughBackgroundStyleApplicator. Moving theSwitch's background color toBackgroundStyleApplicator.setBackgroundColorand 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
Switchlosing its outline,boxShadowandthumbColorripple color whenbackgroundColoris setTest 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;thumbColorsets 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 aRippleDrawable, 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-kotlinpasses.Device: OPPO K12x 5G (CPH2667, Android 16), RNTester debug build, New Architecture, with the
Switchabove next to aViewwith the same style. Logging the prop setters andsetBackgroundColor(not part of this PR) showed, for theSwitch:Switchmounts with a black outline and no offset; after settingbackgroundColorto yellow, its outline is gone, and aSwitchwithboxShadow: '0 0 0 6px red'loses its shadow.Switchmatches theViewon mount, after updatingoutlineOffset, and after settingbackgroundColor.thumbColor: 'red'and the outline, long-pressing theSwitchshows 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 settingbackgroundColorto yellow. In each row theSwitchis on the left and aViewwith the same style on the right; the middle row (nobackgroundColor) is the case not fixed here.Before:
After:
Long-pressing a
SwitchwiththumbColor: 'red',backgroundColor: 'white'and a blue outline, onmain(left; the outline is also black there, from the prop order above) and with this change (right):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
thumbColoris removed is covered by the unit test only, not checked on a device.Not fixed here: a
Switchwith an outline and nobackgroundColordraws 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 behindfixBorderlessRippleAndroid, and it would need the same treatment for the original background.🤖 Generated with Claude Code