issue #195 More control over the timer

[dvberkel]

The pull request is re-implements the timer functionality. The following features are supported.

• [x] Ability to start it manually, so that staying 5 minutes on the first slide counts. Right now one has to go super quickly to the second slide and back to the first one to emulate the behavior.
• [x] #171 resulted in completely remove pausing/resuming the timer, which might actually be nice. Not a killer feature, but nice to have IMO
• [ ] ~This might look over the top but, using the History API (which I think you already use), the current time could be kept in the URL so that refreshing would not restart the timer! At the moment, pressing t and refreshing the page have similar result.~
• [x] And since I am dreaming and suggesting crazy things, having a decreasing timer would be the bomb. For example: starting remark with remark.create({ timer: { start: 20 * 60, critical: 120 } }); would start a 20-minute decreasing timer that changes to red when the 2-minute-left bar is reached.
• [x] The timer: { ... } object could also contain other pieces, like the ability to disable t to avoid accidents, or set the timer as increasing/decreasing, ...
• [x] I’d love to be able to switch off the timer. It’s distracting if you don’t need it.
• [x] I also find seeing the seconds distracting. Maybe minutes-only as an option?

[dvberkel]

@peterj since you assigned me to issue #195 I am addressing this discussion to you, but any maintainer could chime in.

I have looked at the code and the feature request. I think it is feasible to deliver most if not all the requested features. In order to be able to implement them I think the following things need to be done.

1. The SlideshowView now is passed an option.container and constructs a Timer. In order to be able to configure the Timer I suggest to pass the complete option to SlideshowView, letting it retrieve the container itself and passing the timer options to Timer.
2. In order to fulfill all the request the timer should respond to more events. I assuming that it is no problem to introduces timer related events.

If there are no objections I could start implementing these changes.

[peterj]

@dvberkel thanks for taking this on! The plan sounds good to me.

[dvberkel]

With respect to the feature request for the use of the History API to store the elapsed time. Using the History API would mean that a user could use the back button to an earlier moment and "buy" some time, i.e. loose track of elapsed time. That is the reason why I left it off the list.

Even though the request literally asks for the History API, I think that the intention of the feature is to not loose track of elapsed time, even when refreshing in the browser. I do think this is possible by writing the elapsed time e.g. into the query string and reading it back in, but if this is something to be supported is up to the maintainers.

@peterj What is your opinion? Should we support the keeping track of the elapsed time over refreshes?

[dvberkel]

With respect to the progress on this issue. I made all feature requests possible, but I need to wire the new implementation of the timer into the application. I also would like to thoroughly test the new implementation myself and document the options.

When I am done, I would like a code review and incorporate any changes that come from that review.

[dvberkel]

@peterj This pull request is ready for review.

There are two important points:

1. Do we want to retain the elapsed time on refresh? If so, we could open a different pull request.
2. I have described the options on the Configuration page. Is this enough or do we want some examples how to configure a countdown timer?

[peterj]

Nice work @dvberkel! I'll pull your branch and play around as well+ look at the code. As for retaining elapsed time on refresh - hard to say, we could open a separate issue/PR and track it there. In any case, I think it might make sense to make that configurable if we do decide to implement it.

I think the description on the Configuration page is enough - we can always add examples later if it's not clear how to use it.

[dvberkel]

@peterj Let me know your assessment of the code.

[dvberkel]

Happy new year! :tada:

[peterj]

Happy new year @dvberkel! I've played with the features (start/stop, reset, enable/disable, formatter) and it works great.

Two things:

• in the Configuration, the comment on the formatter mentions to show the timer in milliseconds, however, the millis variable is not used (should be a simple fix)

• you mentioned: remark.create({ timer: { start: 20 * 60, critical: 120 } });, however that doesn't seem to be implemented. I am fine if we do that as part of another PR if you wish :)

[dvberkel]

* in the [Configuration](https://github.com/gnab/remark/wiki/Configuration), the comment on the formatter mentions to show the timer in milliseconds, however, the `millis` variable is not used (should be a simple fix)

The comment is meant to say that the formatter receives the elapsed time in milliseconds. The default formatter output a format of H:mm:ss. Should I clarify this in the configuration, or is it best to describe the timer options in a paragraph or two?

* you mentioned: `remark.create({ timer: { start: 20 * 60, critical: 120 } });`, however that doesn't seem to be implemented. I am fine if we do that as part of another PR if you wish :)

Good catch. A decreasing timer can be configured with a suitable formatter. I will create a new issue requesting support for a decreasing timer by configuration. We can pick up that issue from there and create a new pull request for that issue alone.

[peterj]

Aah, ok - the elapsedTime is in milliseconds - make sense now :)