Repository navigation
Commit 19d6d17
Fix Switch losing outline and box shadow when backgroundColor is set 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: bfb0c98060102fc1e79c9a89f4a5d9acb78306a01 parent 1d9ed87 commit 19d6d17
3 files changed
Lines changed: 179 additions & 6 deletions
File tree
- packages/react-native/ReactAndroid/src
- main/java/com/facebook/react
- uimanager/drawable
- views/switchview
- test/java/com/facebook/react/views/switchview
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
167 | 167 | | |
168 | 168 | | |
169 | 169 | | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
170 | 186 | | |
171 | 187 | | |
172 | 188 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
26 | 27 | | |
27 | 28 | | |
28 | 29 | | |
| 30 | + | |
29 | 31 | | |
30 | 32 | | |
31 | 33 | | |
| |||
41 | 43 | | |
42 | 44 | | |
43 | 45 | | |
44 | | - | |
45 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
46 | 56 | | |
47 | 57 | | |
48 | 58 | | |
| |||
60 | 70 | | |
61 | 71 | | |
62 | 72 | | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
67 | 81 | | |
68 | 82 | | |
69 | 83 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
0 commit comments