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

Improve timeline usability (less clicks to apply time filters) #610

Merged
merged 9 commits into from
Oct 5, 2024

Conversation

ideadapt
Copy link
Contributor

@ideadapt ideadapt commented Sep 8, 2024

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:

  • Remove "mode" select. Display both filter directly.
  • Use a list of buttons instead of the dropdown to select "last n seconds" (last_duration).
  • Apply selected last_duration on button click. Still require explicit button click for time range filter to apply.
  • Notify user if no events match (so the user is not irritated, why the timeline does not re-render, addresses an issue mentioned in Timeline - Date range - "Update" button issues #395 )
  • Apply bootstrap styles. Small layout / design adjustments to align stuff a little.

Fixes #395

Before:
before

After:
after


🚀 This description was created by Ellipsis for commit d15b9ef

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.
  • Replaced dropdown with button list for "last n seconds" selection.
  • Immediate application of last_duration on button click.
  • Explicit button click still required for date range filter.
  • Added user notification for no matching events.
  • Applied Bootstrap styles for layout adjustments.
  • src/views/Timeline.vue: Adjusted event count display and added alert for no events.

Generated with ❤️ by ellipsis.dev

Copy link

@ellipsis-dev ellipsis-dev bot left a 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 in 2 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 of sr-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:
    The mode property is set to 'range' in applyRange, but mode 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%
    The applyRange method sets this.mode to 'range' and this.duration to 0, which is logical since a range doesn't need a duration. However, the mode property is not used elsewhere in the code, so setting it might be unnecessary.
3. src/components/InputTimeInterval.vue:141
  • Draft comment:
    The mode property is set to 'last_duration' in applyLastDuration, but mode 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%
    The applyLastDuration method sets this.mode to 'last_duration', but mode 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 between num_events and vis-timeline.chartData. Ensure consistency in the displayed data.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The num_events computed property has a TODO comment indicating a mismatch with vis-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.

Copy link
Member

@ErikBjare ErikBjare left a 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.

src/components/InputTimeInterval.vue Outdated Show resolved Hide resolved
src/components/InputTimeInterval.vue Outdated Show resolved Hide resolved
src/components/InputTimeInterval.vue Outdated Show resolved Hide resolved
src/components/InputTimeInterval.vue Outdated Show resolved Hide resolved
src/components/InputTimeInterval.vue Show resolved Hide resolved
Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.98%. Comparing base (0cf7831) to head (c41e6e1).
Report is 2 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

@ideadapt
Copy link
Contributor Author

ideadapt commented Sep 9, 2024

Great changes!

My main concern i s 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.

I wasn't aware of the mobile app requirement. I optimized it for screens >=640px. My tests today showed that it would work down to 540px:

new-ui-mobile

Testing the old UI on mobile showed that, at least for my computer host names, the timeline is not useful on mobile screens. The bucket events start to get visible with >450px:

old-ui-mobile

Are there requirements to support screens smaller than 540px? If so, I could try to shrink the duration selection on small screens.

@ErikBjare
Copy link
Member

ErikBjare commented Sep 9, 2024

@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 aw-android-test and it takes up ~30% of the width on my phone. Not great, but useable.

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

src/views/Timeline.vue Outdated Show resolved Hide resolved
@ErikBjare
Copy link
Member

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!

@ErikBjare ErikBjare changed the title Improve Timeinterval usability (less clicks to apply time filters) Improve timeline usability (less clicks to apply time filters) Oct 5, 2024
@ErikBjare ErikBjare merged commit 291da6f into ActivityWatch:master Oct 5, 2024
8 checks passed
@ErikBjare
Copy link
Member

Merged! Thank you again for this great work!

@ideadapt
Copy link
Contributor Author

ideadapt commented Oct 5, 2024

Merged! Thank you again for this great work!

Cool! Thank you :)

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

Successfully merging this pull request may close these issues.

Timeline - Date range - "Update" button issues
2 participants