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

Consistently styled buttons #483

Merged
merged 4 commits into from
Oct 4, 2024

Conversation

krugerk
Copy link
Contributor

@krugerk krugerk commented Sep 30, 2024

Use a more consistent iOS styling for in app buttons.

Example:

See #481 (comment)

@theospears
Copy link
Collaborator

I think it's possible this style will be divisive - personally I find it quite loud. But I'm happy to put it in a TestFlight build and collect feedback.

Could you update the PR to merge cleanly, now I have merged the test fixes in #481

@krugerk krugerk force-pushed the consistently-styled-buttons branch from f4f2491 to ed78101 Compare October 3, 2024 09:57
@theospears theospears merged commit 26bf923 into beeminder:master Oct 4, 2024
1 check passed
@theospears
Copy link
Collaborator

theospears commented Oct 4, 2024

I've merged this because I don't think it's a great use of time to tease it apart, but to set expectations, in the future I will not merge PRs which combine behavior changes (like the styling here) and widespread code style changes (like UIColor.beeminder -> UIColor.Beeminder) in the same PR. Small code style changes localized to only the places where there are behavior changes are ok.

The reasons why are:

  1. It makes it harder to review the PR - have to think carefully to decide whether changes are behavioral or not
  2. It makes reverting harder if we decide to change the behavior, as it is more likely to conflict with later PRs

(I'm not looking for a discussion about whether or not this is the correct choice, just telling you the decision and giving some context.)

@theospears
Copy link
Collaborator

I think we should probably make this have a light background in light mode?

IMG_BE5BE99DAF80-1

@krugerk krugerk deleted the consistently-styled-buttons branch October 6, 2024 12:30
@krugerk
Copy link
Contributor Author

krugerk commented Oct 6, 2024

I think we should probably make this have a light background in light mode?

IMG_BE5BE99DAF80-1

Sure, let us try it. How about #489

theospears pushed a commit that referenced this pull request Oct 6, 2024
This addresses the blemish as seen in the screenshot in
#483 (comment).
A square border is drawn around an otherwise rounded button. Now the
border is also rounded.
theospears pushed a commit that referenced this pull request Dec 16, 2024
## Summary
Sign In button had dark corners on light mode although it had rounded
corners. The two did not look good together.

## Validation
ran the app in the simulator

## Issue
Fixes #551 
Related #483
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants