Skip to content
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

Merged
merged 8 commits into from
Aug 25, 2024
Merged

Feature/446 gridlines #559

merged 8 commits into from
Aug 25, 2024

Conversation

charles-b-stb
Copy link
Contributor

@charles-b-stb charles-b-stb commented Aug 17, 2024

Issue

Overview (Required)

  • Simply adding dotted gridlines to the grid style timetable view.

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)

Before After

Movie (Optional)

Before After

@charles-b-stb charles-b-stb marked this pull request as ready for review August 20, 2024 14:43
@charles-b-stb
Copy link
Contributor Author

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. 🙇

Copy link
Contributor

@MrSmart00 MrSmart00 left a 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this comment🤔?

Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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👍

Copy link
Contributor

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🙏

@charles-b-stb
Copy link
Contributor Author

@MrSmart00 Responded to comments, thanks!

@charles-b-stb
Copy link
Contributor Author

@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.

Copy link
Contributor

@MrSmart00 MrSmart00 left a 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👍

@charles-b-stb charles-b-stb merged commit cb967d3 into main Aug 25, 2024
5 checks passed
@charles-b-stb charles-b-stb deleted the feature/446-gridlines branch August 25, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Timetable Grid: Grid lines
2 participants