Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Android border positioning regression #32398

Closed
wants to merge 1 commit into from

Conversation

oblador
Copy link
Contributor

@oblador oblador commented Oct 13, 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

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

Test Plan

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

<View
  style={{
    aspectRatio: 1,
    backgroundColor: 'green',
    borderWidth: 8,
    borderColor: 'black',
    borderStyle: 'dashed',
  }}
/>
Before After
before after

@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Oct 13, 2021
@pull-bot
Copy link

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

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,717,953 -33
android hermes armeabi-v7a 7,246,018 -34
android hermes x86 8,137,536 -34
android hermes x86_64 8,103,027 -35
android jsc arm64-v8a 9,637,144 -86
android jsc armeabi-v7a 8,552,739 -81
android jsc x86 9,651,010 -86
android jsc x86_64 10,260,183 -87

Base commit: 8ba4a2f

@facebook-github-bot facebook-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
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
Contributor

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

@yungsters
Copy link
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-bot facebook-github-bot added the Merged This PR has been merged. label Oct 14, 2021
@facebook-github-bot
Copy link
Contributor

@lunaleaps merged this pull request in d1a33cd.

@oblador
Copy link
Contributor Author

oblador commented Oct 14, 2021

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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications. Shared with Meta Applied 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