From a96fcbb5484e7a7f40b2a95b452480390465de5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20B=C3=B6hler?= Date: Wed, 21 Oct 2026 07:31:34 +0200 Subject: [PATCH] fix(185122): don't anchor product tour steps to hidden elements A stored step selector that only matches a display:none node (a collapsed menu's label, the hidden copy of a responsive menu) was treated as found: the popover pinned to the viewport corner, the overlay blocked the page and a CLICK step had no way forward, so the visitor was stuck until the tour was unpublished (and reloads resumed it). Resolve steps to the first rendered match instead; with none, the existing 5s wait ends in onElementNotFound. Co-Authored-By: Claude Opus 5.5 --- src/GleapTours.js | 19 ++++++-- src/GleapTours.test.js | 106 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 src/GleapTours.test.js diff --git a/src/GleapTours.js b/src/GleapTours.js index df773488..6918d8aa 100644 --- a/src/GleapTours.js +++ b/src/GleapTours.js @@ -1,4 +1,4 @@ -import { querySelectorSafe } from './GleapSafeSelector'; +import { querySelectorAllSafe } from './GleapSafeSelector'; const GleapTours = (function () { 'use strict'; @@ -74,6 +74,19 @@ const GleapTours = (function () { function isElementVisible(el) { return !!(el.offsetWidth || el.offsetHeight || el.getClientRects().length); } + // A selector recorded on one layout can match a display:none node on another + // (a collapsed menu's label, the hidden copy of a responsive menu). Anchoring a + // step to it pins the popover to the viewport corner while the overlay blocks + // the page, so only rendered matches count. + function findRenderedElement(selector) { + const matches = querySelectorAllSafe(selector); + for (let i = 0; i < matches.length; i++) { + if (isElementVisible(matches[i])) { + return matches[i]; + } + } + return null; + } let currentState = {}; function setState(key, value) { currentState[key] = value; @@ -238,7 +251,7 @@ const GleapTours = (function () { if (typeof elemObj === 'string') { // Stored selectors can be hand typed and invalid; that has to end in // onElementNotFound below instead of an exception inside the retry timer. - elemObj = querySelectorSafe(element); + elemObj = findRenderedElement(element); } if (element && !elemObj && attemptTime >= 0) { setTimeout(() => { @@ -273,7 +286,7 @@ const GleapTours = (function () { // Re-query the element if the cached reference is no longer in the DOM. if (!document.body.contains(activeHighlight) && activeStep.element && typeof activeStep.element === 'string') { try { - var freshEl = querySelectorSafe(activeStep.element); + var freshEl = findRenderedElement(activeStep.element); if (freshEl) { activeHighlight = freshEl; setState('__activeElement', freshEl); diff --git a/src/GleapTours.test.js b/src/GleapTours.test.js new file mode 100644 index 00000000..81cb417d --- /dev/null +++ b/src/GleapTours.test.js @@ -0,0 +1,106 @@ +/** + * @jest-environment jsdom + */ +import GleapTours from './GleapTours'; + +// jsdom has no layout, so every element reports no client rects. Treat an +// element as rendered unless it or an ancestor is display:none, which is what +// a browser does for v-show / [hidden] / collapsed-menu labels. +const isHiddenInTest = (el) => { + for (let node = el; node && node.style; node = node.parentElement) { + if (node.style.display === 'none') { + return true; + } + } + return false; +}; + +describe('GleapTours step target resolution', () => { + let tour; + let onElementNotFound; + + beforeEach(() => { + jest.useFakeTimers(); + jest.spyOn(Element.prototype, 'getClientRects').mockImplementation(function () { + return isHiddenInTest(this) ? [] : [{ top: 0, left: 0, width: 40, height: 20 }]; + }); + onElementNotFound = jest.fn(() => tour.destroy()); + }); + + afterEach(() => { + if (tour) { + tour.destroy(); + } + tour = null; + jest.runOnlyPendingTimers(); + jest.useRealTimers(); + jest.restoreAllMocks(); + document.body.innerHTML = ''; + }); + + const startTour = (selector) => { + tour = GleapTours({ + animate: false, + allowClose: false, + onElementNotFound, + steps: [{ element: selector, popover: { description: 'Click on Library', showButtons: [] } }], + }); + tour.drive(); + }; + + it('highlights a rendered target right away', () => { + document.body.innerHTML = ''; + startTour('.menu-item > .label'); + + expect(tour.getActiveElement()).toBe(document.querySelector('.label')); + expect(onElementNotFound).not.toHaveBeenCalled(); + }); + + it('treats a target that only matches a display:none element as not found (#185122)', () => { + // Collapsed sidebar: the label the step was recorded on is hidden via v-show. + document.body.innerHTML = + ''; + startTour('.menu-item > .label'); + + expect(tour.getActiveElement()).toBeUndefined(); + expect(document.querySelector('.label').classList.contains('gleap-tour-active-element')).toBe(false); + + jest.advanceTimersByTime(5200); + + expect(onElementNotFound).toHaveBeenCalledTimes(1); + expect(onElementNotFound.mock.calls[0][0].element).toBe('.menu-item > .label'); + }); + + it('skips hidden matches and highlights the first rendered one', () => { + // Responsive layouts render the same menu twice and hide one of them. + document.body.innerHTML = + '' + + ''; + startTour('.menu-item > .label'); + + expect(tour.getActiveElement()).toBe(document.querySelector('.desktop .label')); + expect(onElementNotFound).not.toHaveBeenCalled(); + }); + + it('keeps waiting for a hidden target that becomes visible within the timeout', () => { + document.body.innerHTML = ''; + startTour('.menu-item > .label'); + + jest.advanceTimersByTime(1000); + document.querySelector('.label').style.display = ''; + jest.advanceTimersByTime(200); + + expect(tour.getActiveElement()).toBe(document.querySelector('.label')); + jest.advanceTimersByTime(5000); + expect(onElementNotFound).not.toHaveBeenCalled(); + }); + + it('still quits when the selector matches nothing', () => { + document.body.innerHTML = ''; + startTour('.menu-item > .label'); + + jest.advanceTimersByTime(5200); + + expect(onElementNotFound).toHaveBeenCalledTimes(1); + }); +});