Skip to content

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ To navigate the slideshow:

* **forward**: K, L, UP, RIGHT, PgDn, and Space
* **reverse**: H, J, LEFT, DOWN, PgUp, and Backspace
* **first slide**: Home
* **last slide**: End

The toggle fullscreen mode, press the **ENTER** key.

Expand Down
7 changes: 7 additions & 0 deletions docs/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ Displays a small progress bar at the top of your document.

**Default**: true

### loop

Allows looping from the last slide to the first, or backwards from the first to
the last.

**Default**: true

### encoding

Content encoding to use on the rendered document.
Expand Down
2 changes: 2 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ Cleaver.prototype.loadStaticAssets = function () {
Cleaver.prototype.renderSlideshow = function () {
var putControls = this.options.controls || (this.options.controls === undefined);
var putProgress = this.options.progress || (this.options.progress === undefined);
var allowLooping = this.options.loop || (this.options.loop === undefined);
var style, script, output;

// Render the slides in a template (maybe as specified by the user)
Expand Down Expand Up @@ -253,6 +254,7 @@ Cleaver.prototype.renderSlideshow = function () {
style: style,
author: this.options.author,
script: script,
allowLooping: allowLooping,
options: this.options
};

Expand Down
44 changes: 40 additions & 4 deletions resources/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Owner

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_SLIDE but, ack!, this method is weird. I like your solution :)

Copy link
Contributor Author

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.

Expand All @@ -13,12 +30,27 @@ function currentPosition() {
function navigate(n) {
var position = currentPosition();
var numSlides = document.getElementsByClassName('slide').length;
var nextPosition;
if (n === FIRST_SLIDE) {
nextPosition = 1;
} else if (n === LAST_SLIDE) {
nextPosition = numSlides;
} else {
if (! ALLOW_LOOPING) {
if (n < 0 && position <= 1) {
return;
}
if (n > 0 && position >= numSlides) {
return;
}
}

/* Positions are 1-indexed, so we need to add and subtract 1 */
var nextPosition = (position - 1 + n) % numSlides + 1;
/* 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;
/* 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');
Expand Down Expand Up @@ -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();
}
Expand Down
1 change: 1 addition & 0 deletions templates/layout.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

<script type="text/javascript">
{{{script}}}
ALLOW_LOOPING = {{#allowLooping}}true{{/allowLooping}}{{^allowLooping}}false{{/allowLooping}};
Copy link
Owner

Choose a reason for hiding this comment

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

could we throw a var in front of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, except that its not a declaration (that's up in script.js), so doing that would end up with double-declaring that variable. Also, since it's going into the window scope (per the way script.js is written), the var keyword is superfluous anyway and I wouldn't have bothered anywhere, except for matching style.

Copy link
Owner

Choose a reason for hiding this comment

The 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>