Added default fallback for target when step is hidden/destroyed (#1132)

* Added default fallback for target when step is hidden so "shepherd-enabled" class is removed from body on non-attached steps * Added unit test to cover scenario Co-authored-by: Tom Wilson <[email protected]>
shipshapecode · Sep 5, 2020 · bd13413 · bd13413
1 parent fc66565
commit bd13413
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 13 deletions.
diff --git a/src/js/step.js b/src/js/step.js
@@ -155,9 +155,7 @@ export class Step extends Evented {
       this.el = null;
     }
 
-    if (this.target) {
-      this._updateStepTargetOnHide();
-    }
+    this._updateStepTargetOnHide();
 
     this.trigger('destroy');
   }
@@ -182,9 +180,7 @@ export class Step extends Evented {
       this.el.hidden = true;
     }
 
-    if (this.target) {
-      this._updateStepTargetOnHide();
-    }
+    this._updateStepTargetOnHide();
 
     this.trigger('hide');
   }
@@ -432,11 +428,13 @@ export class Step extends Evented {
    * @private
    */
   _updateStepTargetOnHide() {
+    const target = this.target || document.body;
+
     if (this.options.highlightClass) {
-      this.target.classList.remove(this.options.highlightClass);
+      target.classList.remove(this.options.highlightClass);
     }
 
-    this.target.classList.remove(
+    target.classList.remove(
       `${this.classPrefix}shepherd-enabled`,
       `${this.classPrefix}shepherd-target`
     );

diff --git a/test/unit/step.spec.js b/test/unit/step.spec.js
@@ -316,7 +316,7 @@ describe('Tour | Step', () => {
       step.destroy();
     });
 
-    it('should update passed in properties', async() => {
+    it('should update passed in properties', async () => {
       step.updateStepOptions({ text: 'updated', title: 'New title' });
 
       expect(step.options.text).toBe('updated');
@@ -337,7 +337,7 @@ describe('Tour | Step', () => {
       );
     });
 
-    it('should not affect other properties', async() => {
+    it('should not affect other properties', async () => {
       step.updateStepOptions({ text: 'updated', title: 'New title' });
       expect(step.options.id).toEqual('test-id');
       expect(step.options.buttons).toEqual([
@@ -356,7 +356,7 @@ describe('Tour | Step', () => {
       expect(document.querySelector('.button2').textContent).toBe('button two');
     });
 
-    it('should update buttons', async() => {
+    it('should update buttons', async () => {
       const buttons = [
         { text: 'button one updated', disabled: true, classes: 'button1' },
         { text: 'button two updated', disabled: false, classes: 'button2' }
@@ -381,7 +381,7 @@ describe('Tour | Step', () => {
       expect(buttonTwo.disabled).toBe(false);
     });
 
-    it('removing title should remove class', async() => {
+    it('removing title should remove class', async () => {
       step.updateStepOptions({ title: '' });
       expect(step.options.title).toEqual('');
 
@@ -396,7 +396,7 @@ describe('Tour | Step', () => {
       expect(element.classList.contains('shepherd-has-title')).toBeFalsy();
     });
 
-    it('updating classes should update element classes', async() => {
+    it('updating classes should update element classes', async () => {
       step.updateStepOptions({ classes: 'test-1 test-2' });
       expect(step.options.classes).toEqual('test-1 test-2');
 
@@ -514,4 +514,65 @@ describe('Tour | Step', () => {
       );
     });
   });
+
+  describe('correct operation of classes on body element when step not attached to an element', () => {
+    const instance = new Shepherd.Tour({
+      defaultStepOptions: {
+        classes: DEFAULT_STEP_CLASS,
+        scrollTo: true,
+        popperOptions: {
+          modifiers: [{ name: 'offset', options: { offset: [0, 32] } }]
+        },
+        showOn,
+        when
+      }
+    });
+
+    const stepWithoutAttachment = instance.addStep({
+      highlightClass: 'highlight',
+      text: 'This is a step for testing',
+      buttons: [
+        {
+          text: 'Next',
+          action: instance.next
+        }
+      ],
+      id: 'test',
+      popperOptions: {
+        modifiers: [{ name: 'foo', options: 'bar' }]
+      }
+    });
+
+    afterEach(() => {
+      instance.complete();
+    });
+
+    describe('.hide()', () => {
+      it('detaches from the step target', () => {
+        instance.start();
+
+        const targetElem = document.body;
+
+        expect(targetElem.classList.contains('shepherd-enabled')).toBe(true);
+
+        stepWithoutAttachment.hide();
+
+        expect(targetElem.classList.contains('shepherd-enabled')).toBe(false);
+      });
+    });
+
+    describe('.destroy()', () => {
+      it('detaches from the step target', () => {
+        instance.start();
+
+        const targetElem = document.body;
+
+        expect(targetElem.classList.contains('shepherd-enabled')).toBe(true);
+
+        stepWithoutAttachment.destroy();
+
+        expect(targetElem.classList.contains('shepherd-enabled')).toBe(false);
+      });
+    });
+  });
 });