From 7e61f014e121c7f6017eab513b41931ae04031b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:04:07 +0000 Subject: [PATCH 1/4] fix(modal): don't clip highlights by root elements whose overflow applies to the viewport `_isScrollable` treated as a scroll container whenever its computed overflow-y was `auto`/`scroll`. But while is `overflow: visible`, body's overflow propagates to the viewport and body's used overflow is `visible`, so it crops nothing. With body sized to the viewport (`height: 100%`), scrolling a target below the initial fold into view moved body's rect off-screen and the overlay opening collapsed to zero height, covering the target it was meant to highlight. is now never treated as a clipper (its overflow always applies to the viewport), and only when 's overflow is not `visible`. Fixes #1984 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011RqamMhESzSaWmRHLw2Vai --- shepherd.js/src/components/shepherd-modal.ts | 18 +++++ .../test/cypress/examples/root-overflow.html | 40 ++++++++++ .../cypress/integration/root-overflow.cy.js | 77 ++++++++++++++++++ .../test/cypress/utils/overlay-openings.js | 26 +++++++ .../unit/components/shepherd-modal.spec.js | 78 +++++++++++++++++++ 5 files changed, 239 insertions(+) create mode 100644 shepherd.js/test/cypress/examples/root-overflow.html create mode 100644 shepherd.js/test/cypress/integration/root-overflow.cy.js create mode 100644 shepherd.js/test/cypress/utils/overlay-openings.js diff --git a/shepherd.js/src/components/shepherd-modal.ts b/shepherd.js/src/components/shepherd-modal.ts index b7cf55318..f6cb93b5d 100644 --- a/shepherd.js/src/components/shepherd-modal.ts +++ b/shepherd.js/src/components/shepherd-modal.ts @@ -261,6 +261,24 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { * @param style `el`'s computed style, already resolved by the caller */ function _isScrollable(el: HTMLElement, style: CSSStyleDeclaration) { + const { documentElement, body } = el.ownerDocument; + + // The root element's overflow always applies to the viewport, never to its + // own box, so it does not crop anything against its own rect. + if (el === documentElement) return false; + + // While is `overflow: visible`, 's overflow propagates to the + // viewport as well and body's used overflow becomes `visible`, even though + // its computed value still reads `auto` / `scroll`. Clipping against + // body's rect then zeroes out targets the viewport has scrolled to as soon + // as body is sized to the viewport (e.g. `height: 100%`), see #1984. + if ( + el === body && + window.getComputedStyle(documentElement).overflowY === 'visible' + ) { + return false; + } + const { overflowY } = style; return ( diff --git a/shepherd.js/test/cypress/examples/root-overflow.html b/shepherd.js/test/cypress/examples/root-overflow.html new file mode 100644 index 000000000..399eb8d41 --- /dev/null +++ b/shepherd.js/test/cypress/examples/root-overflow.html @@ -0,0 +1,40 @@ + + + + + + + + + + + + + +
+
Target
+
+ + diff --git a/shepherd.js/test/cypress/integration/root-overflow.cy.js b/shepherd.js/test/cypress/integration/root-overflow.cy.js new file mode 100644 index 000000000..eb89584ec --- /dev/null +++ b/shepherd.js/test/cypress/integration/root-overflow.cy.js @@ -0,0 +1,77 @@ +import setupTour from '../utils/setup-tour'; +import overlayOpenings from '../utils/overlay-openings'; + +// End-to-end guard for #1984. Whether crops anything depends on overflow +// propagation to the viewport, which happy-dom has no layout engine for, so +// this is the only place the real behaviour is exercised. +describe('modal overlay with overflow set on the root elements', () => { + let Shepherd, tour; + + beforeEach(() => { + Shepherd = null; + + cy.visit('/test/cypress/examples/root-overflow', { + onLoad(contentWindow) { + if (contentWindow.Shepherd) { + return (Shepherd = contentWindow.Shepherd); + } + } + }); + }); + + afterEach(() => { + tour?.complete(); + }); + + const startTour = (rootStyles, scrollTo) => { + cy.document().then((doc) => { + doc.getElementById('root-styles').textContent = rootStyles; + + tour = setupTour( + Shepherd, + {}, + () => [ + { + attachTo: { element: '.target', on: 'bottom' }, + id: 'root-overflow', + text: 'Below the initial viewport', + scrollTo + } + ], + { useModalOverlay: true } + ); + tour.start(); + }); + cy.wait(250); + }; + + it('keeps the opening when body overflow propagates to the viewport', () => { + // is `overflow: visible`, so body's `auto` applies to the viewport + // and body itself crops nothing, even though it is viewport sized. + startTour('html, body { height: 100%; } body { overflow: auto; }', true); + + cy.document().then((doc) => { + const target = doc.querySelector('.target').getBoundingClientRect(); + const [opening] = overlayOpenings(doc); + + expect(target.top).to.be.within(0, doc.defaultView.innerHeight - 40); + expect(opening.y).to.be.closeTo(target.top, 1); + expect(opening.height).to.be.closeTo(40, 1); + }); + }); + + it('still clips by body when body is a scroll container of its own', () => { + // With no longer `visible`, body keeps its overflow and is a real + // scroll container whose rect crops the target below its fold. + startTour( + 'html { height: 100%; overflow: hidden; } body { height: 100%; overflow: auto; }', + false + ); + + cy.document().then((doc) => { + const [opening] = overlayOpenings(doc); + + expect(opening.height).to.equal(0); + }); + }); +}); diff --git a/shepherd.js/test/cypress/utils/overlay-openings.js b/shepherd.js/test/cypress/utils/overlay-openings.js new file mode 100644 index 000000000..ad1a2b5b7 --- /dev/null +++ b/shepherd.js/test/cypress/utils/overlay-openings.js @@ -0,0 +1,26 @@ +/** + * Reads the cutouts of the modal overlay back out of its svg path. + * + * The path is the full-viewport rect followed by one sub-path per opening, + * each starting `M{x + radius},{y}` and reaching its bottom edge with the + * first `V{y + height}`. Only valid for openings without a corner radius. + * + * @param {Document} doc The document the overlay is rendered in + * @returns {{ x: number, y: number, height: number }[]} + */ +export default function overlayOpenings(doc) { + const d = doc + .querySelector('.shepherd-modal-overlay-container path') + .getAttribute('d'); + + return d + .split('Z') + .slice(1) + .filter(Boolean) + .map((subPath) => { + const [, x, y, bottom] = subPath.match( + /^M([-\d.e]+),([-\d.e]+).*?V([-\d.e]+)/ + ); + return { x: Number(x), y: Number(y), height: Number(bottom) - Number(y) }; + }); +} diff --git a/shepherd.js/test/unit/components/shepherd-modal.spec.js b/shepherd.js/test/unit/components/shepherd-modal.spec.js index e6c5dee88..7c32ad458 100644 --- a/shepherd.js/test/unit/components/shepherd-modal.spec.js +++ b/shepherd.js/test/unit/components/shepherd-modal.spec.js @@ -975,6 +975,84 @@ describe('components/ShepherdModal', () => { modal.hide(); }); + // Regression coverage for https://github.com/shipshapecode/shepherd/issues/1984 + // The root element's overflow, and body's whenever is + // `overflow: visible`, applies to the viewport rather than to the + // element's own box, so neither may crop a highlight against its rect. + describe('root elements', () => { + afterEach(() => { + delete document.body.getBoundingClientRect; + delete document.documentElement.getBoundingClientRect; + }); + + // Body sized to the viewport (`height: 100%`) and scrolled entirely + // above it, while the highlight sits fully on screen at y 200-240. + function buildRootCase(overflows) { + const targetEl = makeChild(container, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + const extraEl = makeChild(container, { + x: 200, + y: 200, + width: 100, + height: 40 + }); + const offscreen = { x: 0, y: -1000, width: 1000, height: 700 }; + stubRect(document.body, offscreen); + stubRect(document.documentElement, offscreen); + + mockOverflow(overflows); + + return { targetEl, extraEl }; + } + + it('does not clip by body when its overflow propagates to the viewport', () => { + const modal = createShepherdModal(container); + const { targetEl, extraEl } = buildRootCase( + new Map([[document.body, 'auto']]) + ); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + expect(d).toContain('M200,200'); + expect(d).toContain('V240'); + }); + + it('never clips by the root element', () => { + const modal = createShepherdModal(container); + const { targetEl, extraEl } = buildRootCase( + new Map([[document.documentElement, 'auto']]) + ); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + expect(d).toContain('M200,200'); + expect(d).toContain('V240'); + }); + + it('still clips by body when body is its own scroll container', () => { + const modal = createShepherdModal(container); + // no longer `visible`, so body's overflow stays on body. + const { targetEl, extraEl } = buildRootCase( + new Map([ + [document.documentElement, 'hidden'], + [document.body, 'auto'] + ]) + ); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + // Clipped to body's bottom edge at y -300, so no height is left. + expect(d).not.toContain('V240'); + }); + }); + // A scrollable DOM ancestor only crops a descendant when it is in that // descendant's containing block chain. Resolving a scroll parent per // element made this matter: without the containing block check, an From 03bd6dd019b77aa0335ec56bcd90445f2224f0ca Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:07:42 +0000 Subject: [PATCH 2/4] fix(modal): treat overflow: hidden as cropping, skip inline and display: contents ancestors `_isScrollable` excluded `overflow-y: hidden`, so a highlight inside a collapsed accordion, carousel track or programmatically scrolled hidden pane got a full-size opening over content the user cannot see. It already accepted `clip`, which crops identically, so the handling was inconsistent. `hidden` is now accepted alongside `auto`, `scroll` and `clip`. To keep that from regressing boxes whose computed overflow does not crop: - the root elements are already handled (#1984), which covers a propagated `body { overflow: hidden }`; - non-replaced inlines (overflow does not apply) and `display: contents` (no box, 0x0 rect at the origin) are skipped. The `scrollHeight >= clientHeight` term is dropped: per CSSOM-View the scrolling area is at least the padding box, so it was true for every element and filtered nothing. Fixes #3484 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011RqamMhESzSaWmRHLw2Vai --- shepherd.js/src/components/shepherd-modal.ts | 17 ++-- .../cypress/examples/overflow-clipping.html | 83 +++++++++++++++++++ .../integration/overflow-clipping.cy.js | 76 +++++++++++++++++ .../cypress/integration/root-overflow.cy.js | 14 ++++ .../unit/components/shepherd-modal.spec.js | 76 ++++++++++++++++- 5 files changed, 257 insertions(+), 9 deletions(-) create mode 100644 shepherd.js/test/cypress/examples/overflow-clipping.html create mode 100644 shepherd.js/test/cypress/integration/overflow-clipping.cy.js diff --git a/shepherd.js/src/components/shepherd-modal.ts b/shepherd.js/src/components/shepherd-modal.ts index f6cb93b5d..fb1e15f41 100644 --- a/shepherd.js/src/components/shepherd-modal.ts +++ b/shepherd.js/src/components/shepherd-modal.ts @@ -279,13 +279,16 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { return false; } - const { overflowY } = style; - - return ( - overflowY !== 'hidden' && - overflowY !== 'visible' && - el.scrollHeight >= el.clientHeight - ); + // Overflow does not apply to non-replaced inlines, whose rect is just the + // union of their line boxes, and `display: contents` generates no box at + // all (its rect is 0x0 at the origin). Neither crops anything, whatever + // their computed overflow says. + const { display, overflowY } = style; + if (display === 'inline' || display === 'contents') return false; + + // `hidden` and `clip` crop exactly like `auto` and `scroll`; they only + // differ in whether the user can scroll the clipped content into view. + return overflowY !== 'visible'; } /** diff --git a/shepherd.js/test/cypress/examples/overflow-clipping.html b/shepherd.js/test/cypress/examples/overflow-clipping.html new file mode 100644 index 000000000..bcf14e2a0 --- /dev/null +++ b/shepherd.js/test/cypress/examples/overflow-clipping.html @@ -0,0 +1,83 @@ + + + + + + + + + + +
+
Accordion header
+ +
+ +
+ Inline Inline block +
+ +
+
+
Inside display: contents
+
+
+ + diff --git a/shepherd.js/test/cypress/integration/overflow-clipping.cy.js b/shepherd.js/test/cypress/integration/overflow-clipping.cy.js new file mode 100644 index 000000000..5c7f9444c --- /dev/null +++ b/shepherd.js/test/cypress/integration/overflow-clipping.cy.js @@ -0,0 +1,76 @@ +import setupTour from '../utils/setup-tour'; +import overlayOpenings from '../utils/overlay-openings'; + +// End-to-end guard for #3484. Which ancestors crop a highlight depends on +// layout (line boxes, box generation), which happy-dom does not implement, so +// this is the only place the real behaviour is exercised. +describe('modal overlay clipping by overflow ancestors', () => { + let Shepherd, tour; + + beforeEach(() => { + Shepherd = null; + + cy.visit('/test/cypress/examples/overflow-clipping', { + onLoad(contentWindow) { + if (contentWindow.Shepherd) { + return (Shepherd = contentWindow.Shepherd); + } + } + }); + }); + + afterEach(() => { + tour?.complete(); + }); + + const startTour = (stepOptions) => { + cy.document().then(() => { + tour = setupTour( + Shepherd, + { scrollTo: false }, + () => [{ id: 'clipping', text: 'Clipping', ...stepOptions }], + { useModalOverlay: true } + ); + tour.start(); + }); + cy.wait(250); + }; + + /** The opening cut for `selector`, matched by its left edge. */ + const openingFor = (doc, selector) => { + const rect = doc.querySelector(selector).getBoundingClientRect(); + return overlayOpenings(doc).find(({ x }) => Math.abs(x - rect.left) < 1); + }; + + it('crops a highlight inside a collapsed `overflow: hidden` ancestor', () => { + startTour({ + attachTo: { element: '.anchor', on: 'bottom' }, + extraHighlights: ['.collapsed-item'] + }); + + cy.document().then((doc) => { + const [anchor, collapsed] = overlayOpenings(doc); + + expect(anchor.height).to.be.closeTo(40, 1); + // Without treating `hidden` as cropping, this gets a full 80px hole + // over content the user cannot see. + expect(collapsed.height).to.equal(0); + }); + }); + + it('does not crop by an inline `overflow: hidden` ancestor', () => { + startTour({ attachTo: { element: '.inline-item', on: 'bottom' } }); + + cy.document().then((doc) => { + expect(openingFor(doc, '.inline-item').height).to.be.closeTo(100, 1); + }); + }); + + it('does not crop by a `display: contents` ancestor', () => { + startTour({ attachTo: { element: '.contents-item', on: 'bottom' } }); + + cy.document().then((doc) => { + expect(openingFor(doc, '.contents-item').height).to.be.closeTo(60, 1); + }); + }); +}); diff --git a/shepherd.js/test/cypress/integration/root-overflow.cy.js b/shepherd.js/test/cypress/integration/root-overflow.cy.js index eb89584ec..303b84cc0 100644 --- a/shepherd.js/test/cypress/integration/root-overflow.cy.js +++ b/shepherd.js/test/cypress/integration/root-overflow.cy.js @@ -60,6 +60,20 @@ describe('modal overlay with overflow set on the root elements', () => { }); }); + it('keeps the opening when a propagated body overflow is `hidden`', () => { + // #3484: body's computed overflow-y reads `hidden`, but it applies to the + // viewport, which `scrollIntoView` can still scroll. + startTour('body { height: 100vh; overflow: hidden; }', true); + + cy.document().then((doc) => { + const target = doc.querySelector('.target').getBoundingClientRect(); + const [opening] = overlayOpenings(doc); + + expect(opening.y).to.be.closeTo(target.top, 1); + expect(opening.height).to.be.closeTo(40, 1); + }); + }); + it('still clips by body when body is a scroll container of its own', () => { // With no longer `visible`, body keeps its overflow and is a real // scroll container whose rect crops the target below its fold. diff --git a/shepherd.js/test/unit/components/shepherd-modal.spec.js b/shepherd.js/test/unit/components/shepherd-modal.spec.js index 7c32ad458..d5b4d8991 100644 --- a/shepherd.js/test/unit/components/shepherd-modal.spec.js +++ b/shepherd.js/test/unit/components/shepherd-modal.spec.js @@ -527,12 +527,19 @@ describe('components/ShepherdModal', () => { // `positions` maps an element to the `position` it should report, which // decides whether a scrollable ancestor actually crops it. Anything not // listed reports 'static', matching an ordinary element. - function mockOverflow(overflows, positions = new Map()) { + // `displays` maps an element to the `display` it should report; + // anything not listed reports 'block'. + function mockOverflow( + overflows, + positions = new Map(), + displays = new Map() + ) { const spy = vi .spyOn(window, 'getComputedStyle') .mockImplementation((el) => ({ overflowY: overflows.get(el) ?? 'visible', - position: positions.get(el) ?? 'static' + position: positions.get(el) ?? 'static', + display: displays.get(el) ?? 'block' })); restoreComputedStyle = () => spy.mockRestore(); return spy; @@ -975,6 +982,71 @@ describe('components/ShepherdModal', () => { modal.hide(); }); + // Regression coverage for https://github.com/shipshapecode/shepherd/issues/3484 + describe('which overflow ancestors crop', () => { + // The ancestor is collapsed to zero height at y 100; its child is laid out + // at y 100-180 underneath it. + function buildCroppingCase(overflowY, display) { + const targetEl = makeChild(container, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + const ancestor = makeChild(container, { + x: 0, + y: 100, + width: 500, + height: 0 + }); + const extraEl = makeChild(ancestor, { + x: 200, + y: 100, + width: 100, + height: 80 + }); + + mockOverflow( + new Map([[ancestor, overflowY]]), + new Map(), + new Map([[ancestor, display]]) + ); + + return { targetEl, extraEl }; + } + + const openingFor = (overflowY, display = 'block') => { + const modal = createShepherdModal(container); + const { targetEl, extraEl } = buildCroppingCase(overflowY, display); + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + return modal.getElement().querySelector('path').getAttribute('d'); + }; + + it('crops by an `overflow: hidden` ancestor', () => { + expect(openingFor('hidden')).not.toContain('V180'); + }); + + it('crops by an `overflow: clip` ancestor', () => { + expect(openingFor('clip')).not.toContain('V180'); + }); + + it('crops by an `overflow: auto` ancestor', () => { + expect(openingFor('auto')).not.toContain('V180'); + }); + + it('does not crop by an `overflow: visible` ancestor', () => { + expect(openingFor('visible')).toContain('V180'); + }); + + it('does not crop by an inline ancestor, whatever its overflow', () => { + expect(openingFor('hidden', 'inline')).toContain('V180'); + }); + + it('does not crop by a `display: contents` ancestor', () => { + expect(openingFor('hidden', 'contents')).toContain('V180'); + }); + }); + // Regression coverage for https://github.com/shipshapecode/shepherd/issues/1984 // The root element's overflow, and body's whenever is // `overflow: visible`, applies to the viewport rather than to the From 1e723d9d8df01d6f19a6934c165ca2d1f508df66 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:15:35 +0000 Subject: [PATCH 3/4] fix(modal): keep body as a clipper when containment stops overflow propagation Any `contain` value on or stops body's overflow propagating to the viewport, leaving body a real scroll container even while is `overflow: visible` (verified in Chromium for layout, paint, size, style, content, strict and inline-size). Only skip body when neither is contained. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011RqamMhESzSaWmRHLw2Vai --- shepherd.js/src/components/shepherd-modal.ts | 21 ++++++++--- .../unit/components/shepherd-modal.spec.js | 35 ++++++++++++++++--- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/shepherd.js/src/components/shepherd-modal.ts b/shepherd.js/src/components/shepherd-modal.ts index f6cb93b5d..a152caffb 100644 --- a/shepherd.js/src/components/shepherd-modal.ts +++ b/shepherd.js/src/components/shepherd-modal.ts @@ -254,6 +254,10 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { _addStepEventListeners(); } + function _isContained(style: CSSStyleDeclaration) { + return Boolean(style.contain) && style.contain !== 'none'; + } + /** * Whether `el` crops overflowing descendants on the y-axis. * @@ -272,11 +276,18 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { // its computed value still reads `auto` / `scroll`. Clipping against // body's rect then zeroes out targets the viewport has scrolled to as soon // as body is sized to the viewport (e.g. `height: 100%`), see #1984. - if ( - el === body && - window.getComputedStyle(documentElement).overflowY === 'visible' - ) { - return false; + // Any containment on either element stops that propagation, leaving body + // a scroll container of its own. + if (el === body) { + const rootStyle = window.getComputedStyle(documentElement); + + if ( + rootStyle.overflowY === 'visible' && + !_isContained(rootStyle) && + !_isContained(style) + ) { + return false; + } } const { overflowY } = style; diff --git a/shepherd.js/test/unit/components/shepherd-modal.spec.js b/shepherd.js/test/unit/components/shepherd-modal.spec.js index 7c32ad458..d5971a4c4 100644 --- a/shepherd.js/test/unit/components/shepherd-modal.spec.js +++ b/shepherd.js/test/unit/components/shepherd-modal.spec.js @@ -526,13 +526,19 @@ describe('components/ShepherdModal', () => { // unstyled ancestor up to would count as a scroll container. // `positions` maps an element to the `position` it should report, which // decides whether a scrollable ancestor actually crops it. Anything not - // listed reports 'static', matching an ordinary element. - function mockOverflow(overflows, positions = new Map()) { + // listed reports 'static', matching an ordinary element. `contains` + // maps an element to its `contain` value; anything else reports 'none'. + function mockOverflow( + overflows, + positions = new Map(), + contains = new Map() + ) { const spy = vi .spyOn(window, 'getComputedStyle') .mockImplementation((el) => ({ overflowY: overflows.get(el) ?? 'visible', - position: positions.get(el) ?? 'static' + position: positions.get(el) ?? 'static', + contain: contains.get(el) ?? 'none' })); restoreComputedStyle = () => spy.mockRestore(); return spy; @@ -987,7 +993,7 @@ describe('components/ShepherdModal', () => { // Body sized to the viewport (`height: 100%`) and scrolled entirely // above it, while the highlight sits fully on screen at y 200-240. - function buildRootCase(overflows) { + function buildRootCase(overflows, contains) { const targetEl = makeChild(container, { x: 10, y: 10, @@ -1004,7 +1010,7 @@ describe('components/ShepherdModal', () => { stubRect(document.body, offscreen); stubRect(document.documentElement, offscreen); - mockOverflow(overflows); + mockOverflow(overflows, new Map(), contains); return { targetEl, extraEl }; } @@ -1035,6 +1041,25 @@ describe('components/ShepherdModal', () => { expect(d).toContain('V240'); }); + it('still clips by body when containment stops the propagation', () => { + for (const contained of [document.body, document.documentElement]) { + const modal = createShepherdModal(container); + const { targetEl, extraEl } = buildRootCase( + new Map([[document.body, 'auto']]), + new Map([[contained, 'paint']]) + ); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal + .getElement() + .querySelector('path') + .getAttribute('d'); + expect(d).not.toContain('V240'); + restoreComputedStyle(); + } + }); + it('still clips by body when body is its own scroll container', () => { const modal = createShepherdModal(container); // no longer `visible`, so body's overflow stays on body. From bee49b9acbd5e8e713bf01c9bf0c91c74a01e3ec Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 17:26:24 +0000 Subject: [PATCH 4/4] fix(modal): only skip body when is overflow: visible on both axes Body's overflow propagates to the viewport only while the root's overflow is `visible` on both axes. With `html { overflow-x: clip }`, overflow-y still computes to `visible` but body keeps its own scroll container (verified in Chromium), so check overflowX too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011RqamMhESzSaWmRHLw2Vai --- shepherd.js/src/components/shepherd-modal.ts | 12 +++++---- .../unit/components/shepherd-modal.spec.js | 26 ++++++++++++++++--- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/shepherd.js/src/components/shepherd-modal.ts b/shepherd.js/src/components/shepherd-modal.ts index a152caffb..033eb08db 100644 --- a/shepherd.js/src/components/shepherd-modal.ts +++ b/shepherd.js/src/components/shepherd-modal.ts @@ -271,17 +271,19 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { // own box, so it does not crop anything against its own rect. if (el === documentElement) return false; - // While is `overflow: visible`, 's overflow propagates to the - // viewport as well and body's used overflow becomes `visible`, even though - // its computed value still reads `auto` / `scroll`. Clipping against - // body's rect then zeroes out targets the viewport has scrolled to as soon - // as body is sized to the viewport (e.g. `height: 100%`), see #1984. + // While is `overflow: visible` on both axes, 's overflow + // propagates to the viewport as well and body's used overflow becomes + // `visible`, even though its computed value still reads `auto` / `scroll`. + // Clipping against body's rect then zeroes out targets the viewport has + // scrolled to as soon as body is sized to the viewport (e.g. `height: + // 100%`), see #1984. // Any containment on either element stops that propagation, leaving body // a scroll container of its own. if (el === body) { const rootStyle = window.getComputedStyle(documentElement); if ( + rootStyle.overflowX === 'visible' && rootStyle.overflowY === 'visible' && !_isContained(rootStyle) && !_isContained(style) diff --git a/shepherd.js/test/unit/components/shepherd-modal.spec.js b/shepherd.js/test/unit/components/shepherd-modal.spec.js index d5971a4c4..ded3e1e43 100644 --- a/shepherd.js/test/unit/components/shepherd-modal.spec.js +++ b/shepherd.js/test/unit/components/shepherd-modal.spec.js @@ -528,14 +528,18 @@ describe('components/ShepherdModal', () => { // decides whether a scrollable ancestor actually crops it. Anything not // listed reports 'static', matching an ordinary element. `contains` // maps an element to its `contain` value; anything else reports 'none'. + // `overflowXs` maps an element to its `overflowX`; anything else reports + // 'visible'. function mockOverflow( overflows, positions = new Map(), - contains = new Map() + contains = new Map(), + overflowXs = new Map() ) { const spy = vi .spyOn(window, 'getComputedStyle') .mockImplementation((el) => ({ + overflowX: overflowXs.get(el) ?? 'visible', overflowY: overflows.get(el) ?? 'visible', position: positions.get(el) ?? 'static', contain: contains.get(el) ?? 'none' @@ -993,7 +997,7 @@ describe('components/ShepherdModal', () => { // Body sized to the viewport (`height: 100%`) and scrolled entirely // above it, while the highlight sits fully on screen at y 200-240. - function buildRootCase(overflows, contains) { + function buildRootCase(overflows, contains, overflowXs) { const targetEl = makeChild(container, { x: 10, y: 10, @@ -1010,7 +1014,7 @@ describe('components/ShepherdModal', () => { stubRect(document.body, offscreen); stubRect(document.documentElement, offscreen); - mockOverflow(overflows, new Map(), contains); + mockOverflow(overflows, new Map(), contains, overflowXs); return { targetEl, extraEl }; } @@ -1060,6 +1064,22 @@ describe('components/ShepherdModal', () => { } }); + it('still clips by body when only clips the other axis', () => { + const modal = createShepherdModal(container); + // `overflow-x: clip` alone leaves overflow-y computing to `visible`, + // but body's overflow no longer propagates. + const { targetEl, extraEl } = buildRootCase( + new Map([[document.body, 'auto']]), + new Map(), + new Map([[document.documentElement, 'clip']]) + ); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + expect(d).not.toContain('V240'); + }); + it('still clips by body when body is its own scroll container', () => { const modal = createShepherdModal(container); // no longer `visible`, so body's overflow stays on body.