Skip to content

Conversation

@jhepp
Copy link

@jhepp jhepp commented Apr 13, 2019

Skip autoplay if a key press, a key release, a mouse move, a mouse button click or a touch is noticed.

To skip two autoplays after a user interaction use:
data-user-skip-autoplay="2"

reloadTimeout( event );
}, false );

var user = function( event ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you call this userActivity

"use strict";

var autoplayDefault = 0;
var userSkipAutoplayDefault = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like I was lazy and there's no documentation for this plugin. (Either in the comment above, or a separate README file.) Could you add it now? Including your own addition.

autoplayDefault = util.toNumber( data.autoplay, 0 );
}

if ( data.userSkipAutoplay ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming convention: You must call the attribute data-autoplay-something=""

See https://github.com/impress/impress.js/blob/master/src/plugins/README.md#structure-naming-and-policy

if (status === "delayed") {
if (userSkipAutoplay > 0) {
userSkipAutoplay = userSkipAutoplay - 1;
setAutoplayTimeout( timeout );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this... If I have

data-autoplay="5" data-autoplay-userdelay="2"

...then the total time on the slide is set to 15 seconds? This is a very weird outcome.

A simpler alternative, both for users and as implementation would be to just have userdelay as a boolean:

data-autoplay="5" data-autoplay-userdelay="1"    (or "true")

and then in userActivity() just reset the clock:

var userActivity = function(event) {
    reloadTimeout(event);
}

You could make this enabled by default. It seems like a good feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants