From e32f34161d225bc5410bf5160760324ada98d03a Mon Sep 17 00:00:00 2001 From: Chuck Carpenter Date: Thu, 13 Aug 2026 14:24:00 +0200 Subject: [PATCH] fix: clip each highlighted element by its own scroll containers `positionModal` clipped every highlighted element against a single scroll parent, the one derived from `step.target`. Any element in `extraHighlights` that lived in a different scroll container was therefore clamped against a rect it never intersects, so `_getVisibleHeight` returned a height of 0 and the overlay cut out a degenerate, invisible rect. The same single scroll parent was also applied to the containment check, so two elements in different containers could both clamp to zero height at the same clamped `y`, making `isContained` true and suppressing the extra highlight outright. Each highlighted element now resolves its own scroll containers, and `_getVisibleHeight` intersects against all of them rather than only the nearest. Walking the chain is required, not incidental: resolving per element while still clipping at a single level would regress nested layouts, where an extra inside an inner scroll container that is itself scrolled out of an outer one measures as fully visible against the inner container and cuts a hole in the overlay where nothing is on screen. Resolving per element also forced a second correction. DOM ancestry is not the clipping chain: a `fixed` element is laid out against the viewport, and an `absolute` element is cropped only from its containing block upwards, so a scrollable ancestor below that block paints it without cropping it. The walk now derives the containing block from each ancestor's computed `position` and skips ancestors that do not crop the element. Without this, resolving a chain for every highlight unconditionally would have cost an absolutely positioned dropdown its opening entirely whenever the panel it is nested in scrolled away -- a visible highlight becoming invisible. `offsetParent` would be the conventional way to find the containing block; it is unimplemented in happy-dom, so the unit tests could not exercise it and computed `position` is used instead. Rendering changes in four ways, all of them corrections: - An extra highlight in a different scroll container from the target is now cut out where it actually is, instead of collapsing to an invisible rect. - An extra highlight scrolled out of its own container now clips to zero height; previously it was clipped by the target's container instead. - A highlight, the target included, inside nested scroll containers is now clipped by all of them. Previously only the nearest applied, so a highlight scrolled out of an outer container still cut a hole in the overlay. This reaches past the reported bug, but per-element resolution without it would turn that latent flaw into a live one. - A highlight whose position takes it outside a scrollable DOM ancestor is cut out where it is painted rather than clamped to that ancestor. For the target this corrects existing behavior; for extra highlights it was mostly latent, since nothing clipped them at all unless the target happened to have a scroll parent. The chain is memoized per element per step in a `WeakMap` that is reset in `_cleanupStepEventListeners`, because the containment check is O(n^2) over the highlights and runs on every animation frame, while each walk costs one `window.getComputedStyle` call per ancestor. A highlight moved into a different scroll container mid-step keeps its memoized chain until the next `show()` -- the same once-per-step contract the target already had in `_styleForStep`. The fifth positional parameter is kept and renamed `scrollParent` -> `targetScrollParent`. It is still the target's own nearest scroll parent and is still applied to `targetElement` only; callers of the publicly typed `Tour.modal` are unaffected, since parameter names are not part of a function's structural type. Clipping remains y-axis only, and the walk still stops at shadow-DOM and iframe document boundaries. Transformed and filtered ancestors, which establish a containing block for `fixed` descendants, are not accounted for. All three are pre-existing and out of scope. Fixes #3344 Co-Authored-By: Claude Opus 5 --- docs-src/src/content/docs/guides/usage.md | 7 +- docs-src/src/content/docs/recipes/cookbook.md | 2 + shepherd.js/src/components/shepherd-modal.ts | 144 +++- .../unit/components/shepherd-modal.spec.js | 618 ++++++++++++++++++ 4 files changed, 749 insertions(+), 22 deletions(-) diff --git a/docs-src/src/content/docs/guides/usage.md b/docs-src/src/content/docs/guides/usage.md index 43c18852b..e3928f0a1 100644 --- a/docs-src/src/content/docs/guides/usage.md +++ b/docs-src/src/content/docs/guides/usage.md @@ -292,7 +292,12 @@ function will be called in the `before-show` phase. ``` - `extraHighlights`: An array of extra element selectors to highlight when the overlay is shown The tooltip won’t be fixed to these elements, but they will - be highlighted just like the attachTo element. + be highlighted just like the attachTo element. They do not have to share a + scroll container with the attachTo element: each one is clipped vertically by + the scroll containers that actually crop it, so only the part of it that is + scrolled into view is cut out of the overlay. An element positioned outside + those containers — `fixed`, or `absolute` against a containing block above + them — is cut out in full, matching where it is painted. - `advanceOn`: An action on the page which should advance shepherd to the next step. It should be an object with a string `selector` and an `event` name. For example: `{selector: '.some-element', event: 'click'}`. It doesn't have to be diff --git a/docs-src/src/content/docs/recipes/cookbook.md b/docs-src/src/content/docs/recipes/cookbook.md index b43c646a4..17b57522e 100644 --- a/docs-src/src/content/docs/recipes/cookbook.md +++ b/docs-src/src/content/docs/recipes/cookbook.md @@ -31,6 +31,8 @@ const tour = new Shepherd.Tour({ If an element to be highlighted is contained by another element that is also being highlighted, the contained element will not be highlighted. This is to prevent the contained element from being obscured by the containing element. +Highlighted elements do not have to share a scroll container with the `attachTo` target. Each one is clipped vertically by the scroll containers that actually crop it, so only the part of it that is scrolled into view is cut out of the overlay. An element whose position takes it outside those containers — `fixed`, or `absolute` against a containing block above them, as a dropdown usually is — is cut out in full, wherever it is painted. Clipping is vertical only: an element scrolled out of view horizontally is still cut out in full. + ### Offsets By default, FloatingUI instances are placed directly next to their target. However, if you need to apply some margin diff --git a/shepherd.js/src/components/shepherd-modal.ts b/shepherd.js/src/components/shepherd-modal.ts index 295d88229..b7cf55318 100644 --- a/shepherd.js/src/components/shepherd-modal.ts +++ b/shepherd.js/src/components/shepherd-modal.ts @@ -36,7 +36,7 @@ export interface ShepherdModalAPI { modalOverlayOpeningRadius?: ModalRadiusType, modalOverlayOpeningXOffset?: number, modalOverlayOpeningYOffset?: number, - scrollParent?: HTMLElement | null, + targetScrollParent?: HTMLElement | null, targetElement?: HTMLElement | null, extraHighlights?: HTMLElement[] ) => void; @@ -47,6 +47,9 @@ export interface ShepherdModalAPI { export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { let rafId: number | undefined; + // Memoizes the chain of scroll parents of a highlighted element for the + // lifetime of a single step. Reset in `_cleanupStepEventListeners`. + let _stepScrollParents = new WeakMap(); let openingProperties: OpeningProperty[] = [ { width: 0, height: 0, x: 0, y: 0, r: 0 } ]; @@ -84,12 +87,18 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { element.classList.add('shepherd-modal-is-visible'); } + /** + * @param targetScrollParent The nearest scroll parent of `targetElement` + * only. Extra highlights can live in entirely different scroll containers, + * so each one resolves its own chain via `_cachedScrollParents`. Any scroll + * containers above `targetScrollParent` are resolved here too. + */ function positionModal( modalOverlayOpeningPadding = 0, modalOverlayOpeningRadius: ModalRadiusType = 0, modalOverlayOpeningXOffset = 0, modalOverlayOpeningYOffset = 0, - scrollParent?: HTMLElement | null, + targetScrollParent?: HTMLElement | null, targetElement?: HTMLElement | null, extraHighlights?: HTMLElement[] ) { @@ -97,6 +106,18 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { const elementsToHighlight = [targetElement, ...(extraHighlights || [])]; const newOpenings: OpeningProperty[] = []; + // The target's nearest scroll parent is supplied by the caller; the rest + // of its chain is resolved the same way as for any other highlight. + const targetScrollParents = targetScrollParent + ? [ + targetScrollParent, + ..._cachedScrollParents(targetScrollParent.parentElement) + ] + : []; + + const scrollParentsFor = (el: HTMLElement) => + el === targetElement ? targetScrollParents : _cachedScrollParents(el); + for (const el of elementsToHighlight) { if (!el) continue; @@ -108,18 +129,19 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { continue; } - const { y, height } = _getVisibleHeight(el, scrollParent); + const { y, height } = _getVisibleHeight(el, scrollParentsFor(el)); const { x, width, left } = el.getBoundingClientRect(); // Check if the element is contained by another element. - // Use _getVisibleHeight for otherElement too so both sides - // compare scroll-clipped geometry on the y-axis. + // Use _getVisibleHeight for otherElement too so both sides compare + // scroll-clipped geometry on the y-axis, each element being clipped + // by its own scroll parents. const isContained = elementsToHighlight.some((otherElement) => { if (otherElement === el) return false; const otherRect = otherElement.getBoundingClientRect(); const { y: otherY, height: otherHeight } = _getVisibleHeight( otherElement, - scrollParent + scrollParentsFor(otherElement) ); return ( x >= otherRect.left && @@ -195,6 +217,9 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { rafId = undefined; } + // Scroll parents are only memoized for the duration of a single step. + _stepScrollParents = new WeakMap(); + window.removeEventListener('touchmove', _preventModalBodyTouch, { passive: false } as EventListenerOptions); @@ -209,7 +234,7 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { } = step.options; const iframeOffset = _getIframeOffset(step.target); - const scrollParent = _getScrollParent(step.target); + const targetScrollParent = _getScrollParent(step.target); const rafLoop = () => { rafId = undefined; @@ -218,7 +243,7 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { modalOverlayOpeningRadius, modalOverlayOpeningXOffset + iframeOffset.left, modalOverlayOpeningYOffset + iframeOffset.top, - scrollParent, + targetScrollParent, step.target, step._resolvedExtraHighlightElements ); @@ -229,18 +254,98 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { _addStepEventListeners(); } - function _getScrollParent(el?: HTMLElement | null): HTMLElement | null { - if (!el) return null; + /** + * Whether `el` crops overflowing descendants on the y-axis. + * + * @param el The candidate scroll container + * @param style `el`'s computed style, already resolved by the caller + */ + function _isScrollable(el: HTMLElement, style: CSSStyleDeclaration) { + const { overflowY } = style; + + return ( + overflowY !== 'hidden' && + overflowY !== 'visible' && + el.scrollHeight >= el.clientHeight + ); + } - const isHtmlElement = el instanceof HTMLElement; - const overflowY = isHtmlElement && window.getComputedStyle(el).overflowY; - const isScrollable = overflowY !== 'hidden' && overflowY !== 'visible'; + /** + * Every scroll container that clips `el`, nearest first, including `el` + * itself when it is one. + * + * Clipping against the nearest alone is wrong as soon as scroll containers + * nest: an element sitting inside an inner container that has itself been + * scrolled out of an outer one measures as fully visible against the inner + * container, and the overlay would cut a hole where nothing is on screen. + * + * DOM ancestry is not the clipping chain either. A `fixed` element is laid + * out against the viewport, and an `absolute` element is cropped only from + * its containing block upwards -- scrollable ancestors below that block paint + * it without cropping it. Walking `parentElement` unconditionally would size + * the opening for an absolutely positioned dropdown to whichever panel it + * happens to be nested in rather than to where it is painted, and would drop + * the opening entirely once that panel is scrolled away. + * + * The containing block is derived from each ancestor's computed `position` + * rather than from `offsetParent`, which happy-dom does not implement and the + * unit tests therefore cannot exercise. + */ + function _getScrollParents(el?: HTMLElement | null): HTMLElement[] { + if (!(el instanceof HTMLElement)) return []; + + const { position } = window.getComputedStyle(el); + + // Laid out against the viewport, so no ancestor crops it. + if (position === 'fixed') return []; + + const scrollParents: HTMLElement[] = []; + let crops = position !== 'absolute'; + + for ( + let current: HTMLElement | null = el; + current; + current = current.parentElement + ) { + const style = window.getComputedStyle(current); + + // The nearest positioned ancestor is an absolutely positioned element's + // containing block; from there upwards the usual overflow rules apply. + if (!crops && current !== el && style.position !== 'static') { + crops = true; + } - if (isScrollable && el.scrollHeight >= el.clientHeight) { - return el; + if (crops && _isScrollable(current, style)) { + scrollParents.push(current); + } } - return _getScrollParent(el.parentElement); + return scrollParents; + } + + function _getScrollParent(el?: HTMLElement | null): HTMLElement | null { + return _getScrollParents(el)[0] ?? null; + } + + /** + * Memoized `_getScrollParents`, scoped to the current step. + * + * Resolving inline would be prohibitively expensive: the containment check in + * `positionModal` is O(n^2) over the highlighted elements and runs on every + * animation frame, while each walk costs one `window.getComputedStyle` call + * per ancestor. The answer cannot change between frames of a step, so it is + * resolved once per element instead — matching the once-per-step contract the + * target already has in `_styleForStep`. + */ + function _cachedScrollParents(el?: HTMLElement | null): HTMLElement[] { + if (!el) return []; + + const cached = _stepScrollParents.get(el); + if (cached) return cached; + + const scrollParents = _getScrollParents(el); + _stepScrollParents.set(el, scrollParents); + return scrollParents; } function _getIframeOffset(el?: HTMLElement | null) { @@ -269,15 +374,12 @@ export function createShepherdModal(container: HTMLElement): ShepherdModalAPI { return offset; } - function _getVisibleHeight( - el: HTMLElement, - scrollParent?: HTMLElement | null - ) { + function _getVisibleHeight(el: HTMLElement, scrollParents: HTMLElement[]) { const elementRect = el.getBoundingClientRect(); let top = elementRect.y || elementRect.top; let bottom = elementRect.bottom || top + elementRect.height; - if (scrollParent) { + for (const scrollParent of scrollParents) { const scrollRect = scrollParent.getBoundingClientRect(); const scrollTop = scrollRect.y || scrollRect.top; const scrollBottom = scrollRect.bottom || scrollTop + scrollRect.height; diff --git a/shepherd.js/test/unit/components/shepherd-modal.spec.js b/shepherd.js/test/unit/components/shepherd-modal.spec.js index 918a0c31a..e6c5dee88 100644 --- a/shepherd.js/test/unit/components/shepherd-modal.spec.js +++ b/shepherd.js/test/unit/components/shepherd-modal.spec.js @@ -479,6 +479,624 @@ describe('components/ShepherdModal', () => { const cutouts = d.split('Z').length - 1; expect(cutouts).toBe(2); }); + + describe('across scroll parents', function () { + // Regression coverage for https://github.com/shipshapecode/shepherd/issues/3344 + // Every highlighted element must be clipped by its OWN chain of scroll + // containers, not by the scroll parent of the `attachTo` target. + let restoreComputedStyle; + let restoreRaf; + + function stubRect(el, { x, y, width, height }) { + Object.defineProperty(el, 'getBoundingClientRect', { + configurable: true, + value: () => ({ + x, + y, + width, + height, + top: y, + bottom: y + height, + left: x, + right: x + width + }) + }); + } + + function makeScrollContainer(rect, parent = container) { + const el = document.createElement('div'); + Object.defineProperty(el, 'scrollHeight', { value: 500 }); + Object.defineProperty(el, 'clientHeight', { value: rect.height }); + parent.appendChild(el); + stubRect(el, rect); + return el; + } + + function makeChild(parent, rect) { + const el = document.createElement('div'); + parent.appendChild(el); + stubRect(el, rect); + return el; + } + + // `overflows` maps an element to the `overflowY` it should report. + // Everything else reports 'visible', which is what a real browser returns + // for an ordinary element. happy-dom returns '' instead, and + // `_getScrollParent` reads '' as scrollable, so without this every + // 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()) { + const spy = vi + .spyOn(window, 'getComputedStyle') + .mockImplementation((el) => ({ + overflowY: overflows.get(el) ?? 'visible', + position: positions.get(el) ?? 'static' + })); + restoreComputedStyle = () => spy.mockRestore(); + return spy; + } + + function mockRaf() { + const spy = vi + .spyOn(window, 'requestAnimationFrame') + .mockImplementation(() => 1); + restoreRaf = () => spy.mockRestore(); + return spy; + } + + afterEach(() => { + restoreComputedStyle?.(); + restoreComputedStyle = undefined; + restoreRaf?.(); + restoreRaf = undefined; + }); + + it('cuts out an extra highlight living in a different scroll parent', () => { + const modal = createShepherdModal(container); + + // Scroll container A holds the attachTo target, high on the page. + const containerA = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 100 + }); + const targetEl = makeChild(containerA, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + + // Scroll container B is a sibling further down, holding the extra. + const containerB = makeScrollContainer({ + x: 0, + y: 200, + width: 500, + height: 100 + }); + const extraEl = makeChild(containerB, { + x: 200, + y: 210, + width: 100, + height: 40 + }); + + mockOverflow( + new Map([ + [containerA, 'auto'], + [containerB, 'auto'] + ]) + ); + + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + // Outer path + target cutout + extra cutout + expect(d.split('Z').length - 1).toBe(3); + // The extra is fully visible inside container B: y 210, height 40. + // Before the fix it was clipped by container A (y 0-100) down to a + // degenerate zero-height rect ending at V210. + expect(d).toContain('M200,210'); + expect(d).toContain('V250'); + }); + + it('still clips an extra highlight by its own scroll parent', () => { + const modal = createShepherdModal(container); + + const containerA = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 100 + }); + const targetEl = makeChild(containerA, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + + // Extra lives in the SAME container, but scrolled below its bottom edge. + const extraEl = makeChild(containerA, { + x: 200, + y: 210, + width: 100, + height: 40 + }); + + mockOverflow(new Map([[containerA, 'auto']])); + + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + // Clipped to zero height — starts and ends at y 210. + expect(d).toContain('M200,210'); + expect(d).toContain('V210'); + expect(d).not.toContain('V250'); + }); + + it('clips a highlight by every scroll container above it, not just the nearest', () => { + const modal = createShepherdModal(container); + + // Outer scroll container, on screen, holding the target. + const outer = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 300 + }); + const targetEl = makeChild(outer, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + + // Inner scroll container nested inside `outer` and scrolled out of it. + const inner = makeScrollContainer( + { x: 0, y: 400, width: 500, height: 200 }, + outer + ); + const extraEl = makeChild(inner, { + x: 200, + y: 420, + width: 100, + height: 40 + }); + + mockOverflow( + new Map([ + [outer, 'auto'], + [inner, 'auto'] + ]) + ); + + modal.positionModal(0, 0, 0, 0, outer, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + // The target is on screen and still cut out. + expect(d).toContain('M10,10'); + // The extra is fully visible within `inner`, but `inner` is scrolled + // out of `outer`, so none of it is on screen. Clipping against the + // nearest scroll container alone would punch a 100x40 hole here. + expect(d).toContain('M200,420'); + expect(d).not.toContain('V460'); + }); + + it('uses each element own scroll parents in the containment check', () => { + const modal = createShepherdModal(container); + + // Container A is tall enough that it clips nothing. + const containerA = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 1000 + }); + const targetEl = makeChild(containerA, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + + // `big` overflows its own short container and is not on screen at all... + const containerB = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 50 + }); + const big = makeChild(containerB, { + x: 150, + y: 100, + width: 200, + height: 300 + }); + + // ...but its unclipped rect encloses `small`, which is fully visible. + const containerC = makeScrollContainer({ + x: 0, + y: 200, + width: 500, + height: 100 + }); + const small = makeChild(containerC, { + x: 200, + y: 210, + width: 100, + height: 40 + }); + + mockOverflow( + new Map([ + [containerA, 'auto'], + [containerB, 'auto'], + [containerC, 'auto'] + ]) + ); + + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [big, small]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + // Measured against container A — the target's scroll parent — `big` + // would be y 100 height 300, which contains `small` and suppresses it. + // Measured against its own container B it has zero height, so `small` + // still gets a cutout. + expect(d).toContain('M200,210'); + expect(d).toContain('V250'); + // Outer path + target + big + small + expect(d.split('Z').length - 1).toBe(4); + }); + + it('clips the target by the scroll parent it is handed, not by its own', () => { + const modal = createShepherdModal(container); + + // Nothing in the target's ancestry is scrollable, so resolving its + // scroll parent from the DOM would find none. `_styleForStep` hands the + // scroll parent in, and that is the one the target must be clipped by. + const targetEl = makeChild(container, { + x: 10, + y: 10, + width: 100, + height: 500 + }); + const suppliedScrollParent = { + getBoundingClientRect: () => ({ + x: 10, + y: 100, + width: 500, + height: 250, + top: 100, + bottom: 350, + left: 10, + right: 510 + }) + }; + + mockOverflow(new Map()); + + modal.positionModal(0, 0, 0, 0, suppliedScrollParent, targetEl); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + expect(d).toContain('M10,100'); + expect(d).toContain('V350'); + }); + + it('clips every highlight by the shared scroll parent when they share one', () => { + const modal = createShepherdModal(container); + + const containerA = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 200 + }); + const targetEl = makeChild(containerA, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + // Hangs over the bottom edge of A, so the clip actually bites: the + // bottom 50px are cut off and the cutout is 50 tall, not 100. + const extraEl = makeChild(containerA, { + x: 200, + y: 150, + width: 100, + height: 100 + }); + + mockOverflow(new Map([[containerA, 'auto']])); + + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + + expect(modal.getElement().querySelector('path')).toHaveAttribute( + 'd', + 'M1024,768H0V0H1024V768ZM10,10a0,0,0,0,0-0,0V60a0,0,0,0,0,0,0H110a0,0,0,0,0,0-0V10a0,0,0,0,0-0-0ZM200,150a0,0,0,0,0-0,0V200a0,0,0,0,0,0,0H300a0,0,0,0,0,0-0V150a0,0,0,0,0-0-0Z' + ); + }); + + it('resolves each scroll parent once per step rather than once per frame', () => { + const modal = createShepherdModal(container); + + const containerA = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 100 + }); + const targetEl = makeChild(containerA, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + + const containerB = makeScrollContainer({ + x: 0, + y: 200, + width: 500, + height: 100 + }); + const extraEl = makeChild(containerB, { + x: 200, + y: 210, + width: 100, + height: 40 + }); + + const spy = mockOverflow( + new Map([ + [containerA, 'auto'], + [containerB, 'auto'] + ]) + ); + + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + const afterFirstFrame = spy.mock.calls.length; + expect(afterFirstFrame).toBeGreaterThan(0); + + // A second frame of the same step must not re-walk the ancestor chain. + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + expect(spy.mock.calls.length).toBe(afterFirstFrame); + + // ...but the memo must not survive the step, or it could go stale. + modal.hide(); + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + expect(spy.mock.calls.length).toBeGreaterThan(afterFirstFrame); + }); + + it('memoizes a highlight that resolves to no scroll parent at all', () => { + const modal = createShepherdModal(container); + + const containerA = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 100 + }); + const targetEl = makeChild(containerA, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + + // The extra sits in no scroll container at all, so its resolved chain + // is empty. That answer costs a full ancestor walk to reach and has to + // be memoized just like a non-empty one. + const extraEl = makeChild(container, { + x: 200, + y: 210, + width: 100, + height: 40 + }); + + const spy = mockOverflow(new Map([[containerA, 'auto']])); + + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + const afterFirstFrame = spy.mock.calls.length; + expect(afterFirstFrame).toBeGreaterThan(0); + + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + expect(spy.mock.calls.length).toBe(afterFirstFrame); + + modal.hide(); + modal.positionModal(0, 0, 0, 0, containerA, targetEl, [extraEl]); + expect(spy.mock.calls.length).toBeGreaterThan(afterFirstFrame); + }); + + it('resolves extraHighlights scroll parents through setupForStep', () => { + const modal = createShepherdModal(container); + + const containerA = makeScrollContainer({ + x: 0, + y: 0, + width: 500, + height: 100 + }); + const targetEl = makeChild(containerA, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + + const containerB = makeScrollContainer({ + x: 0, + y: 200, + width: 500, + height: 100 + }); + const extraEl = makeChild(containerB, { + x: 200, + y: 210, + width: 100, + height: 40 + }); + extraEl.classList.add('extra-highlight-3344'); + + const tour = new Tour({ useModalOverlay: true }); + const step = new Step(tour, { + attachTo: { element: targetEl, on: 'bottom' }, + extraHighlights: ['.extra-highlight-3344'] + }); + step._resolveAttachToOptions(); + step.target = targetEl; + step._resolveExtraHiglightElements(); + + mockRaf(); + mockOverflow( + new Map([ + [containerA, 'auto'], + [containerB, 'auto'] + ]) + ); + + // Goes through _styleForStep, which is what resolves the target's own + // scroll parent and passes it in as `targetScrollParent`. + modal.setupForStep(step); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + expect(d.split('Z').length - 1).toBe(3); + expect(d).toContain('M200,210'); + expect(d).toContain('V250'); + + modal.hide(); + }); + + // 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 + // absolutely positioned dropdown painted outside the panel it is nested + // in loses its opening entirely. + describe('elements whose position escapes a scrollable ancestor', () => { + // Panel occupies y 400-500; the extra is a DOM child of it but is + // painted up at y 80-120, the way an absolutely positioned dropdown is. + function buildEscapingCase(extraPosition, panelPosition = 'static') { + const panel = makeScrollContainer({ + x: 0, + y: 400, + width: 500, + height: 100 + }); + const targetEl = makeChild(container, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + const extraEl = makeChild(panel, { + x: 300, + y: 80, + width: 120, + height: 40 + }); + + mockOverflow( + new Map([[panel, 'auto']]), + new Map([ + [panel, panelPosition], + [extraEl, extraPosition] + ]) + ); + + return { panel, targetEl, extraEl }; + } + + it('keeps the full opening for an absolutely positioned extra highlight', () => { + const modal = createShepherdModal(container); + const { targetEl, extraEl } = buildEscapingCase('absolute'); + + // The target has no scroll parent of its own, so the caller passes null. + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + expect(d.split('Z').length - 1).toBe(3); + // Painted at y 80-120 and cut out there, not collapsed against the + // panel's y 400-500. + expect(d).toContain('M300,80'); + expect(d).toContain('V120'); + }); + + it('keeps the full opening for a fixed position extra highlight', () => { + const modal = createShepherdModal(container); + const { targetEl, extraEl } = buildEscapingCase('fixed'); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + expect(d.split('Z').length - 1).toBe(3); + expect(d).toContain('M300,80'); + expect(d).toContain('V120'); + }); + + // The discriminator: absolute positioning does not exempt an element + // from cropping, it only moves which ancestor does the cropping. When + // the scrollable panel IS the containing block, it crops as usual. + it('still clips an absolutely positioned extra to its containing block', () => { + const modal = createShepherdModal(container); + const { targetEl, extraEl } = buildEscapingCase( + 'absolute', + 'relative' + ); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + // Scrolled out of its own containing block, so it collapses to zero + // height rather than cutting a hole where nothing is painted. + expect(d).toContain('M300,400'); + expect(d).toContain('V400'); + expect(d).not.toContain('V120'); + }); + + it('still clips an in-flow extra scrolled out of its own container', () => { + const modal = createShepherdModal(container); + + const panel = makeScrollContainer({ + x: 0, + y: 400, + width: 500, + height: 100 + }); + const targetEl = makeChild(container, { + x: 10, + y: 10, + width: 100, + height: 50 + }); + const extraEl = makeChild(panel, { + x: 300, + y: 600, + width: 120, + height: 40 + }); + + mockOverflow(new Map([[panel, 'auto']])); + + modal.positionModal(0, 0, 0, 0, null, targetEl, [extraEl]); + + const d = modal.getElement().querySelector('path').getAttribute('d'); + + expect(d).toContain('M300,600'); + expect(d).toContain('V600'); + }); + }); + }); }); describe('setupForStep()', function () {