-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
feat: Add logical border radius implementation #35572
Changes from 4 commits
f165b23
04db275
dae78fd
b0dcb5a
d3d2bd9
71fcbdd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,6 +128,10 @@ - (instancetype)initWithFrame:(CGRect)frame | |
_borderBottomRightRadius = -1; | ||
_borderBottomStartRadius = -1; | ||
_borderBottomEndRadius = -1; | ||
_borderEndEndRadius = -1; | ||
_borderEndStartRadius = -1; | ||
_borderStartEndRadius = -1; | ||
_borderStartStartRadius = -1; | ||
_borderCurve = RCTBorderCurveCircular; | ||
_borderStyle = RCTBorderStyleSolid; | ||
_hitTestEdgeInsets = UIEdgeInsetsZero; | ||
|
@@ -668,10 +672,10 @@ - (RCTCornerRadii)cornerRadii | |
CGFloat bottomRightRadius; | ||
|
||
if ([[RCTI18nUtil sharedInstance] doLeftAndRightSwapInRTL]) { | ||
const CGFloat topStartRadius = RCTDefaultIfNegativeTo(_borderTopLeftRadius, _borderTopStartRadius); | ||
const CGFloat topEndRadius = RCTDefaultIfNegativeTo(_borderTopRightRadius, _borderTopEndRadius); | ||
const CGFloat bottomStartRadius = RCTDefaultIfNegativeTo(_borderBottomLeftRadius, _borderBottomStartRadius); | ||
const CGFloat bottomEndRadius = RCTDefaultIfNegativeTo(_borderBottomRightRadius, _borderBottomEndRadius); | ||
const CGFloat topStartRadius = RCTDefaultIfNegativeTo(_borderTopLeftRadius, _borderStartStartRadius > 0 ? _borderStartStartRadius: _borderTopStartRadius); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah good point, I guess we could check for |
||
const CGFloat topEndRadius = RCTDefaultIfNegativeTo(_borderTopRightRadius, _borderStartEndRadius > 0 ? _borderStartEndRadius: _borderTopEndRadius); | ||
const CGFloat bottomStartRadius = RCTDefaultIfNegativeTo(_borderBottomLeftRadius, _borderEndStartRadius > 0 ? _borderEndStartRadius: _borderBottomStartRadius); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with other properties here (and Yoga's stylesheet behavior), we should give preference to the cardinal direction over the flow relative one. E.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, I've just updated it |
||
const CGFloat bottomEndRadius = RCTDefaultIfNegativeTo(_borderBottomRightRadius, _borderEndEndRadius > 0 ? _borderEndEndRadius: _borderBottomEndRadius); | ||
|
||
const CGFloat directionAwareTopLeftRadius = isRTL ? topEndRadius : topStartRadius; | ||
const CGFloat directionAwareTopRightRadius = isRTL ? topStartRadius : topEndRadius; | ||
|
@@ -683,10 +687,10 @@ - (RCTCornerRadii)cornerRadii | |
bottomLeftRadius = RCTDefaultIfNegativeTo(radius, directionAwareBottomLeftRadius); | ||
bottomRightRadius = RCTDefaultIfNegativeTo(radius, directionAwareBottomRightRadius); | ||
} else { | ||
const CGFloat directionAwareTopLeftRadius = isRTL ? _borderTopEndRadius : _borderTopStartRadius; | ||
const CGFloat directionAwareTopRightRadius = isRTL ? _borderTopStartRadius : _borderTopEndRadius; | ||
const CGFloat directionAwareBottomLeftRadius = isRTL ? _borderBottomEndRadius : _borderBottomStartRadius; | ||
const CGFloat directionAwareBottomRightRadius = isRTL ? _borderBottomStartRadius : _borderBottomEndRadius; | ||
const CGFloat directionAwareTopLeftRadius = isRTL ? _borderStartEndRadius > 0 ? _borderStartEndRadius: _borderTopEndRadius : _borderStartStartRadius > 0 ? _borderStartStartRadius: _borderTopStartRadius; | ||
const CGFloat directionAwareTopRightRadius = isRTL ? _borderStartStartRadius > 0 ? _borderStartStartRadius: _borderTopStartRadius : _borderStartEndRadius > 0 ? _borderStartEndRadius: _borderTopEndRadius; | ||
const CGFloat directionAwareBottomLeftRadius = isRTL ? _borderEndEndRadius > 0 ? _borderEndEndRadius: _borderBottomEndRadius : _borderEndStartRadius > 0 ? _borderEndStartRadius: _borderBottomStartRadius; | ||
const CGFloat directionAwareBottomRightRadius = isRTL ? _borderEndStartRadius > 0 ? _borderEndStartRadius: _borderBottomStartRadius : _borderEndEndRadius > 0 ? _borderEndEndRadius: _borderBottomEndRadius; | ||
|
||
topLeftRadius = | ||
RCTDefaultIfNegativeTo(radius, RCTDefaultIfNegativeTo(_borderTopLeftRadius, directionAwareTopLeftRadius)); | ||
|
@@ -946,7 +950,8 @@ -(void)setBorder##side##Radius : (CGFloat)radius \ | |
|
||
setBorderRadius() setBorderRadius(TopLeft) setBorderRadius(TopRight) setBorderRadius(TopStart) | ||
setBorderRadius(TopEnd) setBorderRadius(BottomLeft) setBorderRadius(BottomRight) | ||
setBorderRadius(BottomStart) setBorderRadius(BottomEnd) | ||
setBorderRadius(BottomStart) setBorderRadius(BottomEnd) setBorderRadius(EndEnd) | ||
setBorderRadius(EndStart) setBorderRadius(StartEnd) setBorderRadius(StartStart) | ||
|
||
#pragma mark - Border Curve | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,7 +128,11 @@ public enum BorderRadiusLocation { | |
TOP_START, | ||
TOP_END, | ||
BOTTOM_START, | ||
BOTTOM_END | ||
BOTTOM_END, | ||
END_END, | ||
END_START, | ||
START_END, | ||
START_START | ||
} | ||
|
||
public ReactViewBackgroundDrawable(Context context) { | ||
|
@@ -271,7 +275,7 @@ public void setRadius(float radius) { | |
|
||
public void setRadius(float radius, int position) { | ||
if (mBorderCornerRadii == null) { | ||
mBorderCornerRadii = new float[8]; | ||
mBorderCornerRadii = new float[12]; | ||
Arrays.fill(mBorderCornerRadii, YogaConstants.UNDEFINED); | ||
} | ||
|
||
|
@@ -581,8 +585,13 @@ private void updatePath() { | |
float bottomStartRadius = getBorderRadius(BorderRadiusLocation.BOTTOM_START); | ||
float bottomEndRadius = getBorderRadius(BorderRadiusLocation.BOTTOM_END); | ||
|
||
float endEndRadius = getBorderRadius(BorderRadiusLocation.END_END); | ||
float endStartRadius = getBorderRadius(BorderRadiusLocation.END_START); | ||
float startEndRadius = getBorderRadius(BorderRadiusLocation.START_END); | ||
float startStartRadius = getBorderRadius(BorderRadiusLocation.START_START); | ||
|
||
if (I18nUtil.getInstance().doLeftAndRightSwapInRTL(mContext)) { | ||
if (YogaConstants.isUndefined(topStartRadius)) { | ||
if (YogaConstants.isUndefined(topStartRadius)) { | ||
topStartRadius = topLeftRadius; | ||
} | ||
|
||
|
@@ -598,20 +607,30 @@ private void updatePath() { | |
bottomEndRadius = bottomRightRadius; | ||
} | ||
|
||
final float directionAwareTopLeftRadius = isRTL ? topEndRadius : topStartRadius; | ||
final float directionAwareTopRightRadius = isRTL ? topStartRadius : topEndRadius; | ||
final float directionAwareBottomLeftRadius = isRTL ? bottomEndRadius : bottomStartRadius; | ||
final float directionAwareBottomRightRadius = isRTL ? bottomStartRadius : bottomEndRadius; | ||
final float logicalTopStartRadius = !YogaConstants.isUndefined(startStartRadius) ? startStartRadius : topStartRadius; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above re reversing the precedence here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, updated |
||
final float logicalTopEndRadius = !YogaConstants.isUndefined(startEndRadius) ? startEndRadius : topEndRadius; | ||
final float logicalBottomStartRadius = !YogaConstants.isUndefined(endStartRadius) ? endStartRadius : bottomStartRadius; | ||
final float logicalBottomEndRadius = !YogaConstants.isUndefined(endEndRadius) ? endEndRadius : bottomEndRadius; | ||
|
||
final float directionAwareTopLeftRadius = isRTL ? logicalTopEndRadius : logicalTopStartRadius; | ||
final float directionAwareTopRightRadius = isRTL ? logicalTopStartRadius : logicalTopEndRadius; | ||
final float directionAwareBottomLeftRadius = isRTL ? logicalBottomEndRadius : logicalBottomStartRadius; | ||
final float directionAwareBottomRightRadius = isRTL ? logicalBottomStartRadius : logicalBottomEndRadius; | ||
|
||
topLeftRadius = directionAwareTopLeftRadius; | ||
topRightRadius = directionAwareTopRightRadius; | ||
bottomLeftRadius = directionAwareBottomLeftRadius; | ||
bottomRightRadius = directionAwareBottomRightRadius; | ||
} else { | ||
final float directionAwareTopLeftRadius = isRTL ? topEndRadius : topStartRadius; | ||
final float directionAwareTopRightRadius = isRTL ? topStartRadius : topEndRadius; | ||
final float directionAwareBottomLeftRadius = isRTL ? bottomEndRadius : bottomStartRadius; | ||
final float directionAwareBottomRightRadius = isRTL ? bottomStartRadius : bottomEndRadius; | ||
final float logicalTopStartRadius = !YogaConstants.isUndefined(startStartRadius) ? startStartRadius : topStartRadius; | ||
final float logicalTopEndRadius = !YogaConstants.isUndefined(startEndRadius) ? startEndRadius : topEndRadius; | ||
final float logicalBottomStartRadius = !YogaConstants.isUndefined(endStartRadius) ? endStartRadius : bottomStartRadius; | ||
final float logicalBottomEndRadius = !YogaConstants.isUndefined(endEndRadius) ? endEndRadius : bottomEndRadius; | ||
|
||
final float directionAwareTopLeftRadius = isRTL ? logicalTopEndRadius : logicalTopStartRadius; | ||
final float directionAwareTopRightRadius = isRTL ? logicalTopStartRadius : logicalTopEndRadius; | ||
final float directionAwareBottomLeftRadius = isRTL ? logicalBottomEndRadius : logicalBottomStartRadius; | ||
final float directionAwareBottomRightRadius = isRTL ? logicalBottomStartRadius : logicalBottomEndRadius; | ||
|
||
if (!YogaConstants.isUndefined(directionAwareTopLeftRadius)) { | ||
topLeftRadius = directionAwareTopLeftRadius; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -751,6 +751,26 @@ void YogaLayoutableShadowNode::swapLeftAndRightInViewProps( | |
auto &props = const_cast<ViewProps &>(typedCasting); | ||
|
||
// Swap border node values, borderRadii, borderColors and borderStyles. | ||
if (props.borderRadii.startEnd.has_value()) { | ||
props.borderRadii.topEnd = props.borderRadii.startEnd; | ||
props.borderRadii.startEnd.reset(); | ||
} | ||
|
||
if (props.borderRadii.startStart.has_value()) { | ||
props.borderRadii.topStart = props.borderRadii.startStart; | ||
props.borderRadii.startStart.reset(); | ||
} | ||
|
||
if (props.borderRadii.endEnd.has_value()) { | ||
props.borderRadii.bottomEnd = props.borderRadii.endEnd; | ||
props.borderRadii.endEnd.reset(); | ||
} | ||
|
||
if (props.borderRadii.endStart.has_value()) { | ||
props.borderRadii.bottomStart = props.borderRadii.endStart; | ||
props.borderRadii.endStart.reset(); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the examples below, they are always changing a left-or-right cardinal direction to a flow-relative one. E.g. nothing is done to Are you sure the added code here is needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the review @NickGerleman, I did some investigation and actually, this code is not necessary, everything works just fine without it. I've pushed another commit and now all comments should've been addressed |
||
if (props.borderRadii.topLeft.has_value()) { | ||
props.borderRadii.topStart = props.borderRadii.topLeft; | ||
props.borderRadii.topLeft.reset(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,12 +160,24 @@ struct CascadedRectangleCorners { | |
OptionalT bottomStart{}; | ||
OptionalT bottomEnd{}; | ||
OptionalT all{}; | ||
OptionalT endEnd{}; | ||
OptionalT endStart{}; | ||
OptionalT startEnd{}; | ||
OptionalT startStart{}; | ||
|
||
Counterpart resolve(bool isRTL, T defaults) const { | ||
const auto topLeading = isRTL ? topEnd : topStart; | ||
const auto topTrailing = isRTL ? topStart : topEnd; | ||
const auto bottomLeading = isRTL ? bottomEnd : bottomStart; | ||
const auto bottomTrailing = isRTL ? bottomStart : bottomEnd; | ||
const auto topLeading = isRTL ? startEnd ? startEnd : topEnd | ||
: startStart ? startStart | ||
: topStart; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as elsewhere about precedence, but this ternary is a bit hard to read as-is. Could we avoid the nested ternary (or maybe this is fine when formatted)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that logic was pretty confusing, I've just updated it making things a bit easier to read |
||
const auto topTrailing = isRTL ? startStart ? startStart : topStart | ||
: startEnd ? startEnd | ||
: topEnd; | ||
const auto bottomLeading = isRTL ? endEnd ? endEnd : bottomEnd | ||
: endStart ? endStart | ||
: bottomStart; | ||
const auto bottomTrailing = isRTL ? endStart ? endStart : bottomStart | ||
: endEnd ? endEnd | ||
: bottomEnd; | ||
|
||
return { | ||
/* .topLeft = */ topLeft.value_or( | ||
|
@@ -189,7 +201,11 @@ struct CascadedRectangleCorners { | |
this->topEnd, | ||
this->bottomStart, | ||
this->bottomEnd, | ||
this->all) == | ||
this->all, | ||
this->endEnd, | ||
this->endStart, | ||
this->startEnd, | ||
this->startStart) == | ||
std::tie( | ||
rhs.topLeft, | ||
rhs.topRight, | ||
|
@@ -199,7 +215,11 @@ struct CascadedRectangleCorners { | |
rhs.topEnd, | ||
rhs.bottomStart, | ||
rhs.bottomEnd, | ||
rhs.all); | ||
rhs.all, | ||
rhs.endEnd, | ||
rhs.endStart, | ||
rhs.startEnd, | ||
rhs.startStart); | ||
} | ||
|
||
bool operator!=(const CascadedRectangleCorners<T> &rhs) const { | ||
|
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.
cc @yungsters who recently updated these for prop-types support facebook/react-native-deprecated-modules@5902411
RN's exported prop-types for
View
aren't enforced by RN itself, like it does forText
orImage
. But I suspect we might still want to update ViewPropTypes before RN re-ejects it.