-
Notifications
You must be signed in to change notification settings - Fork 199
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
Feature/446 gridlines #559
Conversation
…for some reason (wip)
…gi/conference-app-2024 into feature/446-gridlines
…vertical grid lines yet.
…ng but if someone can get vertical grid lines working I would appreciate it.
Let's try just adding horizontal graph lines for now. If anyone can think of a reasonable way to add vertical lines I would appreciate it. 🙇 |
… feature/446-gridlines
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.
THX!!
I left comments!
please check them🙏
} else { | ||
shape.frame(width: 1).padding(0) | ||
} | ||
//shape |
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.
What's this comment🤔?
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.
Removed
} | ||
|
||
struct LineShape: Shape { | ||
public var axis: Axis |
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 property is 'let' more than better👍
@@ -192,6 +194,39 @@ struct TimeGroupMiniList: View { | |||
} | |||
} | |||
|
|||
struct DashedDivider: View { | |||
public var axis: Axis |
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 property is 'let' more than better👍
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.
you should change here too🙏
@MrSmart00 Responded to comments, thanks! |
@MrSmart00 Sorry I missed that last time (it looked too similar to the other item I corrected and I did not notice they were separate the first time I looked at it.) Fixed now. |
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.
Thanks your commits!
LGTM👍
Issue
Overview (Required)
Draft: The vertical lines are not yet working and I am looking for advice.
Branched off of the Grid UI pull request so that will need to be merged first.
Links
Screenshot (Optional if screenshot test is present or unrelated to UI)
Movie (Optional)