Repository: FormidableLabs/spectacle
Pull request #502 (2018-03-30 23:35:19)

Add autoplayLoop prop
Hi!

This PR add the possibility to set the autoplay loop from outside (defaults to `true` cause it's the current behavior, although I'd say that `false` should make more sense eventually). 

Any suggestion/feedback/whatever will be very welcome, thanks!

_P.S. tests were already failing before this commit so I'm not sure if that's ok or not_ 💭 

ryan-roemer: Thanks for opening a PR!

I'll let @kenwheeler opine as to the substantive PR when he has a moment, but on the tests front `master` build is fully green, both in Travis [`master`](https://travis-ci.org/FormidableLabs/spectacle/builds/356600522) and on my local machine from a fresh install.

All of the failures I see for one of the travis builds here are for the `autoplay` stuff you've added failing because of mismatch with the fixtures -- https://travis-ci.org/FormidableLabs/spectacle/jobs/360482085 . I'm guessing a jest snapshot update is in order.

As a helpful tip, all code needs to pass:

```sh
$ yarn run build
$ yarn run check
```

... which is what travis is doing...

Thanks!
ryan-roemer: @nuragic -- Can you create a gist of the entire terminal log from a fresh `master` clone + `yarn install` (since we're using a yarn.lock). Then echo your `node -v` and `yarn -v` into the terminal, then run the tests. And paste that all in a gist so I can take a look. Thanks!
nuragic: @ryan-roemer Sorry, I removed my entire folder, cloned the fork again... and it worked 😅 ... wondering what could have originated the problem, anyway... nevermind! Thanks for your time, I've just updated the snapshots for this branch. 
ebrillhart: Hey @nuragic, sorry for the delay - if you could fix the merge conflicts I'll accept this PR. 
nuragic: Hi Emma! No worries... done! 🚢 
ebrillhart: @nuragic ah sorry, it also looks like prettier is failing in CI, which is something we've added since you originally pushed this. If you could pull down, make sure you have latest master merged in/do a fresh yarn or npm install, and run `yarn prettier-fix`, that should solve the problem! Thanks for working with me on this.
nuragic: Upsss sorry 😅 fixed! Thanks to you for your time 🙌 

Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
new file mode 100644
index 0000000..5ee0df8
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -0,0 +1,45 @@
+---
+name: Bug report
+about: Create a report to help us improve
+---
+
+<!--
+
+Have you read Formidable's Code of Conduct? By filing an Issue, you are expected to comply with it, including treating everyone with respect: https://github.com/FormidableLabs/spectacle/blob/master/CONTRIBUTING.md#contributor-covenant-code-of-conduct
+
+-->
+
+### Prerequisites
+
+**Feel free to delete this section if you have checked off all of the following.**
+
+- [ ] I've searched open [issues](https://www.github.com/FormidableLabs/spectacle/issues) to make sure I'm not opening a duplicate issue
+- [ ] This issue not specific to `spectacle-boilerplate` (those issues should be opened [here](https://github.com/FormidableLabs/spectacle-boilerplate/issues/new)).
+- [ ] I have read through the [docs](https://formidable.com/open-source/spectacle/docs/) before asking a question
+- [ ] I am using the latest version of Spectacle
+
+### Describe Your Environment
+
+What version of Spectacle are you using? (can be found by running `npm list spectacle`)
+
+What version of React are you using? (can be found by running `npm list react`)
+
+What browser are you using?
+
+What machine are you on?
+
+### Describe the Problem
+
+It's easier to show us than tell us what's going wrong with your code. Because of this, we ask that you do one of three things to help us reproduce the bug:
+
+1. Create a public minimal repository that we can `git clone`, with install + error reproduction steps in the README.
+2. Fork our [simple Spectacle sample Sandbox](https://codesandbox.io/s/7wo8xv8nw0), reproduce your issue in the code, and paste the link here.
+3. Open up a PR, include "WIP" and the Issue # in the title, and point us to the failing regression tests.
+
+**Expected behavior:** [What you expect to happen]
+
+**Actual behavior:** [What actually happens]
+
+### Additional Information
+
+Any additional information, configuration or data that might be necessary to reproduce the issue.
diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md
new file mode 100644
index 0000000..b22ad2a
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/feature_request.md
@@ -0,0 +1,22 @@
+---
+name: Feature request
+about: Suggest an idea for this project
+---
+
+<!--
+
+Have you read Formidable's Code of Conduct? By filing an Issue, you are expected to comply with it, including treating everyone with respect: https://github.com/FormidableLabs/spectacle/blob/master/CONTRIBUTING.md#contributor-covenant-code-of-conduct
+
+-->
+
+### Description
+
+Including the problem you want to address, use cases, benefits, and/or goals.
+
+### Proposal
+
+How do you propose we implement this change?
+
+### Links / References
+
+Any resources you want to point to for reference or more information.
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
new file mode 100644
index 0000000..8ed8637
--- /dev/null
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -0,0 +1,35 @@
+<!--
+
+Have you read Formidable's Code of Conduct? By filing an Issue, you are expected to comply with it, including treating everyone with respect: https://github.com/FormidableLabs/spectacle/blob/master/CONTRIBUTING.md#contributor-covenant-code-of-conduct
+
+-->
+
+### Description
+
+Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
+
+Fixes # (issue)
+
+#### Type of Change
+
+Please delete options that are not relevant.
+
+- [ ] Bug fix (non-breaking change which fixes an issue)
+- [ ] New feature (non-breaking change which adds functionality)
+- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
+- [ ] This change requires a documentation update
+
+### How Has This Been Tested?
+
+Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
+
+### Checklist: (Feel free to delete this section upon completion)
+
+- [ ] My code follows the style guidelines of this project (I have run `yarn prettier-fix && yarn lint`)
+- [ ] I have added tests that prove my fix is effective or that my feature works
+- [ ] New and existing unit tests pass locally with my changes (I have run `yarn test`)
+- [ ] I have performed a self-review of my own code
+- [ ] I have commented my code, particularly in hard-to-understand areas
+- [ ] I have made corresponding changes to the documentation
+- [ ] My changes generate no new warnings
+- [ ] Any dependent changes have been merged and published in downstream modules
diff --git a/.gitignore b/.gitignore
index a05bbdf..53d28d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 node_modules
+package-lock.json
 dist
 lib
 es
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 3e4c9ea..fcf33f0 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -4,7 +4,7 @@ Thanks for contributing!
 
 ### Installing dependencies
 
-```bash
+```sh
 yarn install
 ```
 
@@ -16,41 +16,33 @@ You will find tests for files colocated with `*.test.js` suffixes. Whenever maki
 
 If you are adding a new feature or some extra functionality, you should also make sure to accompany those changes with appropriate tests.
 
-### Linting and Formating
+### Linting and Formatting
 
-Before commiting any changes, be sure to do `yarn run lint`; this will lint all relevant files using [ESLint](http://eslint.org/) and report on any changes that you need to make.
+Before committing any changes, be sure to do `yarn run lint`; this will lint all relevant files using [ESLint](http://eslint.org/) and report on any changes that you need to make.
 
-You will also want to ensure your code meets the prettier formating guidelines by running `yarn run prettier -l <filename>` on a specific file. If there are differences the script errors out. You can also speciify a glob `yarn run prettier -l "src/**/*.js"` which will return a list of files that do not conform.
+You will also want to ensure your code meets the prettier formatting guidelines by running `yarn run prettier -l <filename>` on a specific file. If there are differences the script errors out. You can also specify a glob `yarn run prettier -l "src/**/*.js"` which will return a list of files that do not conform.
 
-Alternitivley install the Prettier [editor plugin](https://prettier.io/docs/en/editors.html) in your favorite editor. This is the perfered method.
+Alternatively, install the Prettier [editor plugin](https://prettier.io/docs/en/editors.html) in your favorite editor. This is the preferred method.
 
-There is also a pre-commit hook in place to lint all staged files. If any of the staged files do not conform to the eslint rules or the [prettier](https://prettier.io/) formating guidelines, your commit will fail until you resolve all outstanding issues.
+There is also a pre-commit hook in place to lint all staged files. If any of the staged files do not conform to the eslint rules or the [prettier](https://prettier.io/) formatting guidelines, your commit will fail until you resolve all outstanding issues.
 
-To resolve/fix prettier formating problems from the CLI:
+To resolve/fix prettier formatting problems from the CLI:
 
-```
-yarn run prettier --write <filename>
+```sh
+yarn prettier-fix && yarn lint-fix
 ```
 
 This will modify your file in place. You will need to `git add` the file again and re-commit.
 
-Example
-
-```
-$ yarn run prettier --write src/components/time.js
-...
-src/components/time.js 93ms
-✨  Done in 0.58s.
-```
-
 ### Before submitting a PR...
 
 Thanks for taking the time to help us make Spectacle even better! Before you go ahead and submit a PR, make sure that you have done the following:
 
 - Run the tests using `yarn run test`.
 - Run lint and flow using `yarn run lint`
+- Everything else included in our [pull request checklist](https://github.com/FormidableLabs/spectacle/blob/master/.github/PULL_REQUEST_TEMPLATE.md#checklist-feel-free-to-delete-this-section-upon-completion)
 
-## Releasing a new version to NPM (only for project administrators):
+## Releasing a new version to NPM (only for project administrators)
 
 1.  Run `npm version patch` (or `minor`, `major` as appropriate) to run tests and lint, build the `lib` ands `dist` directories, , then update `package.json` and add a git tag.
 2.  Run `npm publish` and publish to npm if all is well.
@@ -113,7 +105,7 @@ further defined and clarified by project maintainers.
 ### Enforcement
 
 Instances of abusive, harassing, or otherwise unacceptable behavior may be
-reported by contacting the project team at ken.wheeler@formidable.com. All
+reported by contacting the project team at emma.brillhart@formidable.com. All
 complaints will be reviewed and investigated and will result in a response that
 is deemed necessary and appropriate to the circumstances. The project team is
 obligated to maintain confidentiality with regard to the reporter of an incident.
diff --git a/README.md b/README.md
index 84b4bfc..559f197 100644
--- a/README.md
+++ b/README.md
@@ -418,6 +418,7 @@ The Deck tag is the root level tag for your presentation. It supports the follow
 | transitionDuration      | PropTypes.number  | Accepts integer value in milliseconds for global transition duration.                                                                                                         |
 | autoplay                | PropTypes.bool    | Automatically advance slides.                                                                                                                                                 |
 | autoplayDuration        | PropTypes.number  | Accepts integer value in milliseconds for global autoplay duration, defaults to 7000.                                                                                         |
+| autoplayLoop            | PropTypes.bool    | Keep slides in loop.                                                                                                                                                          |
 
 <a name="slide-base"></a>
 
diff --git a/docs/tag-api.md b/docs/tag-api.md
index 58abafc..e16c0ad 100644
--- a/docs/tag-api.md
+++ b/docs/tag-api.md
@@ -39,13 +39,13 @@ The deck tag wraps your slides. It supports the following props:
 
 The slide tag represents each slide in the presentation. Giving a slide tag an `id` attribute will replace its number based navigation hash with the `id` provided. It supports the following props, in addition to any of the props outlined in the Base class props listing:
 
-| Name               | PropType         | Description                                                                                                                                                                                                                                                                                                                      |
-| ------------------ | ---------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| align              | PropTypes.string | Accepts a space delimited value for positioning interior content. The first value can be `flex-start` (left), `center` (middle), or `flex-end` (bottom). The second value can be `flex-start` (top) , `center` (middle), or `flex-end` (bottom). You would provide this prop like `align="center center"`, which is its default. |
-| transition         | PropTypes.array  | Accepts `slide`, `zoom`, `fade` or `spin`, and can be combined. Sets the slide transition. **Note: If you use the 'scale' transition, fitted text won't work in Safari.**                                                                                                                                                        |
-| transitionDuration | PropTypes.number | Accepts integer value in milliseconds for slide transition duration.                                                                                                                                                                                                                                                             |
-| notes              | PropTypes.string | Text which will appear in the presenter mode. Can be HTML.                                                                                                                                                                                                                                                                       |
-| id                 | PropTypes.string | Used to create a string based hash.                                                                                                                                                                                                                                                                                              |
+| Name               | PropType         | Description                                                                                                                                                                                                                                                                                                                     |
+| ------------------ | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| align              | PropTypes.string | Accepts a space delimited value for positioning interior content. The first value can be `flex-start` (left), `center` (middle), or `flex-end` (right). The second value can be `flex-start` (top) , `center` (middle), or `flex-end` (bottom). You would provide this prop like `align="center center"`, which is its default. |
+| transition         | PropTypes.array  | Accepts `slide`, `zoom`, `fade` or `spin`, and can be combined. Sets the slide transition. **Note: If you use the 'scale' transition, fitted text won't work in Safari.**                                                                                                                                                       |
+| transitionDuration | PropTypes.number | Accepts integer value in milliseconds for slide transition duration.                                                                                                                                                                                                                                                            |
+| notes              | PropTypes.string | Text which will appear in the presenter mode. Can be HTML.                                                                                                                                                                                                                                                                      |
+| id                 | PropTypes.string | Used to create a string based hash.                                                                                                                                                                                                                                                                                             |
 
 <a name="layout-tags"></a>
 
diff --git a/src/components/deck.js b/src/components/deck.js
index dddd5e6..b991f95 100644
--- a/src/components/deck.js
+++ b/src/components/deck.js
@@ -15,6 +15,7 @@ export default class Deck extends Component {
   static propTypes = {
     autoplay: PropTypes.bool,
     autoplayDuration: PropTypes.number,
+    autoplayLoop: PropTypes.bool,
     children: PropTypes.node,
     controls: PropTypes.bool,
     globalStyles: PropTypes.bool,
diff --git a/src/components/manager.js b/src/components/manager.js
index d71f4c6..dc4f883 100644
--- a/src/components/manager.js
+++ b/src/components/manager.js
@@ -68,6 +68,7 @@ export class Manager extends Component {
   static propTypes = {
     autoplay: PropTypes.bool,
     autoplayDuration: PropTypes.number,
+    autoplayLoop: PropTypes.bool,
     children: PropTypes.node,
     contentHeight: PropTypes.number,
     contentWidth: PropTypes.number,
@@ -102,6 +103,7 @@ export class Manager extends Component {
   static defaultProps = {
     autoplay: false,
     autoplayDuration: 7000,
+    autoplayLoop: true,
     contentWidth: 1000,
     contentHeight: 700,
     disableKeyboardControls: false,
@@ -455,7 +457,11 @@ export class Manager extends Component {
     ) {
       if (slideIndex === slideReference.length - 1) {
         // On last slide, loop to first slide
-        if (this.props.autoplay && this.state.autoplaying) {
+        if (
+          this.props.autoplay &&
+          this.props.autoplayLoop &&
+          this.state.autoplaying
+        ) {
           const slideData = '{ "slide": "0", "forward": "false" }';
           this._goToSlide({ key: 'spectacle-slide', newValue: slideData });
           this.viewedIndexes = new Set();


Test patch:
diff --git a/src/components/__snapshots__/manager.test.js.snap b/src/components/__snapshots__/manager.test.js.snap
index 030c556..0b85fb1 100644
--- a/src/components/__snapshots__/manager.test.js.snap
+++ b/src/components/__snapshots__/manager.test.js.snap
@@ -13,6 +13,7 @@ exports[`<Manager /> should render correctly. 1`] = `
   <Manager
     autoplay={false}
     autoplayDuration={7000}
+    autoplayLoop={true}
     contentHeight={700}
     contentWidth={1000}
     controls={true}
@@ -392,6 +393,7 @@ exports[`<Manager /> should render the export configuration when specified. 1`]
   <Manager
     autoplay={false}
     autoplayDuration={7000}
+    autoplayLoop={true}
     contentHeight={700}
     contentWidth={1000}
     controls={true}
@@ -525,6 +527,7 @@ exports[`<Manager /> should render the overview configuration when specified. 1`
   <Manager
     autoplay={false}
     autoplayDuration={7000}
+    autoplayLoop={true}
     contentHeight={700}
     contentWidth={1000}
     controls={true}
@@ -725,6 +728,7 @@ exports[`<Manager /> should render with slideset slides 1`] = `
   <Manager
     autoplay={false}
     autoplayDuration={7000}
+    autoplayLoop={true}
     contentHeight={700}
     contentWidth={1000}
     controls={true}