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

Bugfix 477 timer shows goal name #481

Merged

Conversation

krugerk
Copy link
Contributor

@krugerk krugerk commented Sep 27, 2024

fixes #477 by displaying the name ("slug") of the goal on the timer view controller

screenshot:

@theospears
Copy link
Collaborator

Thanks for making this fix. I also really appreciate you including a screenshot in the comment.

I have a few requests before merging:

  1. This looks like it is 10% the feature change, and 90% a clean up. I'm on board with the cleanup, but could you put it in its own separate PR?
  2. The change to button text really exacerbates the issue of buttons on this screen not looking like buttons. I've been trying to slowly move BeeSwift towards matching standard apple UI patterns. How would you feel about updating the button styles to be standard iOS buttons?
  3. Please fix the test failures

@krugerk krugerk force-pushed the bugfix/477-timer-shows-goal-name branch from 23a4262 to 4ca0a75 Compare September 27, 2024 20:15
@krugerk
Copy link
Contributor Author

krugerk commented Sep 27, 2024

  1. This looks like it is 10% the feature change, and 90% a clean up. I'm on board with the cleanup, but could you put it in its own separate PR?

Thanks for the feedback! I can definitely split the cleanup into its own PR to keep things organized. I will get that sorted out and let you know once it is ready.

  1. The change to button text really exacerbates the issue of buttons on this screen not looking like buttons. I've been trying to slowly move BeeSwift towards matching standard apple UI patterns. How would you feel about updating the button styles to be standard iOS buttons?

Thanks for pointing that out! I understand the importance of consistency with iOS UI patterns. I am on board with updating the button styles to align better with standard iOS buttons. I have made corresponding changes and have updated the branch and thus (currently) this PR.
At first I just adjusted them to look more like the one you had introduced in the edit data point screen:

And then used some inspiration from the bmndr website for the buttons - yellow on black instead of white on blue:

and

  1. Please fix the test failures

None of the tests ran as the targeted device was not found. I bumped to macos15 and tweaked the commands in fastlane, which now build an iPhone 16 Pro Simulator with iOS 18.0 for the tests.

@krugerk krugerk force-pushed the bugfix/477-timer-shows-goal-name branch from aa01de8 to e9dbd00 Compare September 30, 2024 06:28
@krugerk
Copy link
Contributor Author

krugerk commented Sep 30, 2024

@theospears the Merge Request has been split and the buttons adjusted.

@theospears theospears merged commit 4c4beb7 into beeminder:master Oct 3, 2024
1 check passed
theospears pushed a commit that referenced this pull request Oct 4, 2024
Use a more consistent iOS styling for in app buttons.

Example:
<img width=400
src=https://github.com/user-attachments/assets/5e7c57e2-dcaf-46e1-92b4-fed9f3759370
/>


See
#481 (comment)
theospears pushed a commit that referenced this pull request Oct 5, 2024
clean code

align some inconsistencies with providing a goal
to some view controllers that required one

See
#481 (comment)
@dreeves
Copy link
Member

dreeves commented Oct 6, 2024

@krugerk those yellow-on-black buttons look very nice. thank you!

@krugerk krugerk deleted the bugfix/477-timer-shows-goal-name branch October 9, 2024 08:09
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.

Display goalname on timer screen
3 participants