fix: fix timeline updating before new range is selected (#449 from ac… #219
Conversation
…h#449 from activitywatch)
There was a problem hiding this comment.
Nice work! 💪
I think shying away from v-model might be the wrong call though, I suggest reading up on it (https://vuejs.org/v2/guide/forms.html and https://vuejs.org/v2/guide/components.html#Using-v-model-on-Components) and addressing my comments.
I realize now how this code was far from perfect to begin with (sorry about that, lol), so I took the liberty of adding some comments on the original code as well to highlight issues with it. Hopefully they are helpful to you, just don't get put off by me spamming comments on my own shitty code 🙂.
|
Oh, and it looks like GitHub Actions picked up some minor formatting issues. You can easily fix them with |
|
Thank you for the detailed comments 👍, it really helps especially because I'm new to Vue! Yeah I think it would be much easier if I kept the v-model and started from scratch. |
|
@billangli Starting from scratch sounds like overkill, just fix the things and then clean it up with rebase after :) |
|
@billangli Oh, and if you're interested in getting paid for your contributions, let me know. See ActivityWatch/activitywatch#458 for more info. Edit: I also saw you'd been involved with some BCI stuff at UofT. I'd love to chat with you about that if you have time, since I'm doing my MSc thesis on it (see ActivityWatch/activitywatch#422) |
|
Haha thanks for the offer! I don't think I've done enough yet so I'll pass on that. I just thought of this as a good opportunity to learn some full-stack development. You probably saw my Bella repo, it was a silly hackathon project but I'd be happy to tell you about the idea. I also have a friend who is working on a research project that analyses speech patterns for diseases. It's signal processing so you might find it useful, let me see if he's available too. Edit: He's available! |
Small world. I've worked for a startup that does the same thing 🙂 Not my focus right now though so I'll pass on the intro for now, but thank you for the offer! |
|
@ErikBjare no worries! I totally get it, my friend mentioned to me that your areas are not as related as I thought haha. |
…h#449 from activitywatch)
…gli/aw-webui into dev/timeline-input-button
|
I made the changes that you suggested. I also changed the warnings to be more consistent with the rest of the UI. Let me know what you think.
|
src/views/Timeline.vue
Outdated
| @@ -40,7 +40,8 @@ export default { | |||
| }, | |||
| }, | |||
| mounted: function () { | |||
| this.getBuckets(); | |||
| (this.daterange = [moment().subtract(this.timeintervalDefaultDuration, 'seconds'), moment()]), | |||
There was a problem hiding this comment.
Looks like the parentheses here are unnecessary. Might be that prettier found a , that should have been a ; and decided it needed wrapping.
There was a problem hiding this comment.
Edit: Ignore this particular subcomment, just thinking out loud and I think the way you did it makes sense.
Also, can't you move this initialization of daterange up into data? Looks like you should have everything you need there.
Or maybe that causes the "now" moment() to become stale? I'm a bit fuzzy on when data is actually run and it was hard to search for (surprisingly). Perhaps a comment for why it's there (// Initialized here to make sure moment() doesn't get stale in data) might suffice.
There was a problem hiding this comment.
If you set this.daterange which has a watch, shouldn't this.getBuckets() (which also doesn't take any arguments) run automatically?
There was a problem hiding this comment.
@ErikBjare I think I might have just forgot to check this and I wasn't being careful enough with the rebase, my bad. My intention was to look out for the case where timeintervalDefaultDuration is null because localStorage.durationDefault is null. I now removed this mounted function in Timeline.vue and made similar changes in Bucket.vue. The initial request will be called by valueChanged in a mounted function in InputTimelineInterval.vue.
|
I just realized that Bucket Views (Raw Data->Open(any bucket)) has the same view and therefore the same problem. Are the same components used? If yes, then i think your PR will automatically solve the issue there too |
|
@nicolae-stroncea yep, Bucket view also uses the same component. |
Hello! This is my first pull request, please feel free to let me know if you have feedback. I found this issue on the activitywatch repo (ActivityWatch/activitywatch#449) and I thought that the pull request should go here.
Before My Changes
Timeline View
Bucket View
My Changes
I added a "Show Timeline" button that will send a request to the server if pressed. The default timeline range will still be showed when the page loads in the beginning. I also made the select boxes and the button horizontal because I thought that was better use of space.
In Range mode, the "Show Timeline" button will be disabled (grey) until the user has selected both dates.
Red text telling the user that the date range is invalid will also show if the first date is less than or equal to the second, the button is also greyed out.
The following two images are the new Bucket view.