Move Step cleanup testing to Mocha; Improve clarity of hide/destory c…

…alls.

src/js/step.js

@@ -250,9 +250,17 @@ export class Step extends Evented {
     if (this.tooltip) {
       this.tooltip.destroy();
       this.tooltip = null;
+    }
+
+    if (this.el && this.el instanceof HTMLElement) {
+      this.el.parentNode.removeChild(this.el);
       this.el = null;
     }
 
+    if (this.target) {
+      this.target.classList.remove('shepherd-enabled', 'shepherd-target');
+    }
+
     this.trigger('destroy');
   }
 

src/js/tour.js

@@ -1,4 +1,4 @@
-import { isFunction, isNumber, isString, isUndefined } from 'lodash';
+import { isFunction, isNumber, isString, isUndefined, isEmpty } from 'lodash';
 import { Evented } from './evented.js';
 import { Step } from './step.js';
 import { bindMethods } from './bind.js';
@@ -112,18 +112,12 @@ export class Tour extends Evented {
    * @param {String} event The event name to trigger
    */
   done(event) {
-    if (this.currentStep) {
-      this.currentStep.hide();
+    if (!isEmpty(this.steps)) {
+      this.steps.forEach((step) => step.destroy());
     }
 
     this.trigger(event);
 
-    if (Shepherd.activeTour) {
-      Shepherd.activeTour.steps.forEach((step) => {
-        step.destroy();
-      });
-    }
-
     Shepherd.activeTour = null;
     document.body.classList.remove('shepherd-active');
     this.trigger('inactive', { tour: this });
@@ -224,6 +218,10 @@ export class Tour extends Evented {
    * @param {Boolean} forward True if we are going forward, false if backward
    */
   show(key = 0, forward = true) {
+    if (this.currentStep) {
+      this.currentStep.hide();
+    }
+
     this._setupActiveTour();
 
     const step = isString(key) ? this.getById(key) : this.steps[key];
@@ -258,17 +256,12 @@ export class Tour extends Evented {
   }
 
   /**
-   * If we have a currentStep, the tour is active, so just hide the step and remain active.
-   * Otherwise, make the tour active.
+   * Make this tour "active"
    * @private
    */
   _setupActiveTour() {
-    if (this.currentStep) {
-      this.currentStep.hide();
-    } else {
-      document.body.classList.add('shepherd-active');
-      this.trigger('active', { tour: this });
-    }
+    document.body.classList.add('shepherd-active');
+    this.trigger('active', { tour: this });
 
     Shepherd.activeTour = this;
   }

test/cypress/integration/element-targeting.spec.js

@@ -18,8 +18,17 @@ describe('Attaching tooltips to target elements in the DOM on each step', () =>
   });
 
   describe('Adding/Removing class names to the target of the current step', () => {
+    let tour;
+
+    beforeEach(() => {
+      tour = setupTour(Shepherd);
+    });
+
+    afterEach(() => {
+      tour.complete();
+    });
+
     it('Adds the "shepherd-target" and "shepherd-enabled" classes upon showing a step', () => {
-      const tour = setupTour(Shepherd);
       tour.start();
 
       cy.get('.hero-welcome')
@@ -28,7 +37,6 @@ describe('Attaching tooltips to target elements in the DOM on each step', () =>
     });
 
     it('Removes the "shepherd-target" and "shepherd-enabled" upon hiding a step', () => {
-      const tour = setupTour(Shepherd);
       tour.start();
       tour.next();
 

test/cypress/integration/test.acceptance.js

@@ -224,20 +224,5 @@ describe('Shepherd Acceptance Tests', () => {
         cy.get('.shepherd-step-element').should('have.length', 1);
       });
     });
-
-    describe('Cleaning up', () => {
-      let tour;
-
-      beforeEach(() => {
-        tour = setupTour(Shepherd);
-        tour.start();
-      });
-
-      it('renders no steps after the tour completes', () => {
-        tour.complete();
-
-        cy.get('.shepherd-step-element').should('not.exist');
-      });
-    });
   });
 });

test/unit/test.step.js

@@ -7,14 +7,15 @@ const { assert } = chai;
 import Shepherd from '../../src/js/shepherd.js';
 import { Step } from '../../src/js/step.js';
 import { Tour } from '../../src/js/tour.js';
+import tippy from 'tippy.js';
 import defaultButtons from '../cypress/utils/default-buttons';
 
 // since importing non UMD, needs assignment
 window.Shepherd = Shepherd;
 
 const DEFAULT_STEP_CLASS = 'shepherd-step-tooltip';
 
-describe('Step', () => {
+describe('Tour | Step', () => {
   describe('Shepherd.Step()', () => {
     const instance = new Shepherd.Tour({
       defaultStepOptions: {
@@ -63,10 +64,13 @@ describe('Step', () => {
       }
     });
 
-    after(() => {
-      instance.cancel();
+    beforeEach(() => {
+      tippy.disableAnimations();
     });
 
+    afterEach(() => {
+      instance.complete();
+    });
 
     it('has all the correct properties', () => {
       const values = ['classes', 'scrollTo', 'attachTo', 'id', 'text', 'buttons'];
@@ -102,7 +106,6 @@ describe('Step', () => {
   });
 
   describe('bindAdvance()', () => {
-    let tooltipElem;
     let event;
     let link;
     let hasAdvanced = false;
@@ -114,7 +117,8 @@ describe('Step', () => {
     };
 
     before(() => {
-      tooltipElem = document.createElement('div');
+      const tooltipElem = document.createElement('div');
+
       event = new Event(advanceOnEventName);
 
       link = document.createElement('a');

test/unit/test.tour.js

@@ -7,20 +7,25 @@ import tippy from 'tippy.js';
 import { defaults as tooltipDefaults } from '../../src/js/utils/tooltip-defaults';
 import { spy } from 'sinon';
 
-
 // since importing non UMD, needs assignment
 window.Shepherd = Shepherd;
 
-describe('Tour', function() {
+const DEFAULT_STEP_CLASS = 'shepherd-step-tooltip';
+
+describe('Tour | Top-Level Class', function() {
   let instance, shouldShowStep;
 
   const defaultStepOptions = {
-    classes: 'shepherd-theme-arrows',
+    classes: DEFAULT_STEP_CLASS,
     scrollTo: true
   };
 
+  beforeEach(() => {
+    tippy.disableAnimations();
+  });
+
   afterEach(() => {
-    instance.cancel();
+    instance.complete();
   });
 
   describe('constructor', function() {
@@ -34,7 +39,7 @@ describe('Tour', function() {
       instance = new Shepherd.Tour({ defaultStepOptions });
 
       assert.deepEqual(instance.options.defaultStepOptions, {
-        classes: 'shepherd-theme-arrows',
+        classes: DEFAULT_STEP_CLASS,
         scrollTo: true
       });
     });
@@ -73,7 +78,6 @@ describe('Tour', function() {
       });
 
       instance.addStep('test', {
-        classes: 'foo',
         id: 'test',
         title: 'This is a test step for our tour'
       });
@@ -101,7 +105,7 @@ describe('Tour', function() {
     describe('.addStep()', function() {
       it('adds tour steps', function() {
         assert.equal(instance.steps.length, 4);
-        assert.equal(instance.getById('test').options.classes, 'foo', 'classes passed to step options');
+        assert.equal(instance.getById('test').options.classes, DEFAULT_STEP_CLASS, 'classes passed to step options');
       });
 
       it('adds steps with only one arg', function() {
@@ -197,7 +201,6 @@ describe('Tour', function() {
         });
 
         instance.addStep('test', {
-          classes: 'foo',
           id: 'test',
           title: 'This is a test step for our tour'
         });
@@ -247,30 +250,79 @@ describe('Tour', function() {
     });
 
     describe('.complete()', function() {
-      it('tears down tour on complete', function() {
-        let inactiveFired = false;
-        instance.on('inactive', () => {
-          inactiveFired = true;
+      it('triggers complete event when complete function is called', function() {
+        let completeFired = false;
+
+        instance.on('complete', () => {
+          completeFired = true;
         });
+
+        instance.start();
+        instance.complete();
+        assert.isOk(completeFired, 'complete event fired');
+      });
+
+      it('calls `done()`', () => {
+        const doneSpy = spy(instance, 'done');
+
+        assert.equal(doneSpy.callCount, 0);
+
+        instance.start();
+        instance.complete();
+
+        assert.equal(doneSpy.callCount, 1);
+      });
+    });
+
+    describe('.done()', function() {
+      it('tears down the active tour', function() {
         instance.start();
+
         assert.equal(instance, Shepherd.activeTour, 'activeTour is set to our tour');
+
         instance.complete();
-        assert.isNotOk(Shepherd.activeTour, 'activeTour is torn down');
-        assert.isOk(inactiveFired, 'inactive event fired');
+
+        assert.equal(Shepherd.activeTour, null, '`activeTour` is torn down and removed from the `Shepherd` global');
       });
 
-      it('triggers complete event when complete function is called', function() {
-        let completeFired = false;
-        instance.on('complete', () => {
-          completeFired = true;
+
+      it('removes any of its `Step` tooltip elements from the DOM', function() {
+        const testStep = {
+          id: 'element-removal-test',
+          classes: 'element-removal-test',
+          title: 'This is a test step for our tour'
+        };
+
+        instance.addStep(testStep);
+        instance.start();
+        instance.show('element-removal-test');
+
+        assert.exists(document.querySelector(`.element-removal-test`), 'a step is rendered in the DOM after the tour starts');
+
+        instance.complete();
+
+        assert.notExists(document.querySelector(`.element-removal-test`), 'steps are removed from the DOM after the tour completes');
+      });
+
+
+      it('fires the `inactive` event', function() {
+        let inactiveFired = false;
+
+        instance.on('inactive', () => {
+          inactiveFired = true;
         });
 
         instance.start();
+
+        assert.equal(inactiveFired, false, 'inactive event does not fire before `complete()`');
+
         instance.complete();
-        assert.isOk(completeFired, 'complete event fired');
+
+        assert.equal(inactiveFired, true, 'inactive event fires after `complete()`');
       });
     });
 
+
     describe('.removeStep()', function() {
       it('removes the step when passed the id', function() {
         instance.start();