From 37576082adc79c1b9c66acfce13e1408fed81aba Mon Sep 17 00:00:00 2001 From: Chuck Carpenter Date: Thu, 13 Aug 2026 14:10:20 +0200 Subject: [PATCH] fix: make canClickTarget work when a classPrefix is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Step` adds `shepherd-enabled` and `shepherd-target` to the target with the tour's `classPrefix`, but adds `shepherd-target-click-disabled` unprefixed. The rule that implements click blocking required all three unprefixed on the same element, so with `classPrefix: 'my-tour-'` the target ended up with `shepherd-target-click-disabled my-tour-shepherd-enabled my-tour-shepherd-target`, the selector could not match, and `canClickTarget: false` silently did nothing. Key the rule solely on `shepherd-target-click-disabled`. That class is already unprefixed and is added only when `canClickTarget === false`, so the other two carried no semantic weight — they were only ever incidental. A static stylesheet cannot know the runtime prefix, so dropping them is the fix. The class is repeated three times to hold the selector at its original 0-3-0 specificity, leaving cascade weight unchanged for anyone already overriding it; cssnano preserves the repetition, so the shipped dist/css/shepherd.css keeps 0-3-0 as well. The cypress tests attach to a new fixture in the dummy page whose target and child each carry a competing `pointer-events: auto` declaration — 0-2-0 on the target, 0-1-0 on the child. Without that competition neither half of the rule is actually under test: `pointer-events` is inherited, so a plain child of a blocked target computes `none` even with the `... *` clause deleted, and nothing else on the page contests the specificity. With the fixture, collapsing the repeated class or deleting the descendant clause each turns the tests red. Three caveats worth knowing, since the bug has been latent since 2021: - An app that set `canClickTarget: false` under a `classPrefix` and came to rely on the target staying clickable will now find it blocked. The escape hatch is `canClickTarget: true`, or omitting the option. - Any element that application code manually tagged with `shepherd-target-click-disabled` outside a tour now gets `pointer-events: none`. That class is Shepherd-internal and named for exactly this effect. - A prefixed tour can now trap itself, exactly as an unprefixed one already could. If `stepsContainer` puts the popup inside the target, or an ancestor of the popup (`document.body`, say) is listed in `extraHighlights`, the descendant half of the rule kills the tour's own buttons. Those steps need `canClickTarget: true`. One pre-existing leak is disclosed rather than silently carried. Calling `Step#show()` directly — rather than through `Tour#show`, which hides the current step first — re-resolves `extraHighlights` before tearing the step down, so elements that have since dropped out of the selector keep the classes they were given. Under a prefix that leftover `shepherd-target-click-disabled` used to be inert and is now permanent `pointer-events: none`. The same call leaks `highlightClass` and the prefixed `shepherd-enabled`/`shepherd-target` identically, so it is a `_show` ordering bug that predates this change and wants its own commit, not a widening of a CSS selector fix. The `attachTo` target itself is not affected: `_setupElements` tears down while `step.target` still points at the old element, and only `setupTooltip` reassigns it. landing/src/styles/shepherd.css is a hand-vendored copy of the minified stylesheet that nothing in landing's build regenerates. Its selector is updated here too, so shepherdjs.dev stops shipping a stylesheet that still has the bug. Tours without a `classPrefix` are entirely unaffected: the new selector matches a strict superset of the old one at identical specificity. Fixes #1298 Co-Authored-By: Claude Opus 5 --- docs-src/src/content/docs/guides/styling.md | 7 + docs-src/src/content/docs/guides/usage.md | 10 +- landing/src/styles/shepherd.css | 2 +- .../src/components/shepherd-element.css | 20 ++- shepherd.js/src/step.ts | 4 + shepherd.js/src/tour.ts | 4 + .../test/cypress/dummy/css/welcome.css | 27 ++- shepherd.js/test/cypress/dummy/index.html | 17 ++ .../integration/element-targeting.cy.js | 83 ++++++++++ shepherd.js/test/unit/step.spec.js | 156 ++++++++++++++++++ 10 files changed, 324 insertions(+), 6 deletions(-) diff --git a/docs-src/src/content/docs/guides/styling.md b/docs-src/src/content/docs/guides/styling.md index b2fa035d3..7cb540147 100644 --- a/docs-src/src/content/docs/guides/styling.md +++ b/docs-src/src/content/docs/guides/styling.md @@ -30,3 +30,10 @@ const tour = new Shepherd.Tour({ classPrefix: 'my-tour-' }); ``` + +`classPrefix` applies to exactly two things: the `shepherd-enabled` and `shepherd-target` classes Shepherd puts on +the **target** element (and on any `extraHighlights` elements), and the `data-shepherd-step-id` attribute on the popup. +Everything else keeps its unprefixed name — including the `shepherd-enabled` class on the popup element itself, +and the `shepherd-target-click-disabled` class that `canClickTarget: false` adds to the target. `shepherd.css` keys +rules on those two, and a static stylesheet cannot know your runtime prefix, so prefixing them would break the rules +they drive. diff --git a/docs-src/src/content/docs/guides/usage.md b/docs-src/src/content/docs/guides/usage.md index 43c18852b..b43e408f9 100644 --- a/docs-src/src/content/docs/guides/usage.md +++ b/docs-src/src/content/docs/guides/usage.md @@ -125,7 +125,11 @@ const myTour = new Shepherd.Tour(options); ##### Tour Options - `classPrefix`: The prefix to add to the `shepherd-enabled` and - `shepherd-target` class names as well as the `data-shepherd-step-id`. + `shepherd-target` class names Shepherd puts on the **target** element, as well + as to the `data-shepherd-step-id` attribute. Nothing else is prefixed: the + popup keeps its own unprefixed `shepherd-enabled` class, and + `shepherd-target-click-disabled` stays unprefixed too, because the shipped + stylesheet keys click blocking on it and cannot know your runtime prefix. - `confirmCancel`: - If true, will issue a `window.confirm` before cancelling - If it is a function(support Async Function), it will be called and wait for @@ -241,7 +245,9 @@ function will be called in the `before-show` phase. }, ``` - `canClickTarget` A boolean, that when set to false, will set - `pointer-events: none` on the target + `pointer-events: none` on the target. The blocking is delivered by + `shepherd.css`, so it has no effect if you have opted out of Shepherd's + stylesheet without providing an equivalent rule. - `cancelIcon` Options for the cancel icon - `attrs` Additional HTML attributes to apply to the cancel icon button element. This is useful for adding data attributes for testing or analytics. diff --git a/landing/src/styles/shepherd.css b/landing/src/styles/shepherd.css index 9f8491658..45a72c867 100644 --- a/landing/src/styles/shepherd.css +++ b/landing/src/styles/shepherd.css @@ -5,5 +5,5 @@ .shepherd-header{align-items:center;border-top-left-radius:5px;border-top-right-radius:5px;display:flex;justify-content:flex-end;line-height:2em;padding:.75rem .75rem 0}.shepherd-has-title .shepherd-content .shepherd-header{background:#e6e6e6;padding:1em} .shepherd-text{color:rgba(0,0,0,.75);font-size:1rem;line-height:1.3em;padding:.75em}.shepherd-text p{margin-top:0}.shepherd-text p:last-child{margin-bottom:0} .shepherd-content{border-radius:5px;outline:none;padding:0} -.shepherd-element{background:#fff;border-radius:5px;box-shadow:0 1px 4px rgba(0,0,0,.2);margin: 0;max-width:400px;opacity:0;outline:none;transition:opacity .3s,visibility .3s;visibility:hidden;width:100%;z-index:9999}.shepherd-enabled.shepherd-element{opacity:1;visibility:visible}.shepherd-element[data-popper-reference-hidden]:not(.shepherd-centered){opacity:0;pointer-events:none;visibility:hidden}.shepherd-element,.shepherd-element *,.shepherd-element :after,.shepherd-element :before{box-sizing:border-box}.shepherd-arrow,.shepherd-arrow:before{height:16px;position:absolute;width:16px;z-index:-1}.shepherd-arrow:before{background:#fff;content:"";transform:rotate(45deg)}.shepherd-element[data-popper-placement^=top]>.shepherd-arrow{bottom:-8px}.shepherd-element[data-popper-placement^=bottom]>.shepherd-arrow{top:-8px}.shepherd-element[data-popper-placement^=left]>.shepherd-arrow{right:-8px}.shepherd-element[data-popper-placement^=right]>.shepherd-arrow{left:-8px}.shepherd-element.shepherd-centered>.shepherd-arrow{opacity:0}.shepherd-element.shepherd-has-title[data-popper-placement^=bottom]>.shepherd-arrow:before{background-color:#e6e6e6}.shepherd-target-click-disabled.shepherd-enabled.shepherd-target,.shepherd-target-click-disabled.shepherd-enabled.shepherd-target *{pointer-events:none} +.shepherd-element{background:#fff;border-radius:5px;box-shadow:0 1px 4px rgba(0,0,0,.2);margin: 0;max-width:400px;opacity:0;outline:none;transition:opacity .3s,visibility .3s;visibility:hidden;width:100%;z-index:9999}.shepherd-enabled.shepherd-element{opacity:1;visibility:visible}.shepherd-element[data-popper-reference-hidden]:not(.shepherd-centered){opacity:0;pointer-events:none;visibility:hidden}.shepherd-element,.shepherd-element *,.shepherd-element :after,.shepherd-element :before{box-sizing:border-box}.shepherd-arrow,.shepherd-arrow:before{height:16px;position:absolute;width:16px;z-index:-1}.shepherd-arrow:before{background:#fff;content:"";transform:rotate(45deg)}.shepherd-element[data-popper-placement^=top]>.shepherd-arrow{bottom:-8px}.shepherd-element[data-popper-placement^=bottom]>.shepherd-arrow{top:-8px}.shepherd-element[data-popper-placement^=left]>.shepherd-arrow{right:-8px}.shepherd-element[data-popper-placement^=right]>.shepherd-arrow{left:-8px}.shepherd-element.shepherd-centered>.shepherd-arrow{opacity:0}.shepherd-element.shepherd-has-title[data-popper-placement^=bottom]>.shepherd-arrow:before{background-color:#e6e6e6}.shepherd-target-click-disabled.shepherd-target-click-disabled.shepherd-target-click-disabled,.shepherd-target-click-disabled.shepherd-target-click-disabled.shepherd-target-click-disabled *{pointer-events:none} .shepherd-modal-overlay-container{height:0;left:0;opacity:0;overflow:hidden;pointer-events:none;position:fixed;top:0;transition:all .3s ease-out,height 0ms .3s,opacity .3s 0ms;width:100vw;z-index:9997}.shepherd-modal-overlay-container.shepherd-modal-is-visible{height:100vh;opacity:.5;transform:translateZ(0);transition:all .3s ease-out,height 0s 0s,opacity .3s 0s}.shepherd-modal-overlay-container.shepherd-modal-is-visible path{pointer-events:all} \ No newline at end of file diff --git a/shepherd.js/src/components/shepherd-element.css b/shepherd.js/src/components/shepherd-element.css index 6bc841d30..adf053f98 100644 --- a/shepherd.js/src/components/shepherd-element.css +++ b/shepherd.js/src/components/shepherd-element.css @@ -76,7 +76,23 @@ background-color: #e6e6e6; } -.shepherd-target-click-disabled.shepherd-enabled.shepherd-target, -.shepherd-target-click-disabled.shepherd-enabled.shepherd-target * { +/** +* Blocks clicks on the target of a step with `canClickTarget: false`. +* +* Keyed only on `shepherd-target-click-disabled`, which +* `Step#_styleTargetElementForStep` adds unprefixed, and only when +* `canClickTarget === false`. The sibling +* `shepherd-enabled`/`shepherd-target` classes cannot be part of this selector: +* `classPrefix` prefixes them at runtime and a static stylesheet cannot know +* the prefix, so requiring them silently disabled click blocking for every tour +* using `classPrefix` (#1298). +* +* The class is repeated three times on purpose, to preserve this selector's +* original 0-3-0 specificity so that existing overrides keep winning. Do not +* "clean this up" to a single class without reading #1298 first. +*/ +.shepherd-target-click-disabled.shepherd-target-click-disabled.shepherd-target-click-disabled, +.shepherd-target-click-disabled.shepherd-target-click-disabled.shepherd-target-click-disabled + * { pointer-events: none; } diff --git a/shepherd.js/src/step.ts b/shepherd.js/src/step.ts index c8804a265..649113b6e 100644 --- a/shepherd.js/src/step.ts +++ b/shepherd.js/src/step.ts @@ -88,6 +88,10 @@ export interface StepOptions { /** * A boolean, that when set to false, will set `pointer-events: none` on the target. + * + * The blocking is delivered by `shepherd.css` (via the `shepherd-target-click-disabled` + * class), so it has no effect if you have opted out of Shepherd's stylesheet without + * providing an equivalent rule. */ canClickTarget?: boolean; diff --git a/shepherd.js/src/tour.ts b/shepherd.js/src/tour.ts index d1e4824df..e182148c5 100644 --- a/shepherd.js/src/tour.ts +++ b/shepherd.js/src/tour.ts @@ -44,6 +44,10 @@ export interface TourOptions { confirmCancelMessage?: string; /** * The prefix to add to the `shepherd-enabled` and `shepherd-target` class names as well as the `data-shepherd-step-id`. + * + * Only those apply. The popup keeps its own unprefixed `shepherd-enabled` class, and + * `shepherd-target-click-disabled` is never prefixed either, since `shepherd.css` keys + * click blocking on it and cannot know the runtime prefix. */ classPrefix?: string; /** diff --git a/shepherd.js/test/cypress/dummy/css/welcome.css b/shepherd.js/test/cypress/dummy/css/welcome.css index 8b9e2ac57..fa01b585b 100644 --- a/shepherd.js/test/cypress/dummy/css/welcome.css +++ b/shepherd.js/test/cypress/dummy/css/welcome.css @@ -120,4 +120,29 @@ pre { .shepherd-text a:visited:hover, .shepherd-text a:active:hover { border-bottom-style: solid; -} \ No newline at end of file +} + +/* + * Fixtures for the `canClickTarget: false` click-blocking tests (#1298). + * + * These rules deliberately COMPETE with the click-blocking rule in + * shepherd.css, so that the cypress assertions exercise the shipped selector + * rather than passing for free. This file is linked after shepherd.css, so at + * equal specificity these win. + * + * - `.click-block-fixture.click-block-fixture-override` is 0-2-0 on the target. + * It loses to the click-blocking rule only while that rule holds its 0-3-0 + * specificity. Collapse the repeated class in shepherd-element.css and the + * target computes `auto` again. + * - `.click-block-fixture-child` gives the child its own `pointer-events` + * value, so the child cannot simply inherit `none` from the target. It loses + * only to the descendant (`... *`) half of the click-blocking rule. Delete + * that half and the child computes `auto` again. + */ +.click-block-fixture.click-block-fixture-override { + pointer-events: auto; +} + +.click-block-fixture-child { + pointer-events: auto; +} diff --git a/shepherd.js/test/cypress/dummy/index.html b/shepherd.js/test/cypress/dummy/index.html index 7bef4f966..e1492c801 100644 --- a/shepherd.js/test/cypress/dummy/index.html +++ b/shepherd.js/test/cypress/dummy/index.html @@ -126,6 +126,23 @@

Example

+ + +
+ Click blocking fixture +
diff --git a/shepherd.js/test/cypress/integration/element-targeting.cy.js b/shepherd.js/test/cypress/integration/element-targeting.cy.js index cf73ad3fa..686c842f6 100644 --- a/shepherd.js/test/cypress/integration/element-targeting.cy.js +++ b/shepherd.js/test/cypress/integration/element-targeting.cy.js @@ -44,6 +44,89 @@ describe('Attaching tooltips to target elements in the DOM on each step', () => }); }); + describe('Blocking clicks on the target with `canClickTarget: false`', () => { + let tour; + + // A single step attached to the click-blocking fixture in the dummy page. + // + // That fixture is used instead of `.hero-welcome` on purpose. `pointer-events` + // is an inherited property, so a child of a blocked target computes `none` + // even with the descendant half of the rule deleted; and nothing on the page + // competes with the rule's specificity. The fixture and its child each carry + // their own competing `pointer-events: auto` declaration (see + // dummy/css/welcome.css), so these assertions fail if the shipped rule loses + // either its `... *` half or its 0-3-0 specificity. + const clickBlockStep = (shepherd) => [ + { + id: 'click-block', + text: 'Click blocking fixture step', + attachTo: { + element: '[data-test-click-block-target]', + on: 'top' + }, + buttons: [ + { + action: shepherd.cancel, + text: 'Exit' + } + ] + } + ]; + + afterEach(() => { + tour.complete(); + }); + + it('leaves the target and its children clickable when `canClickTarget` is not set', () => { + tour = setupTour(Shepherd, {}, clickBlockStep); + tour.start(); + + cy.get('[data-test-click-block-target]') + .should('have.class', 'shepherd-target') + .and('have.css', 'pointer-events', 'auto'); + cy.get('[data-test-click-block-child]').should( + 'have.css', + 'pointer-events', + 'auto' + ); + }); + + it('blocks clicks on the target and its children when no classPrefix is set', () => { + tour = setupTour(Shepherd, { canClickTarget: false }, clickBlockStep); + tour.start(); + + cy.get('[data-test-click-block-target]').should( + 'have.css', + 'pointer-events', + 'none' + ); + cy.get('[data-test-click-block-child]').should( + 'have.css', + 'pointer-events', + 'none' + ); + }); + + // Regression test for #1298: `classPrefix` prefixes the `shepherd-enabled` + // and `shepherd-target` classes, which the click-blocking rule used to + // require, so `canClickTarget: false` silently did nothing. + it('blocks clicks on the target and its children when a classPrefix is set', () => { + tour = setupTour(Shepherd, { canClickTarget: false }, clickBlockStep, { + classPrefix: 'my-tour-' + }); + tour.start(); + + cy.get('[data-test-click-block-target]') + .should('have.class', 'my-tour-shepherd-target') + .and('have.css', 'pointer-events', 'none'); + cy.get('[data-test-click-block-child]').should( + 'have.css', + 'pointer-events', + 'none' + ); + }); + }); + describe('Unique selectors with multiple Tours', function () { let firstTour, secondTour; diff --git a/shepherd.js/test/unit/step.spec.js b/shepherd.js/test/unit/step.spec.js index 094236656..ef8e6e2a1 100644 --- a/shepherd.js/test/unit/step.spec.js +++ b/shepherd.js/test/unit/step.spec.js @@ -663,6 +663,162 @@ describe('Tour | Step', () => { }); }); + /** + * The click-blocking CSS rule keys solely on the unprefixed + * `shepherd-target-click-disabled` class, because `classPrefix` prefixes the + * sibling `shepherd-enabled`/`shepherd-target` classes at runtime and a + * static stylesheet cannot know the prefix (#1298). These tests pin which + * elements carry that class and when. + * + * They deliberately do not assert computed `pointer-events`: this suite runs + * in happy-dom and never loads `shepherd.css`, so such an assertion would + * pass vacuously. The cascade half of the fix — the `... *` descendant + * clause and the 0-3-0 specificity — is pinned in + * `test/cypress/integration/element-targeting.cy.js` against a fixture that + * competes with both. + */ + describe('canClickTarget with classPrefix', () => { + const CLICK_DISABLED = 'shepherd-target-click-disabled'; + let instance, targetElem, extraElem; + + function buildTour(stepOptions) { + instance = new Shepherd.Tour({ classPrefix: 'my-tour-' }); + instance.addStep({ + id: 'test', + text: 'This is a step for testing', + attachTo: { element: '.click-disabled-target', on: 'top' }, + ...stepOptions + }); + return instance; + } + + beforeEach(() => { + targetElem = document.createElement('div'); + targetElem.classList.add('click-disabled-target'); + document.body.appendChild(targetElem); + + extraElem = document.createElement('div'); + extraElem.classList.add('click-disabled-extra'); + document.body.appendChild(extraElem); + }); + + afterEach(() => { + instance?.complete(); + instance = null; + targetElem.remove(); + extraElem.remove(); + }); + + it('adds `shepherd-target-click-disabled` unprefixed, alongside the prefixed classes', () => { + buildTour({ canClickTarget: false }).start(); + + expect(targetElem.classList.contains(CLICK_DISABLED)).toBe(true); + expect(targetElem.classList.contains('my-tour-shepherd-enabled')).toBe( + true + ); + expect(targetElem.classList.contains('my-tour-shepherd-target')).toBe( + true + ); + }); + + it('removes all three classes on `hide()`', () => { + buildTour({ canClickTarget: false }).start(); + instance.getCurrentStep().hide(); + + expect(targetElem.classList.contains(CLICK_DISABLED)).toBe(false); + expect(targetElem.classList.contains('my-tour-shepherd-enabled')).toBe( + false + ); + expect(targetElem.classList.contains('my-tour-shepherd-target')).toBe( + false + ); + }); + + it('removes all three classes on `destroy()`', () => { + buildTour({ canClickTarget: false }).start(); + instance.getCurrentStep().destroy(); + + expect(targetElem.classList.contains(CLICK_DISABLED)).toBe(false); + expect(targetElem.classList.contains('my-tour-shepherd-enabled')).toBe( + false + ); + expect(targetElem.classList.contains('my-tour-shepherd-target')).toBe( + false + ); + }); + + // The class now carries the whole meaning of the CSS rule, so it must not + // appear on targets that never opted in: any element wearing it is + // unclickable. + it('does not add `shepherd-target-click-disabled` when `canClickTarget` is unset', () => { + buildTour({}).start(); + + expect(targetElem.classList.contains('my-tour-shepherd-target')).toBe( + true + ); + expect(targetElem.classList.contains(CLICK_DISABLED)).toBe(false); + }); + + it('does not add `shepherd-target-click-disabled` when `canClickTarget` is true', () => { + buildTour({ canClickTarget: true }).start(); + + expect(targetElem.classList.contains('my-tour-shepherd-target')).toBe( + true + ); + expect(targetElem.classList.contains(CLICK_DISABLED)).toBe(false); + }); + + it('adds `shepherd-target-click-disabled` to `extraHighlights` elements too', () => { + buildTour({ + canClickTarget: false, + extraHighlights: ['.click-disabled-extra'] + }).start(); + + expect(extraElem.classList.contains(CLICK_DISABLED)).toBe(true); + expect(extraElem.classList.contains('my-tour-shepherd-target')).toBe( + true + ); + }); + + it('does not add `shepherd-target-click-disabled` to `extraHighlights` elements when `canClickTarget` is unset', () => { + buildTour({ extraHighlights: ['.click-disabled-extra'] }).start(); + + expect(extraElem.classList.contains('my-tour-shepherd-target')).toBe( + true + ); + expect(extraElem.classList.contains(CLICK_DISABLED)).toBe(false); + }); + + it('removes `shepherd-target-click-disabled` from `extraHighlights` elements on `hide()`', () => { + buildTour({ + canClickTarget: false, + extraHighlights: ['.click-disabled-extra'] + }).start(); + expect(extraElem.classList.contains(CLICK_DISABLED)).toBe(true); + + instance.getCurrentStep().hide(); + + expect(extraElem.classList.contains(CLICK_DISABLED)).toBe(false); + expect(extraElem.classList.contains('my-tour-shepherd-target')).toBe( + false + ); + }); + + it('removes `shepherd-target-click-disabled` from `extraHighlights` elements when the tour completes', () => { + buildTour({ + canClickTarget: false, + extraHighlights: ['.click-disabled-extra'] + }).start(); + expect(extraElem.classList.contains(CLICK_DISABLED)).toBe(true); + + instance.complete(); + instance = null; + + expect(extraElem.classList.contains(CLICK_DISABLED)).toBe(false); + expect(targetElem.classList.contains(CLICK_DISABLED)).toBe(false); + }); + }); + describe('lazy attachTo evaluation', () => { // We test this using attachTo.element callback. // Note that lazy evaluation largely relies on `parseAttachTo`, however this does