react-native: RN 0.72: Broken border colors / radius on Android
Description
When borderRadius
is set, borderColor
is backgroundColor
by default (which is wrong), and only borderColor
can change its color, borderTopColor
etc. don’t work (also wrong).
React Native Version
0.72.0-rc.5
Output of npx react-native info
info Fetching system and libraries information...
System:
OS: Linux 5.15 Arch Linux
CPU: (16) x64 AMD Ryzen 7 5800H with Radeon Graphics
Memory: 13.05 GB / 31.35 GB
Shell:
version: "5.9"
path: /bin/zsh
Binaries:
Node:
version: 18.14.2
path: ~/.nvm/versions/node/v18.14.2/bin/node
Yarn:
version: 3.5.0
path: /usr/sbin/yarn
npm:
version: 9.5.0
path: ~/.nvm/versions/node/v18.14.2/bin/npm
Watchman:
version: 20221016.020512.0
path: /usr/sbin/watchman
SDKs:
Android SDK:
API Levels:
- "31"
- "33"
- "33"
Build Tools:
- 30.0.2
- 30.0.3
- 31.0.0
- 33.0.0
- 33.0.2
System Images:
- android-33 | Google APIs Intel x86_64 Atom
Android NDK: Not Found
IDEs:
Android Studio: AI-222.4459.24.2221.9971841
Languages:
Java:
version: 11.0.17
path: /usr/sbin/javac
Ruby: Not Found
npmPackages:
"@react-native-community/cli": Not Found
react: Not Found
react-native: Not Found
npmGlobalPackages:
"*react-native*": Not Found
Android:
hermesEnabled: true
newArchEnabled: false
iOS:
hermesEnabled: true
newArchEnabled: Not found
Steps to reproduce
- Use the example code I provided
- Observe border color is green (same as background)
- Change
borderTopColor
toborderColor
- Observe border color becomes orange
Snack, code example, screenshot, or link to a repository
<View
style={{
width: 128,
height: 128,
borderWidth: 5,
borderRadius: 5,
borderTopColor: 'orange',
backgroundColor: 'green',
}}
/>
About this issue
- Original URL
- State: closed
- Created a year ago
- Comments: 33 (20 by maintainers)
Commits related to this issue
- Fix Android border clip check (#37828) Summary: Instead of requiring all types of border color values to be present we should only take into consideration the left, top, right, bottom, and allEdges ... — committed to facebook/react-native by gabrieldonadel a year ago
- Fix Android border clip check (#37828) Summary: Instead of requiring all types of border color values to be present we should only take into consideration the left, top, right, bottom, and allEdges ... — committed to facebook/react-native by gabrieldonadel a year ago
- Revert "fix: border width top/bottom not matching the border radius" (#37840) Summary: In an effort to fix https://github.com/facebook/react-native/issues/37753, this PR reverts the changes introduce... — committed to facebook/react-native by gabrieldonadel a year ago
- Fix Android border clip check (#37828) Summary: Instead of requiring all types of border color values to be present we should only take into consideration the left, top, right, bottom, and allEdges ... — committed to expo/react-native by gabrieldonadel a year ago
- Revert "fix: border width top/bottom not matching the border radius" (#37840) Summary: In an effort to fix https://github.com/facebook/react-native/issues/37753, this PR reverts the changes introduce... — committed to expo/react-native by gabrieldonadel a year ago
I can start working on this task. I remain available. What timeframe do I have? Thanks
we’ve done the cherry picking, we’re starting testing, unless something unexpected goes horribly wrong in the next 12/24hrs RC6 should come out and soon after 0.71.11
Thanks @fabriziobertoglio1987 for the investigation!
I’ve spent some time this morning diving a bit more into this from the perspective of trying to figure out which commit created the problem; here are a few things I’ve learned.
First off, to repro this problem is also possible to quickly just add the sample code in RNTester, under
packages/rn-tester/js/examples/View/ViewExample.js
; here’s how things look in the 0.72-stable branch with the test code added:So, yes, the Android issue is fully confirmed there. After setting that up, I made a local branch and tried to revert the commits that @NickGerleman pointed out.
By reverting “feat: Add logical border block color properties” https://github.com/facebook/react-native/commit/597a1ff60b3e1844b4794fb4acd40fa073f2e93b I actually got some improvement to the situation, shown here:
Reverting “feat: Add logical border radius implementation” https://github.com/facebook/react-native/commit/4ae4984094e4846bc2bc0e3374ab5d934ee6bc5f on top of the other revert didn’t change anything:
And this is consistent with what Fabrizio is saying, meaning that there was a second underlying bug already present since 0.71.x. I didn’t dig into what in 0.71 might have caused the problem.
So this leaves us into a position where:
I think that if viable, we should properly fix both.
If that’s too complicated/would take more than a couple days, we should revert the commit by Gabriel, bring back the status of things to the one we had in 0.71, and proceed with 0.72.0 while proper fixes are landed in main and then backported to 0.71 and 0.72.
For sake of completeness, @cortinico found this commit which touched that file in 0.72
@fabriziobertoglio1987 I don’t know if the example I put up is accurate, but I expect my UI elements to not lose borders due to a RN upgrade, is all
Relevant changes:
cc @gabrieldonadel @lunaleaps @necolas
update, released both:
thanks again everyone for your work, it’s really important to find and tackle this type of issues before stable release 🙇♂️
great stuff @gabrieldonadel! I locally applied both of your PRs on top of 0.72 locally and I can confirm that with both, the regression is fully addressed 🎉
And by doing the revert of relevant commit in 0.71 I can confirm it fixes that problem too -> https://github.com/facebook/react-native/issues/36569#issuecomment-1589010699
Ok, I did some more investigation on this. I tested multiple versions of react-native until I found the one where
borderColor
+borderRadius
was working, the last version where this was working 100% was0.70.6
, which means that something on 0.71 broke thisThis is fixed here https://github.com/facebook/react-native/pull/37828
Iirc there were some changes here added for impl of web props. Let me see if I can find the commit.
Hello everyone.
This issue seems to have similarities with a previously reported issue in version 0.71.x.
Here’s the link to the previous issue for reference: #36569.
Thanks @gabrieldonadel for your amazing help!
We’ll pick the following into 0.72
The issue is caused by the missing clipping effect of the top border when using borderColorTop. The same issue does not reproduce when using borderColor.
This is the result after commenting the logic responsible for drawing top border.
The clipping is done from updatePath: https://github.com/facebook/react-native/blob/03f70bf995379f08a77abcf96bb0e31ff75ca8c3/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java#L581 https://github.com/facebook/react-native/blob/03f70bf995379f08a77abcf96bb0e31ff75ca8c3/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java#L792-L814
@gabrieldonadel is this something that you might be willing to take a look at as the author of the changes?
We can also otherwise revert these and see if it resolves the issue. Though due to the nature of the source build, it does seem like that might need another RC if we cannot locally repro.
@MicroDroid Actually,
main
is NOT stable. The stable versions’ code lives in their own*-stable
branch (e.g.:0.71-stable
) and the respective RCs, stable, and patches are released from these respective branches only. Once the PR w/ fix is opened/merged you can expect more details about the same under this issue only 😃Hopefully, that won’t be the case, as Lorenzo said 👇: