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

feat: started working on customizable views #233

Merged
merged 11 commits into from Nov 2, 2020
Merged

Conversation

ErikBjare
Copy link
Member

@ErikBjare ErikBjare commented Oct 30, 2020

Was surprisingly easy to get the basics working.

TODO

  • Add view
  • Remove view
  • Add visualization to view
  • Remove visualization from view
  • Migrate editor view
  • Change visualization (to work as before)
  • Save views
  • Restore default views
  • Set/rename view name
    • Done by adding a "new view" modal, created views still cannot be renamed
  • Add notice for visualizations that are missing prerequisite data

Extras (can wait for another PR)

  • Reorder visualizations in view
  • Add ability to set options for individual visualizations
    • Such as width/size, or visualization-specific options.

Screenshot

image

@ErikBjare
Copy link
Member Author

Added the ability to remove visualizations, and an 'edit view' button:

image

image

@ErikBjare ErikBjare marked this pull request as ready for review November 1, 2020 12:48
@ErikBjare
Copy link
Member Author

@johan-bjareholt Ready for review 🎉

Copy link
Member

@johan-bjareholt johan-bjareholt left a comment

Choose a reason for hiding this comment

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

I get this error message on two occasions:

  • When opening a custom view
  • When looking at a custom view and pressing the "new view" button
Invalid prop: type check failed for prop "view_id". Expected String with value "5", got Number with value 5.

import _ from 'lodash';
import moment from 'moment';

// TODO: Move somewhere else, possibly turn into a serverside transform
Copy link
Member

Choose a reason for hiding this comment

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

First I thought "Would be much easier to do with a query, this is overcomplicated".

Then I realized that this also creates an average usage over the period of a week/month, that would not be easy to do in a query as it's designed right now, that's pretty clever and very useful data!

Copy link
Member Author

@ErikBjare ErikBjare Nov 2, 2020

Choose a reason for hiding this comment

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

Wow. I did not have that functionality in mind when I wrote it, but you're right!

Looks like it's a bit broken for those views though, doesn't seem to add up the hours?

aw-summary(:fields="top_titles", :namefunc="e => e.data.title", :colorfunc="e => e.data.app", with_limit)

div(v-if="periodLength == 'day'")
aw-sunburst-clock(:date="date", :afkBucketId="bucket_id_afk", :windowBucketId="bucket_id_window")
Copy link
Member

Choose a reason for hiding this comment

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

There is now no way to use this visualization.
Personally I never use it, but it's worth mentioning.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a commented-out version to SelectableVisualization. We'll have to fix it later if there's interest.

h6 No browser buckets available
small
| This feature requires a browser watcher.
| You can find a list of all watchers in #[a(href="https://activitywatch.readthedocs.io/en/latest/watchers.html") the documentation].
Copy link
Member

Choose a reason for hiding this comment

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

It's a shame that this kind of notice is not there anymore.

Copy link
Member Author

@ErikBjare ErikBjare Nov 2, 2020

Choose a reason for hiding this comment

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

Yeah, I thought a bit about this since the same thing applies for the editor view.

Basically it would be nice if we'd have a way to indicate why a visualization cannot be selected in the drop-down, or alternatively, always let users select visualizations but then show a notice per-visualization if the requisite data is missing. (might be a somewhat easy edit to SelectableVisualization.vue)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

this.$store.dispatch('views/load');
},
remove() {
this.$store.commit('views/removeView', { view_id: this.view.id });
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should gracefully fail if there is only a single view left and you try to delete it?

Copy link
Member Author

@ErikBjare ErikBjare Nov 2, 2020

Choose a reason for hiding this comment

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

I think the current behavior is graceful enough: Removing the last view leads to zero views, and also no save button to persist the empty state. The 'New view' button still works.

src/views/activity/Activity.vue Outdated Show resolved Hide resolved
@ErikBjare
Copy link
Member Author

@johan-bjareholt I fixed the issues you mentioned in your review. I'll go ahead and merge this and will fix eventual issues in separate PRs.

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.

None yet

2 participants