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

Swift UI styling #2110

Merged
merged 2 commits into from
Aug 1, 2024
Merged

Swift UI styling #2110

merged 2 commits into from
Aug 1, 2024

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Jul 31, 2024

📲 What

Create and use swiftui button styles to match our UIKit styling for buttons. Note that this is done with SwiftUI.ButtonStyles, not using view modifiers. This gives us access to the isPressed field, which looks equivalent to the UIKit highlighted state.

I've only copied over the styles that I think we need. If I've missed anything, either for a different kind of button or something from BaseStyles, we can add it when we need it. (Or let me know, and I can add it here.)

Note that tests don't cover the isPressed state - I couldn't find a way to do this. If it's possible and anyone knows how to do it, let me know, and I'll add a "highlighted" state snapshot test.

👀 See

Jira

Button styles can be seen in the snapshot tests.

✅ Acceptance criteria

  • Button styles match our prelude styles and are easy to use.

@ifosli ifosli self-assigned this Jul 31, 2024
@ifosli ifosli marked this pull request as ready for review July 31, 2024 21:06
func makeBody(configuration: Configuration) -> some View {
configuration.label
.frame(maxWidth: .infinity)
.font(Font(UIFont.boldSystemFont(ofSize: 16)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be ksr_body or one of the other defined fonts? (Same question for the other button styles in this file)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so - I've copied all the details here from our current buttons (except the fact that these buttons can grow to multiple lines), and I don't think this matches any of our current fonts. We should maybe update the font we use on our buttons, but that feels a little out of scope for this. I'll confirm with Alison, though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed with Alison that we're matching old styling with these buttons (and hopefully eventually we'll update all of them to something that matches the rest of the app better!)

Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I think adding the other button styles as we need them makes sense!

@ifosli ifosli merged commit 299f8b1 into main Aug 1, 2024
4 checks passed
@ifosli ifosli deleted the swiftUIStyling branch August 1, 2024 21:18
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.

3 participants