-
-
Notifications
You must be signed in to change notification settings - Fork 153
Enhancement: Improve default time suggestions for newly created items #588
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
base: main
Are you sure you want to change the base?
Enhancement: Improve default time suggestions for newly created items #588
Conversation
… marked already finished.
…d-change or during submission.
| # If the current time is on the currently viewed day, assume that | ||
| # the user forgot to hit "start now" and use the current time as | ||
| # the default end of the event, and -1 hour as the default start time. | ||
| if range_t1 <= now <= range_t2: | ||
| t1 = now - 3600 | ||
| t2 = now |
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 don't understand how this improved things, to be honest. Above, t1 is already set using _get_sensible_start_time().
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.
Within _get_sensible_start_time would be a better place to hold this logic.
I think the intent is clear in the comments. If you think this functionality is something you would like to include then I can update the code to integrate it better into _get_sensible_start_time(). If not, then you can just reject the code and you won't hurt my feelings 😄
| else: # Otherwise assume the user is filling in a historical event on the viewed day | ||
| t1, t2 = window.canvas.range.get_range() | ||
| # If t2 - t1 is exactly 1 day, subtract 5 minutes from t2 to make it the same day as t1 | ||
| if t2 - t1 == 86400: | ||
| t2 = t2 - 300 |
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.
This means that of the currently viewed range is one week, the user gets a record spanning that whole week 😉
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.
That is exactly right. I frequently find myself missing entries from my timesheets when I review them at the end of the month either because I was away from my desk at the time, or for some other reason.
In this situation I go back and view the timesheets and if I want to add a record I always want to add it within the time range I am viewing. While this default does set an unreasonable amount of time for the event, it has the advantage of having the correct year, month, (probably also day) already selected and makes entering a historical event easier. It might be even better to calculate the center time within view and make the event one hour long centered within the time range, but even this would undoubtedly need to be changed, so it really provides no advantage in terms of data entry speed.
This is a very small change which improves the default time suggestions for newly created items (Using the "Record" button), which are then marked "Already Done".
In the case that the current system time is on the currently viewed day in the timeline, we assume the user forgot to hit "Start Now" on a recent event and the default start time is set to one hour prior to now and the default end time is set to now.
In the case that the currently viewed timeline is in the past, we assume the user is filling in an already finished past record and we use the currently viewed time range as the start and end. If this time range is exactly one day, we subtract 5 minutes from the end time so the end falls on the same day as the start.