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

  1. Use the example code I provided
  2. Observe border color is green (same as background)
  3. Change borderTopColor to borderColor
  4. 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

Most upvoted comments

I can start working on this task. I remain available. What timeframe do I have? Thanks

+5
fabOnReact on Jun 9, 2023

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

+4
kelset on Jun 13, 2023

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: Screenshot 2023-06-12 at 11 23 21

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: revert just logical border block

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: revert both

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.

+4
kelset on Jun 12, 2023

For sake of completeness, @cortinico found this commit which touched that file in 0.72

+3
cipolleschi on Jun 12, 2023

@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

+3
MicroDroid on Jun 10, 2023

Relevant changes:

  1. https://github.com/facebook/react-native/pull/35572
  2. https://github.com/facebook/react-native/pull/35999

cc @gabrieldonadel @lunaleaps @necolas

+3
NickGerleman on Jun 8, 2023

update, released both:

thanks again everyone for your work, it’s really important to find and tackle this type of issues before stable release 🙇‍♂️

+2
kelset on Jun 15, 2023

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 🎉

Screenshot 2023-06-13 at 10 52 58

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

+2
kelset on Jun 13, 2023

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% was 0.70.6, which means that something on 0.71 broke this

image
+2
gabrieldonadel on Jun 12, 2023
  • there’s a bigger regression on top of it, the one reported here, that is caused by 597a1ff by @gabrieldonadel

This is fixed here https://github.com/facebook/react-native/pull/37828

+2
gabrieldonadel on Jun 12, 2023

Iirc there were some changes here added for impl of web props. Let me see if I can find the commit.

+2
NickGerleman on Jun 8, 2023

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.

+1
leymytel on Jun 12, 2023

Thanks @gabrieldonadel for your amazing help!

We’ll pick the following into 0.72

+1
lunaleaps on Jun 12, 2023

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.

  • When using borderColor, the rectangle is clipped on the top to display the border
  • When using borderTopColor, the rectangle is not clipped on the top and the border is not displayed
borderColor (fixed) borderTopColor (not fixed)
<image src="https://user-images.githubusercontent.com/24992535/244877294-b2f93151-91b1-46f1-bbdd-6f0d711755bc.png" width="350" /> <image src="https://user-images.githubusercontent.com/24992535/244877399-b17a2392-b496-4fcc-9764-845bc03062ab.png" width="350" />

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

+1
fabOnReact on Jun 10, 2023

@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.

+1
NickGerleman on Jun 9, 2023

So once you fix this, could I install react-native package from main GitHub branch so that I can make a new release for the app I’m working on? Or would that not work?

I ask because I am not sure how native code is bundled or whatever.

@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 😃

(and I don’t want to wait 2 more weeks for another RC)

Hopefully, that won’t be the case, as Lorenzo said 👇:

“This looks like something that needs to be addressed ASAP” - https://github.com/facebook/react-native/issues/37753#issuecomment-1582346453

+1
Pranav-yadav on Jun 8, 2023
Read more comments on GitHub
← react-native: [RN 0.70.0-rc.2] React Native Lottie - function getModule: assertion failed (moduleAsValue.isObject())
react-native: RN 0.72-rc.3 release builds crash on startup due to bytecode version mismatch →