Remove the string option for advanceOn in favor of object

README.md

@@ -61,7 +61,10 @@ tour.addStep('example', {
   title: 'Example Shepherd',
   text: 'Creating a Shepherd is easy too! Just create ...',
   attachTo: '.hero-example bottom',
-  advanceOn: '.docs-link click'
+  advanceOn: {
+    selector: '.docs-link',
+    event: 'click'
+  }
 });
 
 tour.start();

index.md

@@ -213,9 +213,9 @@ the step will execute. For example:
     }
   }
   ```
-- `advanceOn`: An action on the page which should advance shepherd to the next step.  It can be of the form `"selector event"`, or an object with those
-properties.  For example: `".some-element click"`, or `{selector: '.some-element', event: 'click'}`.  It doesn't have to be an event inside
-the tour, it can be any event fired on any element on the page.  You can also always manually advance the Tour by calling `myTour.next()`.
+- `advanceOn`: An action on the page which should advance shepherd to the next step.  It should be an object with a string `selector` and an `event` name.
+For example: `{selector: '.some-element', event: 'click'}`.  It doesn't have to be an event inside the tour, it can be any event fired on any element on the page.  
+You can also always manually advance the Tour by calling `myTour.next()`.
 - `highlightClass`: An extra class to apply to the `attachTo` element when it is highlighted (that is, when its step is active). You can then target that selector in your CSS.
 - `showCancelLink`: Should a cancel "✕" be shown in the header of the step?
 - `showOn`: A function that, when it returns true, will show the step. If it returns false, the step will be skipped.

package.json

@@ -52,7 +52,6 @@
     "lodash.defer": "^4.1.0",
     "lodash.iselement": "^4.1.1",
     "lodash.isobjectlike": "^4.0.0",
-    "lodash.zipobject": "^4.1.3",
     "smoothscroll-polyfill": "^0.4.4",
     "tippy.js": "^4.3.4"
   },

src/js/step.js

@@ -54,15 +54,8 @@ export class Step extends Evented {
    * in the middle of the screen, without an arrow pointing to the target.
    * @param {HTMLElement|string} options.attachTo.element
    * @param {string} options.attachTo.on
-   * @param {Object|string} options.advanceOn An action on the page which should advance shepherd to the next step.
-   * It can be of the form `"selector event"`:
-   * ```js
-   * const new Step(tour, {
-   *   advanceOn: '.some .selector-path click',
-   *   ...moreOptions
-   * })'
-   * ```
-   * ...or an object with those properties:
+   * @param {Object} options.advanceOn An action on the page which should advance shepherd to the next step.
+   * It should be an object with a string `selector` and an `event` name
    * ```js
    * const new Step(tour, {
    *   advanceOn: { selector: '.some .selector-path', event: 'click' },

src/js/utils/bind.js

@@ -1,8 +1,9 @@
-import { parseShorthand } from './general.js';
 import { isString, isUndefined } from './type-check';
 
 /**
  * Sets up the handler to determine if we should advance the tour
+ * @param selector
+ * @return {Function}
  * @private
  */
 function _setupAdvanceOnHandler(selector) {
@@ -23,19 +24,33 @@ function _setupAdvanceOnHandler(selector) {
  */
 export function bindAdvance() {
   // An empty selector matches the step element
-  const { event, selector } = parseShorthand(this.options.advanceOn, ['selector', 'event']);
-  const handler = _setupAdvanceOnHandler.call(this, selector);
+  const { event, selector } = this.options.advanceOn || {};
+  if (event) {
+    const handler = _setupAdvanceOnHandler.call(this, selector);
 
-  // TODO: this should also bind/unbind on show/hide
-  const el = document.querySelector(selector);
-  if (!isUndefined(selector) && el) {
-    el.addEventListener(event, handler);
+    // TODO: this should also bind/unbind on show/hide
+    let el;
+    try {
+      el = document.querySelector(selector);
+    } catch(e) {
+      // TODO
+    }
+    if (!isUndefined(selector) && !el) {
+      return console.error(`No element was found for the selector supplied to advanceOn: ${selector}`);
+    } else if (el) {
+      el.addEventListener(event, handler);
+      this.on('destroy', () => {
+        return el.removeEventListener(event, handler);
+      });
+    } else {
+      document.body.addEventListener(event, handler, true);
+      this.on('destroy', () => {
+        return document.body.removeEventListener(event, handler, true);
+      });
+    }
   } else {
-    document.body.addEventListener(event, handler, true);
+    return console.error('advanceOn was defined, but no event name was passed.');
   }
-  this.on('destroy', () => {
-    return document.body.removeEventListener(event, handler, true);
-  });
 }
 
 /**

src/js/utils/general.js

@@ -1,6 +1,5 @@
 import isObjectLike from 'lodash.isobjectlike';
 import { isString, isUndefined } from './type-check';
-import zipObject from 'lodash.zipobject';
 import tippy from 'tippy.js';
 import { missingTippy } from './error-messages';
 
@@ -100,7 +99,7 @@ export function drop(arr, n = 1) {
  */
 export function _parseAttachToOpts(opts) {
   if (isObjectLike(opts)) {
-    if (opts.hasOwnProperty('element') && opts.hasOwnProperty('on')) {
+    if (Object.prototype.hasOwnProperty.call(opts, 'element') && Object.prototype.hasOwnProperty.call(opts, 'on')) {
       return opts;
     }
     return null;
@@ -119,22 +118,6 @@ export function _parseAttachToOpts(opts) {
   };
 }
 
-/**
- * @param obj
- * @param {Array} props
- * @return {*}
- */
-export function parseShorthand(obj, props) {
-  if (obj === null || isUndefined(obj)) {
-    return obj;
-  } else if (isObjectLike(obj)) {
-    return obj;
-  }
-
-  const values = obj.split(' ');
-  return zipObject(props, values);
-}
-
 /**
  * Determines options for the tooltip and initializes
  * `this.tooltip` as a Tippy.js instance.

test/dummy/index.html

@@ -60,7 +60,7 @@ <h3>Example</h3>
               title: 'Example Shepherd',
               text: 'Creating a Shepherd is easy too! Just create ...',
               attachTo: '.hero-example bottom',
-              advanceOn: '.docs-link click'
+              advanceOn: { selector: '.docs-link', event: 'click' }
             });
 
             tour.start();

test/unit/step.spec.js

@@ -143,9 +143,7 @@ describe('Tour | Step', () => {
       next() { hasAdvanced = true; }
     };
 
-    beforeAll(() => {
-      const tooltipElem = document.createElement('div');
-
+    beforeEach(() => {
       event = new Event(advanceOnEventName);
 
       link = document.createElement('a');
@@ -155,24 +153,10 @@ describe('Tour | Step', () => {
       document.body.appendChild(link);
     });
 
-    afterAll(() => {
+    afterEach(() => {
       link.remove();
     });
 
-    it('triggers the `advanceOn` option via string', () => {
-      const step = new Step(tourProto, {
-        advanceOn: `.${advanceOnSelector} ${advanceOnEventName}`
-      });
-
-      step.isOpen = () => true;
-
-      step.bindAdvance();
-      link.dispatchEvent(event);
-
-      expect(link.classList.contains(advanceOnSelector)).toBe(true);
-      expect(hasAdvanced).toBe(true);
-    });
-
     it('triggers the `advanceOn` option via object', () => {
       const step = new Step(tourProto, {
         advanceOn: { selector: `.${advanceOnSelector}`, event: advanceOnEventName }
@@ -362,10 +346,6 @@ describe('Tour | Step', () => {
       }
     });
 
-    beforeAll(() => {
-      document.body.setAttribute('data-shepherd-step', 1);
-    });
-
     it('triggers the before-hide event', () => {
       step.on('before-hide', () => beforeHideTriggered = true);
       step.hide();
@@ -376,10 +356,6 @@ describe('Tour | Step', () => {
     it('calls tour.modal.hide', () => {
       expect(modalHideCalled, 'tour.modal.hide called').toBeTruthy();
     });
-
-    it('removes the data-shepherd-step attribute', () => {
-      expect(document.body.hasAttribute('data-shepherd-step'), 'step attribute is removed').toBeFalsy();
-    });
   });
 
   describe('parseAttachTo()', function() {
@@ -417,7 +393,7 @@ describe('Tour | Step', () => {
       const step = new Step({
         next: () => true
       }, {
-        advanceOn: '.click-test test'
+        advanceOn: { selector: '.click-test', event: 'test' }
       });
       const bindFunction = spy(step, 'bindAdvance');
       step.setupElements();

test/unit/utils/general.spec.js

@@ -1,6 +1,5 @@
 import {
-  _parseAttachToOpts,
-  parseShorthand
+  _parseAttachToOpts
 } from '../../../src/js/utils/general.js';
 
 describe('Utils', function() {
@@ -24,18 +23,4 @@ describe('Utils', function() {
       expect(_parseAttachToOpts(attachTo), 'when `on` is not a valid direction, return null').toBe(null);
     });
   });
-
-  describe('parseShorthand', function() {
-    it('null or undefined', function() {
-      expect(parseShorthand(null), 'null returns null').toBe(null);
-      expect(parseShorthand(undefined), 'undefined returns undefined').toBe(undefined);
-    });
-
-    it('string of values', function() {
-      const values = '.foo click';
-      const { event, selector } = parseShorthand(values, ['selector', 'event']);
-      expect(event, 'maps event from string to event prop').toBe('click');
-      expect(selector, 'maps selector from string to selector prop').toBe('.foo');
-    });
-  });
 });

yarn.lock

@@ -5891,11 +5891,6 @@ lodash.uniq@^4.5.0:
   resolved "https://registry.yarnpkg.com/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773"
   integrity sha1-0CJTc662Uq3BvILklFM5qEJ1R3M=
 
-lodash.zipobject@^4.1.3:
-  version "4.1.3"
-  resolved "https://registry.yarnpkg.com/lodash.zipobject/-/lodash.zipobject-4.1.3.tgz#b399f5aba8ff62a746f6979bf20b214f964dbef8"
-  integrity sha1-s5n1q6j/YqdG9peb8gshT5ZNvvg=
-
 lodash@4.17.11, lodash@^4.1.0, lodash@^4.15.0, lodash@^4.17.10, lodash@^4.17.11, lodash@^4.17.4, lodash@^4.2.0:
   version "4.17.11"
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.11.tgz#b39ea6229ef607ecd89e2c8df12536891cac9b8d"