-
Notifications
You must be signed in to change notification settings - Fork 338
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
Added new individual corner radius and border modifier #4328
Conversation
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.
Absolutely beautiful! Had a few questions and suggestions.
|
||
} | ||
|
||
struct RaidusInfo { |
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.
Typo 🙂
- struct RaidusInfo {
+ struct RadiusInfo {
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 is how I leave my mark everywhere I go but... I'll change it just for you 🫶
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.
Haha I'm honored ❤️
let topLeft: CGFloat? | ||
let topRight: CGFloat? | ||
let bottomLeft: CGFloat? | ||
let bottomRight: CGFloat? |
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.
How does this work with RTL languages? Not sure how SwiftUI handles that in general, but should we rename them like so?
- let topLeft: CGFloat?
- let topRight: CGFloat?
- let bottomLeft: CGFloat?
- let bottomRight: CGFloat?
+ let topStart: CGFloat?
+ let topEnd: CGFloat?
+ let bottomStart: CGFloat?
+ let bottomEnd: CGFloat?
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.
I should put a todo in here but we would have to manually flip it in this case for RTL languages since there isn't native support for this. Thanks for catching!
// Start from the top-left corner | ||
path.move(to: CGPoint(x: rect.minX + topLeft, y: rect.minY)) | ||
|
||
// Top edge and top-right corner | ||
path.addLine(to: CGPoint(x: rect.maxX - topRight, y: rect.minY)) | ||
path.addQuadCurve(to: CGPoint(x: rect.maxX, y: rect.minY + topRight), | ||
control: CGPoint(x: rect.maxX, y: rect.minY)) | ||
|
||
// Right edge and bottom-right corner | ||
path.addLine(to: CGPoint(x: rect.maxX, y: rect.maxY - bottomRight)) | ||
path.addQuadCurve(to: CGPoint(x: rect.maxX - bottomRight, y: rect.maxY), | ||
control: CGPoint(x: rect.maxX, y: rect.maxY)) | ||
|
||
// Bottom edge and bottom-left corner | ||
path.addLine(to: CGPoint(x: rect.minX + bottomLeft, y: rect.maxY)) | ||
path.addQuadCurve(to: CGPoint(x: rect.minX, y: rect.maxY - bottomLeft), | ||
control: CGPoint(x: rect.minX, y: rect.maxY)) | ||
|
||
// Left edge and top-left corner | ||
path.addLine(to: CGPoint(x: rect.minX, y: rect.minY + topLeft)) | ||
path.addQuadCurve(to: CGPoint(x: rect.minX + topLeft, y: rect.minY), | ||
control: CGPoint(x: rect.minX, y: rect.minY)) |
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.
Beautiful! 😚👌
topLeft: CGFloat?, | ||
topRight: CGFloat?, | ||
bottomLeft: CGFloat?, | ||
bottomRight: CGFloat? |
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.
Not sure if this makes sense, but we could also make these non-nil and default to 0? That way you can apply rounding to some corners, without having to specify all corners.
} | ||
|
||
var border: BorderInfo? | ||
var radiuses: RaidusInfo? |
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.
Is my understanding correct that we now have 2 ways to define corner radiuses, RadiusInfo
and CornerRadiuses
? Is that necessary, or can we consolidate the 2? I think it would be great if we could have a single concept of "corner radiuses".
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.
Yeaaaahhh, this is kind of confusing 🤔 I'll take another look to see if they can combined in a sensible way 💪
62fe323
to
55f6ad1
Compare
1ba5ddf
to
8fb89a4
Compare
1e9ed50
to
690d572
Compare
8fb89a4
to
fc7311d
Compare
Motivation
We need borders for package selection states
Description
Replaced
.roundedCorner()
view modifier (which was from Paywalls V1) with new.borderCorner()
view modifierAlso added a bunch of SwiftUI previews (that will be used as snapshots once we enable Emerge)
SwiftUI Previews