[Bug] - fix cancel on any step with a title (#228)

* fixes insertion of title element to not use innerHTML

* add test to confirm concel functionality after first step

* change sinon to devDep

package.json

@@ -90,6 +90,7 @@
     "postcss-loader": "^3.0.0",
     "replace": "^1.0.0",
     "sass-loader": "^7.1.0",
+    "sinon": "^6.1.5",
     "source-map-loader": "^0.2.3",
     "start-server-and-test": "^1.7.0",
     "style-loader": "^0.22.1",

src/js/bind.js

@@ -84,4 +84,4 @@ export function bindMethods(methods) {
   methods.map((method) => {
     this[method] = this[method].bind(this);
   });
-}
+}

src/js/step.js

@@ -72,7 +72,6 @@ export class Step extends Evented {
       header.appendChild(link);
 
       element.classList.add('shepherd-has-cancel-link');
-
       this.bindCancelLink(link);
     }
   }
@@ -138,6 +137,14 @@ export class Step extends Evented {
     const element = createFromHTML(`<div class='${classes}' data-id='${this.id}' id="${this.options.idAttribute}"}>`);
     const header = document.createElement('header');
 
+    if (this.options.title) {
+      const title = document.createElement('h3');
+      title.classList.add('shepherd-title');
+      title.innerHTML = `${this.options.title}`;
+      header.prepend(title);
+      element.classList.add('shepherd-has-title');
+    }
+
     content.classList.add('shepherd-content');
     element.appendChild(content);
     content.appendChild(header);
@@ -153,11 +160,6 @@ export class Step extends Evented {
     this._addButtons(content);
     this._addCancelLink(element, header);
 
-    if (this.options.title) {
-      header.innerHTML += `<h3 class="shepherd-title">${this.options.title}</h3>`;
-      element.classList.add('shepherd-has-title');
-    }
-
     return element;
   }
 
@@ -222,7 +224,7 @@ export class Step extends Evented {
     const { when } = this.options;
 
     this.destroy();
-    this.id = this.options.id || this.id || `step-${uniqueId()}`;
+    this.id = this.options.id || `step-${uniqueId()}`;
 
     _.forOwn(when, (handler, event) => {
       this.on(event, handler, this);

test/cypress/integration/test.acceptance.js

@@ -147,6 +147,19 @@ describe('Shepherd Acceptance Tests', () => {
       cy.get('.shepherd-cancel-link').click();
       cy.get('body').should('not.have.class', 'shepherd-active');
     });
+
+    it('Cancel link cancels the tour from another step', () => {
+      const tour = setupTour(Shepherd);
+      tour.start();
+      cy.get('body').should('have.class', 'shepherd-active');
+      // Click next
+      cy.contains('Next').click();
+      // Step two text should be visible
+      cy.get('.shepherd-text')
+        .contains('Including Shepherd is easy!').should('be.visible');
+      cy.get('.shepherd-cancel-link:nth-child(2)').click();
+      cy.get('body').should('not.have.class', 'shepherd-active');
+    });
   });
 
   it.skip('Defaults classes applied', () => {

test/unit/test.step.js

@@ -1,5 +1,6 @@
 import chai from 'chai';
 import chaiAsPromised from 'chai-as-promised';
+import { spy } from 'sinon';
 
 chai.use(chaiAsPromised);
 const { assert } = chai;
@@ -144,6 +145,25 @@ describe('Step', () => {
 
       assert.isOk(advanced, 'next triggered for advanceOn');
     });
+
+    it('it should call removeEventListener when destoryed', function(done){
+      const el = document.createElement('div');
+      const body = spy(document.body, 'removeEventListener');
+      const step = new Step({
+        next: () => true
+      }, {
+        advanceOn: { event: 'test' }
+      });
+      step.el = el;
+      step.el.hidden = false;
+
+      step.bindAdvance();
+      step.trigger('destroy');
+      assert.ok(body.called);
+      body.restore();
+      done();
+    });
+
   });
 
   describe('bindButtonEvents()', () => {