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

Apply day offsets more consistently in more places #526

Open
duxovni opened this issue Jan 18, 2025 · 2 comments
Open

Apply day offsets more consistently in more places #526

duxovni opened this issue Jan 18, 2025 · 2 comments

Comments

@duxovni
Copy link
Contributor

duxovni commented Jan 18, 2025

  1. It'd be nice to have the "today" and "yesterday" buttons in the time selection dialog take the day offset into account. The "today" button could work the same way as the standalone "today" nav button that's only displayed on wide enough screens – it sets your time range to the current offset day, unless the range is already the current offset day, in which case it sets it to the current non-offset day.

  2. It'd also be nice to have TimeTagger automatically set the time range to the current offset day when you open it, rather than the current non-offset day. That's a bit more of a pain because the UI gets set up before the day offset settings are retrieved from the server, though. The best idea I've got is: when we retrieve updated settings from the server, if the day offset settings have changed from what we had previously, and if we're viewing the current non-offset day, adjust the time range to the current offset day using the updated settings. (Alternatively, I guess just caching the offset settings more persistently would be an option?)

I have patches that accomplish both of these things, although patch number 1 for the time selection dialog doesn't implement the back-and-forth offset/non-offset toggle for the "yesterday" button, and patch number 2 for the time range initialization is pretty janky – stores.py was the easiest place for me to implement it, but it doesn't really belong in there.

I can create actual PRs for these, I just wanted to discuss it first to see whether there's a better way of doing this (and whether this is actually functionality you'd like to have in the first place.)

Patch 1: Time selection dialog
diff --git a/timetagger/app/dialogs.py b/timetagger/app/dialogs.py
index 116e024..1f22b01 100644
--- a/timetagger/app/dialogs.py
+++ b/timetagger/app/dialogs.py
@@ -618,15 +618,23 @@ class TimeSelectionDialog(BaseDialog):
         else:
             return
 
-        t1 = dt.floor(dt.now(), rounder)
-        if last:
-            t1 = dt.add(t1, "-" + rounder)
-        t2 = dt.add(t1, rounder)
-        t2 = dt.add(t2, "-1D")  # range is inclusive
+        if rounder == "1D":
+            t1, t2 = window.canvas.range.get_today_range()
+            if last:
+                t1 = dt.add(t1, "-1D")
+                t2 = dt.add(t2, "-1D")
+            window.canvas.range.animate_range(t1, t2, None, False)  # without snap
+        else:
+            t1 = dt.floor(dt.now(), rounder)
+            if last:
+                t1 = dt.add(t1, "-" + rounder)
+            t2 = dt.add(t1, rounder)
+            t2 = dt.add(t2, "-1D")  # range is inclusive
+
+            self._t1_input.value = dt.time2localstr(t1).split(" ")[0]
+            self._t2_input.value = dt.time2localstr(t2).split(" ")[0]
+            self._update_range()
 
-        self._t1_input.value = dt.time2localstr(t1).split(" ")[0]
-        self._t2_input.value = dt.time2localstr(t2).split(" ")[0]
-        self._update_range()
         self.close()
 
     def _update_range(self):
Patch 2: Time range initialization
diff --git a/timetagger/app/front.py b/timetagger/app/front.py
index fc35b09..0715cf8 100644
--- a/timetagger/app/front.py
+++ b/timetagger/app/front.py
@@ -395,10 +395,11 @@ class TimeRange:
         # The animate variable is normally None. During animation, it is a tuple
         self._animate = None
 
-        # Init time to the current full day
+        # Initialize self._t1 and self._t2 so get_today_range() will work
         self._t1 = dt.floor(self._canvas.now(), "1D")
         self._t2 = dt.add(self._t1, "1D")
-        self._t1, self._t2 = self.get_snap_range()  # snap non-animated
+        # Init time to the current offset day
+        self._t1, self._t2 = self.get_today_range()
 
     def get_today_range(self):
         """Get the sensible range for "today"."""
diff --git a/timetagger/app/stores.py b/timetagger/app/stores.py
index cbf2c02..5199dc6 100644
--- a/timetagger/app/stores.py
+++ b/timetagger/app/stores.py
@@ -857,7 +857,17 @@ class ConnectedDataStore(BaseDataStore):
                 console.warn(err)
         # Sync the settings API
         if this_is_js():
+            old_snap_offset = window.simplesettings.get("today_snap_offset")
+            old_end_offset = window.simplesettings.get("today_end_offset")
             window.simplesettings.update_store(self.settings)
+            new_snap_offset = window.simplesettings.get("today_snap_offset")
+            new_end_offset = window.simplesettings.get("today_end_offset")
+            if new_snap_offset != old_snap_offset or new_end_offset != old_end_offset:
+                if window.canvas:
+                    current_t1, current_t2 = window.canvas.range.get_target_range()
+                    if current_t1 == dt.floor(dt.now(), "1D") and current_t2 == dt.add(current_t1, "1D"):
+                        t1, t2 = window.canvas.range.get_today_range()
+                        window.canvas.range.animate_range(t1, t2)
 
     async def _sync(self):
         # Ignore sync if there is no auth info
@almarklein
Copy link
Owner

Yeah open to a pr for both cases, preferably separate ones.

I think the first is relatively straight forward. For the second, I like the idea of caching the settings, or at least the offset? So that it can be used at starup to set the initial time range.

@duxovni
Copy link
Contributor Author

duxovni commented Jan 27, 2025

Sounds good! I'm pretty busy this week, but I can try and submit PRs when I've got more spare time.

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

No branches or pull requests

2 participants