Restore attachTo tabindex when tour is hidden (#3351)

Author: RobbieTheWagner

shepherd.js/src/components/shepherd-element.svelte

@@ -44,6 +44,7 @@
     const attachTo = step._getResolvedAttachToOptions();
     if (attachTo?.element) {
       attachToElement = attachTo.element;
+      step._storeOriginalTabIndex(attachToElement);
       attachToElement.tabIndex = 0;
       focusableAttachToElements = [
         attachToElement,

shepherd.js/src/step.ts

@@ -305,6 +305,7 @@ export interface StepOptionsWhen {
 export class Step extends Evented {
   _resolvedAttachTo: StepOptionsAttachTo | null;
   _resolvedExtraHighlightElements?: HTMLElement[];
+  _originalTabIndexes: Map<Element, string>;
   classPrefix?: string;
   declare cleanup: Function | null;
   el?: HTMLElement | null;
@@ -331,6 +332,13 @@ export class Step extends Evented {
      */
     this._resolvedAttachTo = null;
 
+    /**
+     * Map to store original tabIndex values of elements that are modified during the tour.
+     * @type {Map<Element, string | null>}
+     * @private
+     */
+    this._originalTabIndexes = new Map();
+
     autoBind(this);
 
     this._setOptions(options);
@@ -369,6 +377,7 @@ export class Step extends Evented {
     }
 
     this._updateStepTargetOnHide();
+    this._originalTabIndexes.clear();
 
     this.trigger('destroy');
   }
@@ -480,6 +489,37 @@ export class Step extends Evented {
     return this.target;
   }
 
+  /**
+   * Stores the original tabIndex value of an element before modifying it.
+   * Only stores the value if the element has a tabindex attribute.
+   * @param {Element} element The element whose tabIndex will be stored
+   * @private
+   */
+  _storeOriginalTabIndex(element: Element): void {
+    const originalValue = element.getAttribute('tabindex');
+    if (originalValue !== null) {
+      this._originalTabIndexes.set(element, originalValue);
+    }
+  }
+
+  /**
+   * Restores the original tabIndex values for all elements that were modified during the tour.
+   * If an element is in the map, restores its original value.
+   * If an element is not in the map, removes the tabindex attribute (it didn't have one originally).
+   * Note: Does not clear the map to allow for multiple show/hide cycles.
+   * @private
+   */
+  _restoreOriginalTabIndexes(): void {
+    const target = this.target;
+    if (target) {
+      if (this._originalTabIndexes.has(target)) {
+        target.setAttribute('tabindex', this._originalTabIndexes.get(target)!);
+      } else {
+        target.removeAttribute('tabindex');
+      }
+    }
+  }
+
   /**
    * Creates Shepherd element for step based on options
    *
@@ -722,5 +762,7 @@ export class Step extends Evented {
         `${this.classPrefix}shepherd-target`
       );
     });
+
+    this._restoreOriginalTabIndexes();
   }
 }

shepherd.js/test/unit/step.spec.js

@@ -780,4 +780,141 @@ describe('Tour | Step', () => {
       });
     });
   });
+
+  describe('tabIndex preservation', () => {
+    let instance;
+    let testElement;
+
+    beforeEach(() => {
+      // Create a test element
+      testElement = document.createElement('div');
+      testElement.id = 'tabindex-test-element';
+      document.body.appendChild(testElement);
+
+      instance = new Shepherd.Tour({
+        steps: [
+          {
+            id: 'test-step',
+            text: 'Test step',
+            attachTo: { element: '#tabindex-test-element', on: 'top' }
+          }
+        ]
+      });
+    });
+
+    afterEach(() => {
+      instance.complete();
+      testElement?.remove();
+    });
+
+    it('stores and restores original tabIndex when element has no tabindex attribute', () => {
+      // Initially, the element should have no tabindex attribute
+      expect(testElement.hasAttribute('tabindex')).toBe(false);
+
+      // Start the tour
+      instance.start();
+
+      // During the tour, tabIndex should be set to 0
+      expect(testElement.tabIndex).toBe(0);
+      expect(testElement.getAttribute('tabindex')).toBe('0');
+
+      // Hide the step
+      instance.getCurrentStep().hide();
+
+      // After hiding, the tabindex attribute should be removed
+      expect(testElement.hasAttribute('tabindex')).toBe(false);
+    });
+
+    it('stores and restores original tabIndex when element has tabindex="-1"', () => {
+      // Set tabindex to -1 initially
+      testElement.setAttribute('tabindex', '-1');
+      expect(testElement.getAttribute('tabindex')).toBe('-1');
+
+      // Start the tour
+      instance.start();
+
+      // During the tour, tabIndex should be set to 0
+      expect(testElement.tabIndex).toBe(0);
+      expect(testElement.getAttribute('tabindex')).toBe('0');
+
+      // Hide the step
+      instance.getCurrentStep().hide();
+
+      // After hiding, tabIndex should be restored to -1
+      expect(testElement.getAttribute('tabindex')).toBe('-1');
+    });
+
+    it('stores and restores original tabIndex when element has tabindex="5"', () => {
+      // Set tabindex to 5 initially
+      testElement.setAttribute('tabindex', '5');
+      expect(testElement.getAttribute('tabindex')).toBe('5');
+
+      // Start the tour
+      instance.start();
+
+      // During the tour, tabIndex should be set to 0
+      expect(testElement.tabIndex).toBe(0);
+      expect(testElement.getAttribute('tabindex')).toBe('0');
+
+      // Hide the step
+      instance.getCurrentStep().hide();
+
+      // After hiding, tabIndex should be restored to 5
+      expect(testElement.getAttribute('tabindex')).toBe('5');
+    });
+
+    it('restores tabIndex when step is destroyed', () => {
+      // Set tabindex to -1 initially
+      testElement.setAttribute('tabindex', '-1');
+
+      // Start the tour
+      instance.start();
+
+      // During the tour, tabIndex should be set to 0
+      expect(testElement.getAttribute('tabindex')).toBe('0');
+
+      // Destroy the step
+      instance.getCurrentStep().destroy();
+
+      // After destroying, tabIndex should be restored to -1
+      expect(testElement.getAttribute('tabindex')).toBe('-1');
+    });
+
+    it('handles multiple show/hide cycles correctly', () => {
+      // Set tabindex to 2 initially
+      testElement.setAttribute('tabindex', '2');
+
+      // Start the tour (first show)
+      instance.start();
+      expect(testElement.getAttribute('tabindex')).toBe('0');
+
+      // Hide the step
+      instance.getCurrentStep().hide();
+      expect(testElement.getAttribute('tabindex')).toBe('2');
+
+      // Show again
+      instance.getCurrentStep().show();
+      expect(testElement.getAttribute('tabindex')).toBe('0');
+
+      // Hide again
+      instance.getCurrentStep().hide();
+      expect(testElement.getAttribute('tabindex')).toBe('2');
+    });
+
+    it('only stores the original value once, not intermediate values', () => {
+      // Set tabindex to 3 initially
+      testElement.setAttribute('tabindex', '3');
+
+      // Start the tour
+      instance.start();
+      expect(testElement.getAttribute('tabindex')).toBe('0');
+
+      // Manually change tabIndex (simulating some other code changing it)
+      testElement.setAttribute('tabindex', '7');
+
+      // Hide the step - should restore to original value (3), not intermediate (7)
+      instance.getCurrentStep().hide();
+      expect(testElement.getAttribute('tabindex')).toBe('3');
+    });
+  });
 });