-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
[Android][Fix]Properly set borderRadius on Android #25626
Conversation
In orde to properly set the view's borderRadius the inner*RadiusX/inner*RadiusY should not be used, since their values are obtained via the following formula: int innerTopLeftRadiusX = Math.max(topLeftRadius - borderWidth.left, 0); If topLeftRadius and borderWidth.left have the same value innerTopLeftRadiusX will be zero and it will cause the top left radius to never be set since "(innerTopLeftRadiusX > 0 ? extraRadiusForOutline : 0)" will evaluate to zero. In order to prevent this issue the condition will only consider topLeftRadius, topRightRadius, and etc.
|
Awesome, thanks @cabelitos! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mdvacca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR was approved internally and we are landing it.
Thanks for working on this @cabelitos |
This pull request was successfully merged by @cabelitos in b432b8f. When will my fix make it into a release? | Upcoming Releases |
thank you @mdvacca and your coworkers for reviewing and approving this fix. I'm glad that I could help. |
Summary: In order to properly set the view's borderRadius the inner*RadiusX/inner*RadiusY should not be used, since their values are obtained via the following formula: int innerTopLeftRadiusX = Math.max(topLeftRadius - borderWidth.left, 0); If topLeftRadius and borderWidth.left have the same value innerTopLeftRadiusX will be zero and it will cause the top left radius to never be set since "(innerTopLeftRadiusX > 0 ? extraRadiusForOutline : 0)" will evaluate to zero. In order to prevent this issue the condition will only consider topLeftRadius, topRightRadius, and etc. I took a closer look to see if this fix does not causes a regression in #22511 [Android] [FIX] - Correctly set the border radius on android Pull Request resolved: #25626 Test Plan: Using the following code and Android certify that the border radius is correctly applied. ```javascript import React from "react"; import { ScrollView, StyleSheet, Text, View } from "react-native"; export default class App extends React.Component<Props> { render() { return ( <ScrollView style={styles.container}> <View style={styles.box1}> <Text>borderWidth: 2</Text> <Text>borderRadius: 2</Text> </View> <View style={styles.box2}> <Text>borderWidth: 2</Text> <Text>borderRadius: 2.000001</Text> </View> <View style={styles.box3}> <Text>borderWidth: 5</Text> <Text>borderRadius: 5</Text> </View> <View style={styles.box4}> <Text>borderWidth: 5</Text> <Text>borderRadius: 5.000001</Text> </View> <View style={styles.box5}> <Text>borderWidth: 10</Text> <Text>borderRadius: 10</Text> </View> <View style={styles.box6}> <Text>borderWidth: 10</Text> <Text>borderRadius: 11</Text> </View> <View style={styles.box7}> <Text>borderWidth: 10</Text> <Text>borderRadius: 5</Text> </View> <Text>Testing if this does not cause a regression in https://github.com/facebook/react-native/issues/22511</Text> <View style={{ margin: 10, backgroundColor: 'red', width: 100, height: 100, borderWidth: 5, borderBottomLeftRadius: 15, }} /> <View style={{ margin: 10, backgroundColor: 'green', borderWidth: 5, width: 100, height: 100, }} /> </ScrollView> ); } } const styles = StyleSheet.create({ container: { flex: 1 }, box1: { margin: 10, height: 100, borderRadius: 2, borderWidth: 2, borderColor: "#000000" }, box2: { margin: 10, height: 100, borderRadius: 2.000001, borderWidth: 2, borderColor: "#000000" }, box3: { margin: 10, height: 100, borderRadius: 5, borderWidth: 5, borderColor: "#000000" }, box4: { margin: 10, height: 100, borderRadius: 5.000001, borderWidth: 5, borderColor: "#000000" }, box5: { margin: 10, height: 100, borderRadius: 10, borderWidth: 10, borderColor: "#000000" }, box6: { margin: 10, height: 100, borderRadius: 11, borderWidth: 10, borderColor: "#000000" }, box7: { margin: 10, height: 100, borderRadius: 5, borderWidth: 10, borderColor: "#000000" } }); ```   Closes #25591 Reviewed By: osdnk Differential Revision: D16243602 Pulled By: mdvacca fbshipit-source-id: 1e16572fdf6936aa19c3d4c01ff6434028652942
Summary
In order to properly set the view's borderRadius the innerRadiusX/innerRadiusY should not
be used, since their values are obtained via the following formula:
int innerTopLeftRadiusX = Math.max(topLeftRadius - borderWidth.left, 0);
If topLeftRadius and borderWidth.left have the same value innerTopLeftRadiusX will
be zero and it will cause the top left radius to never be set since
"(innerTopLeftRadiusX > 0 ? extraRadiusForOutline : 0)" will evaluate to zero.
In order to prevent this issue the condition will only consider topLeftRadius, topRightRadius, and etc.
I took a closer look to see if this fix does not causes a regression in #22511
Changelog
[Android] [FIX] - Correctly set the border radius on android
Test Plan
Using the following code and Android certify that the border radius is correctly applied.
Without the fix
With the fix
Closes #25591