-
-
Notifications
You must be signed in to change notification settings - Fork 944
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
Added 12 hour set time interface #907
Conversation
Removed redundant if statements in hour button logic. Spacing is now in line with repo guidelines.
Also corrected orphaned bracket
Does this also fix the alarm app time setting? |
Reopening pull request because it was closed due to branch issues |
This PR only changes the set time interface. I am currently looking into the alarm app too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review! I will get started on addressing the issues. |
Consolidated 24 hour to 12 hour time conversion logic into function, addressed formatting issues, cleaned up code.
Consolidated 12 hour label changes to function. Removed use of strings, struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a couple of comments, but it might not require changes.
Changed setHourLabels function to handle 24 hour time labeling, changed to private
Corrected formatting and removed unnecessary variable time24H
Thanks for the PR and for the review, @eliwss0 & @Riksu9000 ! |
Added appropriate set time interface for 12 hour setting. Time is still kept and set as 24 hours, but interface changes to reflect 12 hour time if user chooses 12 hour format.