-
Notifications
You must be signed in to change notification settings - Fork 6.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
skip autoplay for some time after user interaction #729
base: master
Are you sure you want to change the base?
Conversation
@@ -59,6 +72,15 @@ | |||
reloadTimeout( event ); | |||
}, false ); | |||
|
|||
var user = function( event ) { |
There was a problem hiding this comment.
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
@@ -10,6 +10,8 @@ | |||
"use strict"; | |||
|
|||
var autoplayDefault = 0; | |||
var userSkipAutoplayDefault = 0; |
There was a problem hiding this comment.
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.
@@ -36,15 +39,25 @@ | |||
autoplayDefault = util.toNumber( data.autoplay, 0 ); | |||
} | |||
|
|||
if ( data.userSkipAutoplay ) { |
There was a problem hiding this comment.
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 ); |
There was a problem hiding this comment.
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.
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"