fix: border rendering problem in Android #36129
Conversation
facebook-github-bot
added
the
CLA Signed
Base commit: 96df8c0 |
|
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
/rebase |
| @@ -963,7 +965,9 @@ private void updatePath() { | |||
| } | |||
|
|
|||
| mInnerBottomRightCorner.x = mInnerClipTempRectForBorderRadius.right; | |||
| mInnerBottomRightCorner.y = mInnerClipTempRectForBorderRadius.bottom * -2; | |||
| mInnerBottomRightCorner.y = borderWidth.bottom != 0 | |||
There was a problem hiding this comment.
Could you explain why don't need this logic for mInnerTopLeftCorner and mInnerTopRightCorner?
There was a problem hiding this comment.
Yes, it can be added (my bad, I kinda had tunnel vision). In the cases where this bug happened it won't cause any difference though, because mInnerClipTempRectForBorderRadius.top was always 0. Do you want me to make a commit adding the same logic to mInnerTopLeftCorner and mInnerTopRightCorner?
There was a problem hiding this comment.
If it doesn't make a difference, we shouldn't add it. So you're saying we don't need similar logic for the top corners?
There was a problem hiding this comment.
I'm not totally sure. From what I saw from the code, mInnerClipTempRectForBorderRadius is set based on the getBounds of the drawable. In cases where the bounds don't change, top will always be 0. Is there any possibility of the bounds changing in this case?
Update: ah, there is if we change the size of drawable...I'll test it (nvm, not the case, from what I can tell top will always remain 0)
|
Looks like this logic was changed not too long ago in #34362. It seems there was no test-case added at that time, can you verify this doesn't regress it? |
Seems like there is no regression 👍 |
Summary: Fixes facebook#36036 The problem was in `ReactViewBackgroundDrawable.java` that was not accounting for adjacent borders that add width set to 0. ## Changelog [Android] [Fixed] - Fix border rendering issue when bottom borders has no width Pull Request resolved: facebook#36129 Test Plan: | Previously | Now with the fix | | --------------- | --------------- | | <img width="417" alt="image" src="https://user-images.githubusercontent.com/25725586/218149384-00e2145c-3c84-4590-87be-3258574489e5.png"> | <img width="414" alt="image" src="https://user-images.githubusercontent.com/25725586/218148215-a8d37158-0feb-47ae-874b-cba2f422d792.png"> | Reviewed By: cipolleschi Differential Revision: D43303228 Pulled By: javache fbshipit-source-id: cf9d30fe12a5740d9ee8974a66904fd0850e7606
|
@javache Please add this to 0.71 |
|
@zbranevichfinstek: please raise this in the 0.71 release thread - https://github.com/reactwg/react-native-releases/discussions |
|
I don't want to upgrade the react native version, so is there any way to fix this issue? Dependencies: { |
Summary
Fixes #36036
The problem was in
ReactViewBackgroundDrawable.javathat was not accounting for adjacent borders that add width set to 0.Changelog
[Android] [Fixed] - Fix border rendering issue when bottom borders has no width
Test Plan