Skip to content

Fix Android border positioning regression - #32398

Closed
oblador wants to merge 1 commit into
react:mainfrom
oblador:fix-android-border-regression
Closed

Fix Android border positioning regression#32398
oblador wants to merge 1 commit into
react:mainfrom
oblador:fix-android-border-regression

Conversation

@oblador

Copy link
Copy Markdown
Contributor

Summary

#29099 introduced a regression where non-rounded borders on Android would render partly outside of the bounds of the view as I reported in #32393. This PR addresses that by rendering the borders completely inside the view like it works on iOS, previous version of RN and for rounded corners.

Changelog

[Android] [Fixed] - Fix Android border positioning regression

Test Plan

Rendering the following code (as reported in the issue) in the RN Tester app:

<Viewstyle={{aspectRatio: 1,backgroundColor: 'green',borderWidth: 8,borderColor: 'black',borderStyle: 'dashed',}}/>
BeforeAfter
beforeafter

@pull-bot

Copy link
Copy Markdown

PR build artifact for 5a01e8f is ready.
To use, download tarball from this CircleCI job then run yarn add <path to tarball> in your React Native project.

@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a7,717,953-33
androidhermesarmeabi-v7a7,246,018-34
androidhermesx868,137,536-34
androidhermesx86_648,103,027-35
androidjscarm64-v8a9,637,144-86
androidjscarmeabi-v7a8,552,739-81
androidjscx869,651,010-86
androidjscx86_6410,260,183-87

Base commit: 8ba4a2f

@facebook-github-botfacebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 13, 2021
@mikehardy

Copy link
Copy Markdown
Contributor

One would expect that with a .java only change, the iOS CI failures are not related.
This looks great!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@lunaleaps has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@yungsters

Copy link
Copy Markdown
Contributor

Sweet. Thanks for finding and fixing this, @oblador.

Looks like this regression was introduced by cb0e1d6, tagging @fabriziobertoglio1987 as a heads up.

@facebook-github-botfacebook-github-bot added the Merged This PR has been merged. label Oct 14, 2021
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@lunaleaps merged this pull request in d1a33cd.

@oblador

Copy link
Copy Markdown
ContributorAuthor

Thanks for the super quick resolution 🙏 👏

lunaleaps pushed a commit that referenced this pull request Oct 15, 2021
Summary:
#29099 introduced a regression where non-rounded borders on Android would render partly outside of the bounds of the view as I reported in #32393. This PR addresses that by rendering the borders completely inside the view like it works on iOS, previous version of RN and for rounded corners.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[Android] [Fixed] - Fix Android border positioning regression
Pull Request resolved: #32398
Test Plan:
Rendering the following code (as reported in the issue) in the RN Tester app:
```jsx
<View
style={{
aspectRatio: 1,
backgroundColor: 'green',
borderWidth: 8,
borderColor: 'black',
borderStyle: 'dashed',
}}
/>
```
|Before|After|
|--|--|
|![before](https://user-images.githubusercontent.com/378279/137178113-dd2fea7e-48c8-450b-be3a-92706ef93194.png)|![after](https://user-images.githubusercontent.com/378279/137178140-b5ce7b3d-d455-48a9-a57f-0f3194a65c9a.png)|
Reviewed By: yungsters
Differential Revision: D31623647
Pulled By: lunaleaps
fbshipit-source-id: c38d172ae4a9dc48f800c63258223a59e2f621ed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugCLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@oblador@pull-bot@analysis-bot@mikehardy@facebook-github-bot@yungsters@react-native-bot