-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
add support for borderTopLeftRadius and the three other one-corner-on… #1871
Changes from all commits
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 |
---|---|---|
|
@@ -161,15 +161,53 @@ public void SetSource(Border view, JArray sources) | |
} | ||
} | ||
|
||
/// <summary> | ||
/// Enum values correspond to positions of prop names in ReactPropGroup attribute | ||
/// applied to <see cref="SetBorderRadius(Border, int, double)"/> | ||
/// </summary> | ||
private enum Radius | ||
{ | ||
All, | ||
TopLeft, | ||
TopRight, | ||
BottomLeft, | ||
BottomRight, | ||
} | ||
|
||
/// <summary> | ||
/// The border radius of the <see cref="ReactRootView"/>. | ||
/// </summary> | ||
/// <param name="view">The image view instance.</param> | ||
/// <param name="index">The prop index.</param> | ||
/// <param name="radius">The border radius value.</param> | ||
[ReactProp("borderRadius")] | ||
public void SetBorderRadius(Border view, double radius) | ||
[ReactPropGroup( | ||
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. modify WPF implementation to match as well. test prop in Playground and provide code+screenshot for both UWP and WPF. 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. @matthargett I have another PR ready to be submitted which fixes a bug in borderRadius and which has WPF part updated too. Is it okay if you approve this PR and I will do the Playground screenshots and code as part of the next PR? 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'd prefer to see a contribution upstream for a 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. ReactNative already has examples of border*Radius at https://github.com/facebook/react-native/edit/master/RNTester/js/BorderExample.js see styles border6, border9, border10, border14. 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. Yes - but not for Image 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. |
||
ViewProps.BorderRadius, | ||
ViewProps.BorderTopLeftRadius, | ||
ViewProps.BorderTopRightRadius, | ||
ViewProps.BorderBottomLeftRadius, | ||
ViewProps.BorderBottomRightRadius)] | ||
public void SetBorderRadius(Border view, int index, double radius) | ||
{ | ||
view.CornerRadius = new CornerRadius(radius); | ||
var cornerRadius = view.CornerRadius; | ||
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.
Can
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. Spoke with Yury. In reply to: 197945901 [](ancestors = 197945901) |
||
switch ((Radius)index) | ||
{ | ||
case Radius.All: | ||
cornerRadius = new CornerRadius(radius); | ||
break; | ||
case Radius.TopLeft: | ||
cornerRadius.TopLeft = radius; | ||
break; | ||
case Radius.TopRight: | ||
cornerRadius.TopRight = radius; | ||
break; | ||
case Radius.BottomLeft: | ||
cornerRadius.BottomLeft = radius; | ||
break; | ||
case Radius.BottomRight: | ||
cornerRadius.BottomRight = radius; | ||
break; | ||
} | ||
view.CornerRadius = cornerRadius; | ||
} | ||
|
||
/// <summary> | ||
|
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.
BEGIN NITPICK
In general I don't know why we add private enums when ints work just fine.
END NITPICK