-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
Improve timeline usability (less clicks to apply time filters) #610
Conversation
…because there are no matching events to render). addresses issues reported in ActivityWatch#395
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.
👍 Looks good to me! Reviewed everything up to d15b9ef in 11 seconds
More details
- Looked at
183
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. src/components/InputTimeInterval.vue:15
- Draft comment:
Consider removing the.sr-only
class from the radio inputs as the labels are already visible and clickable. This might improve accessibility by ensuring screen readers can correctly identify the radio inputs. - Reason this comment was not posted:
Confidence changes required:50%
The change from a dropdown to radio buttons for selecting the duration is a good usability improvement. However, the use ofsr-only
class for the radio inputs might not be necessary since the labels are visible and clickable. This could be a potential accessibility issue if the screen readers do not pick up the radio inputs correctly.
2. src/components/InputTimeInterval.vue:136
- Draft comment:
Themode
property is set to 'range' inapplyRange
, butmode
is not used elsewhere in the code. Consider removing it if it's not needed. - Reason this comment was not posted:
Confidence changes required:50%
TheapplyRange
method setsthis.mode
to 'range' andthis.duration
to 0, which is logical since a range doesn't need a duration. However, themode
property is not used elsewhere in the code, so setting it might be unnecessary.
3. src/components/InputTimeInterval.vue:141
- Draft comment:
Themode
property is set to 'last_duration' inapplyLastDuration
, butmode
is not used elsewhere in the code. Consider removing it if it's not needed. - Reason this comment was not posted:
Confidence changes required:50%
TheapplyLastDuration
method setsthis.mode
to 'last_duration', butmode
is not used elsewhere in the code. This might be unnecessary.
4. src/views/Timeline.vue:70
- Draft comment:
Address the TODO comment regarding the mismatch betweennum_events
andvis-timeline.chartData
. Ensure consistency in the displayed data. - Reason this comment was not posted:
Confidence changes required:50%
Thenum_events
computed property has a TODO comment indicating a mismatch withvis-timeline.chartData
. This should be addressed to ensure consistency in the displayed data.
Workflow ID: wflow_ktBxwZocQqRQmRSp
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
Great changes!
My main concern is how does this look on small screens now? Some of the things in the design that you rightfully questioned (the flex and right-float) were actually done they were for small screens, so they look good in the aw-android app.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #610 +/- ##
=======================================
Coverage 25.98% 25.98%
=======================================
Files 27 27
Lines 1643 1643
Branches 292 292
=======================================
Hits 427 427
- Misses 1157 1190 +33
+ Partials 59 26 -33 ☔ View full report in Codecov by Sentry. |
@ideadapt Yes, the same UI is used for aw-android which runs on phones. The bucket id is a lot shorter on Android, mine is We used to support landscape mode, so one could make it work pretty well if one wanted, but there was an issue with full reloads on orientation changes, so we disabled it "temporarily" and thats where its still at: ActivityWatch/aw-android#108 So yeah, still need to support/think about small screens, but I'm okay with things not being perfect (since they're not anyway with the tight timeline). |
…el it "reload" instead of former "update".
Again, very nice work! I think I'll take it from here :) Will be really great to have both this and #609 in the next release! |
Merged! Thank you again for this great work! |
Cool! Thank you :) |
I use the Timeline view at least once a day.
The number of clicks required to filter the time range started to bug me, so I tried to improve it:
Fixes #395
Before:
After:
Summary:
Improves Timeline view usability by reducing clicks for time filters, replacing dropdowns with buttons, and adding user notifications for no events.
Key points:
src/components/InputTimeInterval.vue
: Removed "mode" select, displaying both filters directly.last_duration
on button click.src/views/Timeline.vue
: Adjusted event count display and added alert for no events.Generated with ❤️ by ellipsis.dev