date picker wraps around to 1 for the first of the current month #496
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krugerk
commented
Oct 21, 2024
krugerk
commented
Oct 21, 2024
krugerk
commented
Oct 21, 2024
krugerk
force-pushed
the
bugfix/issue386-date-picker-wraps-around-to-1-for-the-first-of-the-current-month
branch
from
cbe6b00 to
a6e7e93
Compare
The Edit Datapoint screen contains a date picker. The Add Datapoint screen, the lower part of the goal screen, did not contain a date picker. Instead it contained a single digit to represent the date of the to-be-created datapoint and furthermore sent this single digit to the backend as part of `urtext`. The backend itself then interpreted the values sent and really had no way to accurately map the "day of the month" number to any particular year or month. Here the Add Datapoint section has its own date which is then sent to the backend when adding the new datapoint. Since the app now provides all of year, month, and day, there is less ambiguity with regards to the daystamp for the datapoint. The (date) value shown is either a simple single number (the day) when within the same month, the month and day when no longer in the same month but still the same year, and all three components when the point is for a different calendar year. Fixes beeminder#386
no longer unnecessarily wrapping conditionals or arguments in parentheses
krugerk
force-pushed
the
bugfix/issue386-date-picker-wraps-around-to-1-for-the-first-of-the-current-month
branch
from
04bd28c to
e3b6e51
Compare
theospears
approved these changes
Nov 15, 2024
| @@ -368,12 +371,14 @@ class GoalViewController: UIViewController, UIScrollViewDelegate, DatapointTabl | |||
| var components = DateComponents() | |||
| components.day = Int(self.dateStepper.value) | |||
|
|
|||
| let newDate = (calendar as NSCalendar?)?.date(byAdding: components, to: Date(), options: []) | |||
| let now = Date() | |||
| guard let newDate = calendar.date(byAdding: components, to: now) else { return } | |||
There was a problem hiding this comment.
Nit: This seems like the return here should never happen. I like logging a warning in these cases.
| @@ -368,12 +371,14 @@ class GoalViewController: UIViewController, UIScrollViewDelegate, DatapointTabl | |||
| var components = DateComponents() | |||
| components.day = Int(self.dateStepper.value) | |||
|
|
|||
| let newDate = (calendar as NSCalendar?)?.date(byAdding: components, to: Date(), options: []) | |||
| let now = Date() | |||
There was a problem hiding this comment.
This does surprising things if you have the view open and midnight rolls around. But this PR doesn't change that weird behavior, it was always there.
There was a problem hiding this comment.
True. The app will work with the data it has from the time it last fetched it. One can use other means to adjust the deadline and the one client might not know about it while it is at the timer screen, add data screen, or edit data screen. (See #26)
Is the server posting to the app via remote notification that a goal has been updated? Does the app still need to fetch (pull) to find out?
krugerk
deleted the
bugfix/issue386-date-picker-wraps-around-to-1-for-the-first-of-the-current-month
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Edit Datapoint screen contains a date picker. The Add Datapoint screen, the lower part of the goal screen, did not contain a date picker. Instead it contained a single digit to represent the date of the to-be-created datapoint and furthermore sent this single digit to the backend as part of
urtext. The backend itself then interpreted the values sent and really had no way to accurately map the "day of the month" number to any particular year or month.With this merge request, the Add Datapoint section has been given its own date which is then sent to the backend when adding the new datapoint. Since the app now provides all of year, month, and day, there is less ambiguity related to the daystamp for the datapoint.
Furthermore, similar to how the Android app / Beedroid handles this, the format shown varies: the (date) value shown is either a simple single number (the day) when within the same month, the month and day when no longer in the same month but still the same year, and all three components when the point is for a different calendar year.
example, stepping through dates on 2024-Oct-21:
stepping.through.dates.mp4
Fixes #386