FEATURE: Enable Skipping Current User #220
Conversation
| @@ -605,6 +605,45 @@ export const StartTimer = (state, { timerStartedAt, timerDuration }) => [ | |||
| }), | |||
| ]; | |||
|
|
|||
| export const Skip = (state, { timerDuration, timerStartedAt = null }) => { | |||
| console.log("action - Next"); | |||
There was a problem hiding this comment.
Let's remove this console.log
There was a problem hiding this comment.
Thanks for catching. Left over when I was trying to understand a few things about the system.
| action: CycleMob, // eslint-disable-line no-use-before-define | ||
| props: { }, | ||
| }), | ||
| effects.andThen({ |
There was a problem hiding this comment.
For now, I think we'll remove StartTimer (and the timerStartedAt stuff). I'm willing to be wrong and have a larger discussion on it, though.
There was a problem hiding this comment.
I'm good with that, however that leaves a few other questions to be answered
First, when Skip (or whatever we call it) is clicked does it only cycle or does it end the turn and cycle?
Second, if we land on the semantics that skip will only cycle the mob, and possibly end the existing session, if one is active, does the skip button appear all of the time? This would result in being able to cycle people before the turn.
Finally, if the decision is to leave the button all of the time, is there a better location for the button? Two thoughts I had:
- Place the button else where
- Place skip to left of Start, this way start/pause interchange with one another and if someone double clicks it only pauses and doesn't accidentally skip. I was thinking we might wish to make a similar change either way.
There was a problem hiding this comment.
Functionally, I think we can always stop the timer. If there is no timer running, then that should essentially be a no-op, so there shouldn't be any harm.
In terms of placement, I'm not picky. Move it to a few sane places, and take screenshots and let's take a peek 👀
|
Since this has been open for a number of months and no movement, I'm going to close, but please feel free to re-open. |
PR for issue #218