Start stop button#493
Conversation
carmichaelong
left a comment
carmichaelong
left a comment
There was a problem hiding this comment.
@AlbertoCasasOrtiz I left a couple comments to respond to or address. Thanks!
| const res = await axios.get(`/sessions/${this.session.id}/cancel_trial/`, {}) | ||
| this.cancelPoll() | ||
| this.state = 'ready' | ||
| // End cooldown after processing cancellation |
There was a problem hiding this comment.
This comment doesn't align with the code (which is already in a helper function). Perhaps you meant to add catching an error and resetting the cooldown to false like above?
There was a problem hiding this comment.
I think I meant start cooldown after procesing cancellation
There was a problem hiding this comment.
Please let me know if I'm missing something here. Seems like it will have a cooldown already from L1423 since the state changed (consistent with the other cases).
Is there a reason why an additional delay should be set after a trial is cancelled? And in this case I think this timer would run not add to the timer started on L1423 (i.e., this would be a second timer that ends at a different time?). I guess this since this second one could end earlier, it would release the button a second earlier?
There was a problem hiding this comment.
I know this isn't a part of your changes, but given the cooldown on the button, is this line needed anymore? (I don't think this line technically blocked a user from hitting the button twice quickly too??)
carmichaelong
left a comment
carmichaelong
left a comment
There was a problem hiding this comment.
@AlbertoCasasOrtiz I had issues with building this branch:
ERROR Failed to compile with 1 error 9:03:16 PM
error in ./src/components/pages/Session.vue
Module Error (from ./node_modules/eslint-loader/index.js):
/repos/opencap-viewer/src/components/pages/Session.vue
69:95 error 'invalid' is defined but never used vue/no-unused-vars
| this.buttonCooldown = true; | ||
| setTimeout(() => { | ||
| this.buttonCooldown = false; | ||
| }, 2000); // 2 seconds |
There was a problem hiding this comment.
Haven't been able to test until it builds, but I was curious about the 1 second delay vs 2 second delay. I'd lean towards 1 second if we think it would be long enough to prevent misclicks (I'd prefer as short as we can go).
Perhaps you used 2 seconds because of the case where the timer runs out and turns into cancel? If so, maybe it should actually be variable:
- start/stop could be faster (<= 1 second)
- cancel could be slower (maybe like >= 1.5 seconds?)
2 participants
Added cooldown of 1 second when the start/stop/cancel button changes status, to avoid double clicks.