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

Lighter button fill in light mode #489

Merged

Conversation

krugerk
Copy link
Contributor

@krugerk krugerk commented Oct 6, 2024

Provides a lighter fill color for the button when in light mode and handles updating it dynamically as the appearance changes.

Examples:

Light

Simulator Screenshot - Test-iPhone - 2024-10-06 at 14 09 30

Dark

Simulator Screenshot - Test-iPhone - 2024-10-06 at 14 09 05

@krugerk
Copy link
Contributor Author

krugerk commented Oct 6, 2024

contains changes from PRs #487 and #486

@krugerk krugerk force-pushed the lighter-button-fill-in-light-mode branch from ef2356c to a693d19 Compare October 6, 2024 20:21
BeeKit/UI/BSButton.swift Outdated Show resolved Hide resolved
[UITraitUserInterfaceLevel.self]) {
(self: Self, previousTraitCollection: UITraitCollection) in
self.resetStyle()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

As this is duplicated in both init methods, perhaps it should instead live in setUp?

case .dark:
return UIColor.black
default:
return UIColor(red: 59.0/255.0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder what it would look like if we used the same grey as the +/- buttons. Possibly too low contrast with the yellow?

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 think you are right. The contrast seems to work for me on the sign in button but looks too low contrast elsewhere.

How about something like this - yellow on black in dark and black on grey in light mode?

yellowonblacktoblackongrey.mp4

Copy link
Collaborator

Choose a reason for hiding this comment

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

What if we skipped the border entirely and just had the solid button, either gray or yellow?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Grey (OK):
Screenshot 2024-10-06 at 2 49 29 PM

Yellow + Black Text (Probably too loud):
Screenshot 2024-10-06 at 2 50 42 PM

Yellow + White Text (Unreadable):
Screenshot 2024-10-06 at 2 51 14 PM

Copy link
Collaborator

Choose a reason for hiding this comment

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

And for completeness, default system blue:
Screenshot 2024-10-06 at 2 53 23 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the current target? Yellow text on Black background with yellow border in dark mode and black text on grey and without border in light mode?

Copy link
Collaborator

Choose a reason for hiding this comment

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

My take would be:

  • Yellow border and text, black background when shown in dark mode
  • Black on grey without border in light mode

But I don't feel like I have particularly good graphics design taste, so I'm happy for you to make a call on what you want to try, and we can put it out to test flight to get wider feedback.

@krugerk krugerk force-pushed the lighter-button-fill-in-light-mode branch from a693d19 to 77f78e3 Compare October 6, 2024 20:37
@theospears theospears merged commit c751942 into beeminder:master Oct 14, 2024
1 check passed
@krugerk krugerk deleted the lighter-button-fill-in-light-mode branch October 14, 2024 22:39
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