-
-
Notifications
You must be signed in to change notification settings - Fork 131
started working on porting new Activity view to Android #183
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
Conversation
8c9f0c5 to
91353a4
Compare
91353a4 to
13777cb
Compare
|
@johan-bjareholt There's still some work left to do, but first I'd like to discuss my approach to dealing with the overlap between Android and desktop stuff. It isn't perfect, but it works and I'm not sure what would be the best way to do it. |
src/store/modules/activity.ts
Outdated
| state.buckets.browser_buckets.length > 0; | ||
| const active_available = state.buckets.afk_buckets.length > 0; | ||
| const editor_available = state.buckets.editor_buckets.length > 0; | ||
| const android_available = state.buckets.window_buckets[0].includes('android'); |
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.
Oh, the android bucket just have a normal bucket type?
That certainly makes things confusing.
Let's just hope that no one has 'android' in their hostname lol.
Can't you have something like ".startsWith('aw-watcher-android')" or something instead?
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.
Also, I think you need to check that "window_buckets" has a length of 1 or more before doing that check to avoid errors on desktop.
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.
Wouldn't it be easier if you in the "buckets" vuex instead had a "state.buckets.android_buckets" instead of here?
You could then also exclude the android buckets from the usual window buckets.
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.
Oh, the android bucket just have a normal bucket type?
Yeah, bad initial choice on my part. I expect us to change this eventually in some Great Migration (as with currentwindow, afkstatus).
Let's just hope that no one has 'android' in their hostname lol.
Can't you have something like ".startsWith('aw-watcher-android')" or something instead?
Fixed.
Also, I think you need to check that "window_buckets" has a length of 1 or more before doing that check to avoid errors on desktop.
Fixed.
Wouldn't it be easier if you in the "buckets" vuex instead had a "state.buckets.android_buckets" instead of here?
You could then also exclude the android buckets from the usual window buckets.
Fixed.
|
@johan-bjareholt Also note the fix in 035d4cc, looks like you broke it before. |
|
@johan-bjareholt I think this should be merged now. There's still stuff that could be nicer but it works and I won't have more time to work on this anytime soon. |
johan-bjareholt
left a comment
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, the rough stuff since last time has been fixed so I think it's fine!
| }, | ||
| top_categories: function() { | ||
| return this.$store.state.activity.category.top; | ||
| visualizations: function() { |
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.
Nice!
| window_buckets: [], | ||
| editor_buckets: [], | ||
| browser_buckets: [], | ||
| afk: [], |
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.
Nice!
No description provided.