Skip to content

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.

Sign up for GitHub

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

Closed
DataScienceDIY wants to merge 2 commits into from
Closed

Enhancement: Improve default time suggestions for newly created items #588

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a98ef01
Enhancement: Improve default time suggestions for newly created items…
Jan 8, 2026
46baff7
Fix: Bug where changing time2 would cause time1 to be changed even mi…
Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions timetagger/app/dialogs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,20 @@ def _on_mode_change(self):
if self.radio_startrlr.checked:
self.reset(t1, t1)
else:
if self.radio_finished.checked: # If the user just checked the finished radio butotn
range_t1, range_t2 = window.canvas.range.get_range()
now = dt.now()
# 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
Comment on lines +843 to +848
Copy link
Copy Markdown
Owner

@almarklein almarklein Jan 29, 2026

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().

Copy link
Copy Markdown
Contributor Author

@DataScienceDIY DataScienceDIY Jan 29, 2026

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
Comment on lines +849 to +853
Copy link
Copy Markdown
Owner

@almarklein almarklein Jan 29, 2026

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 😉

Copy link
Copy Markdown
Contributor Author

@DataScienceDIY DataScienceDIY Jan 29, 2026

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.

Copy link
Copy Markdown
Owner

@almarklein almarklein Feb 2, 2026

Choose a reason for hiding this comment

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

Right. I'm happy to add this improvement, if it's more or less in this shape:

    if self.radio_startrlr.checked:  # current code
        self.reset(t1, t1)
    if self.radio_finished.checked:  # new branch
        range_t1, range_t2 = window.canvas.range.get_range() 
        if not (range_t1 <= now <= range_t2):
            ... select one hour in the middle of the range, these numbers are expressed in seconds, so is fairly easy

self.reset(t1, t2)
else:
# Switch between "already running" and "finished".
Expand Down Expand Up @@ -1072,10 +1086,11 @@ def onchanged(self, action):
mm, ss = mm + self._stepwise_delta(mm, -(_stepsize)), 0
d1 = window.Date(year1, month1 - 1, day1, hh, mm, ss)
self.t1 = dt.to_time_int(d1)
if self.ori_t1 == self.ori_t2:
self.t2 = self.t1 = min(self.t1, now)
elif self.t1 >= self.t2:
self.t2 = self.t1 + 1

if self.ori_t1 == self.ori_t2:
self.t2 = self.t1 = min(self.t1, now)
elif self.t1 >= self.t2:
self.t2 = self.t1 + 1

elif what == "time2":
# Changing time2 -> update t2, keep t1 and t2 in check
Expand All @@ -1094,11 +1109,12 @@ def onchanged(self, action):
mm, ss = mm + self._stepwise_delta(mm, -(_stepsize)), 0
d2 = window.Date(year2, month2 - 1, day2, hh, mm, ss)
self.t2 = dt.to_time_int(d2)
if self.ori_t1 == self.ori_t2:
self.t2 = self.t1
elif self.t2 <= self.t1:
self.t1 = self.t2
self.t2 = self.t1 + 1

if self.ori_t1 == self.ori_t2:
self.t2 = self.t1
elif self.t2 <= self.t1:
self.t1 = self.t2
self.t2 = self.t1 + 1

elif what == "duration":
# Changing duration -> update t2, but keep it in check
Expand Down
Loading