-
Notifications
You must be signed in to change notification settings - Fork 300
Feature/no slide looping #154
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
1bac41d
7c371c8
312af87
5eb8308
e276b95
7b01d2a
3d3c863
4b4837a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,23 @@ function currentPosition() { | |
| return parseInt(document.querySelector('.slide:not(.hidden)').id.slice(6)); | ||
| } | ||
|
|
||
| /** | ||
| * If passed to navigate, will move to the first slide, independent of the | ||
| * current location, rather than a relative amount. | ||
| */ | ||
| var FIRST_SLIDE = -9999999; | ||
|
|
||
| /** | ||
| * If passed to navigate, will move to the last slide, independent of the | ||
| * current location, rather than a relative amount. | ||
| */ | ||
| var LAST_SLIDE = 9999999; | ||
|
|
||
| /** | ||
| * Whether to allow looping from the last back to the first or backwards from | ||
| * the first to the last. | ||
| */ | ||
| var ALLOW_LOOPING = true; | ||
|
|
||
| /** | ||
| * Navigates forward n pages | ||
|
|
@@ -13,12 +30,27 @@ function currentPosition() { | |
| function navigate(n) { | ||
| var position = currentPosition(); | ||
| var numSlides = document.getElementsByClassName('slide').length; | ||
|
|
||
| /* Positions are 1-indexed, so we need to add and subtract 1 */ | ||
| var nextPosition = (position - 1 + n) % numSlides + 1; | ||
|
|
||
| /* Normalize nextPosition in-case of a negative modulo result */ | ||
| nextPosition = (nextPosition - 1 + numSlides) % numSlides + 1; | ||
| var nextPosition; | ||
| if (n === FIRST_SLIDE) { | ||
| nextPosition = 1; | ||
| } else if (n === LAST_SLIDE) { | ||
| nextPosition = numSlides; | ||
| } else { | ||
| if (! ALLOW_LOOPING) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think cleaver just uses 2 spaces for tabs (which I don't really like these days but oh well), could we change these indents?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fixed the indents. Is there a reason the linter doesn't check that, if it's something people care about?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No reason except I never added it :( |
||
| if (n < 0 && position <= 1) { | ||
| return; | ||
| } | ||
| if (n > 0 && position >= numSlides) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| /* Positions are 1-indexed, so we need to add and subtract 1 */ | ||
| nextPosition = (position - 1 + n) % numSlides + 1; | ||
|
|
||
| /* Normalize nextPosition in-case of a negative modulo result */ | ||
| nextPosition = (nextPosition - 1 + numSlides) % numSlides + 1; | ||
| } | ||
|
|
||
| document.getElementById('slide-' + position).classList.add('hidden'); | ||
| document.getElementById('slide-' + nextPosition).classList.remove('hidden'); | ||
|
|
@@ -130,6 +162,10 @@ document.addEventListener('DOMContentLoaded', function () { | |
| navigate(-1); | ||
| } else if (kc === 38 || kc === 39 || kc === 32 || kc === 75 || kc === 76 || kc === 34) { | ||
| navigate(1); | ||
| } else if (kc === 36) { | ||
| navigate(FIRST_SLIDE); | ||
| } else if (kc === 35) { | ||
| navigate(LAST_SLIDE); | ||
| } else if (kc === 13) { | ||
| toggleFullScreen(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
|
|
||
| <script type="text/javascript"> | ||
| {{{script}}} | ||
| ALLOW_LOOPING = {{#allowLooping}}true{{/allowLooping}}{{^allowLooping}}false{{/allowLooping}}; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we throw a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could, except that its not a declaration (that's up in
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good catch, you're absolutely right :) Thanks for the explanation. |
||
| </script> | ||
| </body> | ||
| </html> | ||
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.
Was going to ask why not just use 1 for
FIRST_SLIDEbut, ack!, this method is weird. I like your solution :)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.
Yeah, since it uses relative values, "1" means "advance one slide", so had to pick values outside any useful range. I suppose if someone ends up with a 10,000,000 slide presentation there will be problems with this feature, but it seems like that's a fair "bug" to leave latent.