From 2d45e87ed51ca28a2550727dea87b379cb8904b4 Mon Sep 17 00:00:00 2001 From: Chuck Carpenter Date: Thu, 13 Aug 2026 13:07:18 +0200 Subject: [PATCH] fix: keep the arrow middleware last when merging custom Floating UI options `getFloatingUIOptions` pushed `arrow()` into `options.middleware` before merging `floatingUIOptions`. Because `deepmerge` concatenates arrays, any user supplied middleware landed *after* the arrow. Floating UI runs middleware sequentially, threading `x`/`y` from one to the next, and `arrow()` computes its offset from the coordinates as they stand on its own turn. Anything running after it moves the tooltip again and silently invalidates `middlewareData.arrow`, which `placeArrow()` writes straight to the DOM. The arrow ends up off by exactly however far the trailing middleware shifted the element -- 60px for an `offset({ crossAxis: 60 })`, 12px for the `offset({ mainAxis: 0, crossAxis: 12 })` recipe in our own cookbook. Merge the user options first, then append the arrow, so it always runs last. When the user supplies their own `arrow()` middleware theirs still wins and ours is not appended, preserving the previous behavior. Tooltip coordinates and resolved placement are unchanged in every case; only the arrow moves. Fixes #3034 Co-Authored-By: Claude Opus 5 --- shepherd.js/src/utils/floating-ui.ts | 74 +++++-- .../test/unit/utils/floating-ui.spec.js | 184 ++++++++++++++++++ 2 files changed, 244 insertions(+), 14 deletions(-) create mode 100644 shepherd.js/test/unit/utils/floating-ui.spec.js diff --git a/shepherd.js/src/utils/floating-ui.ts b/shepherd.js/src/utils/floating-ui.ts index acec7b78f..76c46325f 100644 --- a/shepherd.js/src/utils/floating-ui.ts +++ b/shepherd.js/src/utils/floating-ui.ts @@ -9,6 +9,7 @@ import { limitShift, shift, type ComputePositionConfig, + type Middleware, type MiddlewareData, type Placement, type Alignment @@ -210,24 +211,69 @@ export function getFloatingUIOptions( }) ); - if (arrowEl) { - const arrowOptions = - typeof step.options.arrow === 'object' - ? step.options.arrow - : { padding: 4 }; + if (!hasAutoPlacement) options.placement = attachToOptions.on as Placement; + } - options.middleware.push( - arrow({ - element: arrowEl, - padding: hasEdgeAlignment ? arrowOptions.padding : 0 - }) - ); - } + const mergedOptions: ComputePositionConfig = deepmerge( + options, + step.options.floatingUIOptions || {} + ); - if (!hasAutoPlacement) options.placement = attachToOptions.on as Placement; + // `arrow()` has to be the *last* middleware to run. `Floating UI` executes + // middleware sequentially, threading `x`/`y` from one to the next, and + // `arrow()` computes its offset from the coordinates as they stand on its own + // turn. Any middleware that runs after it (a user supplied `offset()`, + // `shift()`, etc.) moves the tooltip again and silently invalidates + // `middlewareData.arrow`, which `placeArrow()` writes straight to the DOM. + // Since user options are merged in above -- and `deepmerge` concatenates + // arrays -- Shepherd's arrow is appended afterwards rather than pushed in + // before the merge. + if ( + !shouldCenter && + arrowEl && + !hasArrowMiddleware(mergedOptions.middleware) + ) { + const arrowOptions = + typeof step.options.arrow === 'object' + ? step.options.arrow + : { padding: 4 }; + + mergedOptions.middleware = [ + ...(mergedOptions.middleware ?? []), + arrow({ + element: arrowEl, + padding: hasEdgeAlignment ? arrowOptions.padding : 0 + }) + ]; } - return deepmerge(options, step.options.floatingUIOptions || {}); + return mergedOptions; +} + +/** + * Type guard filtering out the falsy entries `Floating UI` allows in a + * middleware array. + * + * @param middleware A single entry of a `middleware` array + * @private + */ +function isMiddleware( + middleware: Middleware | false | null | undefined +): middleware is Middleware { + return Boolean(middleware); +} + +/** + * Determines whether a middleware array already contains an `arrow` middleware, + * in which case the user's own arrow wins and Shepherd does not add its own. + * + * @param middleware The merged `middleware` array, which may contain falsy entries + * @private + */ +function hasArrowMiddleware(middleware: ComputePositionConfig['middleware']) { + return Boolean( + middleware?.some((item) => isMiddleware(item) && item.name === 'arrow') + ); } function addArrow(step: Step) { diff --git a/shepherd.js/test/unit/utils/floating-ui.spec.js b/shepherd.js/test/unit/utils/floating-ui.spec.js new file mode 100644 index 000000000..d110df2df --- /dev/null +++ b/shepherd.js/test/unit/utils/floating-ui.spec.js @@ -0,0 +1,184 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { arrow, offset, shift } from '@floating-ui/dom'; +import { Step } from '../../../src/step'; +import { getFloatingUIOptions } from '../../../src/utils/floating-ui'; + +describe('Floating UI Utils', function () { + let targetElement; + let stepElement; + + /** + * Names of the middleware, in execution order. Falsy entries are preserved so + * that ordering assertions still line up when the user passes them in. + */ + const middlewareNames = ({ middleware }) => + middleware.map((item) => (item ? item.name : item)); + + const createStep = (options) => { + const step = new Step({}, { arrow: true, ...options }); + // `addArrow()` only returns the arrow element when the step is rendered. + step.el = stepElement; + return step; + }; + + beforeEach(() => { + targetElement = document.createElement('div'); + targetElement.classList.add('floating-ui-test'); + document.body.appendChild(targetElement); + + stepElement = document.createElement('div'); + const arrowElement = document.createElement('div'); + arrowElement.classList.add('shepherd-arrow'); + stepElement.appendChild(arrowElement); + document.body.appendChild(stepElement); + }); + + afterEach(() => { + document.body.removeChild(targetElement); + document.body.removeChild(stepElement); + }); + + describe('getFloatingUIOptions()', function () { + it('keeps user middleware and places the arrow middleware last', function () { + const step = createStep({ + attachTo: { element: '.floating-ui-test', on: 'right' }, + floatingUIOptions: { + middleware: [offset(16), shift({ padding: 32 })] + } + }); + + const floatingUIOptions = getFloatingUIOptions( + step.options.attachTo, + step + ); + + expect(middlewareNames(floatingUIOptions)).toEqual([ + 'flip', + 'shift', + 'offset', + 'shift', + 'arrow' + ]); + }); + + it('does not append its own arrow when the user supplies one', function () { + const userArrowElement = document.createElement('div'); + const step = createStep({ + attachTo: { element: '.floating-ui-test', on: 'right' }, + floatingUIOptions: { + middleware: [arrow({ element: userArrowElement }), offset(16)] + } + }); + + const floatingUIOptions = getFloatingUIOptions( + step.options.attachTo, + step + ); + + const names = middlewareNames(floatingUIOptions); + expect(names).toEqual(['flip', 'shift', 'arrow', 'offset']); + expect(names.filter((name) => name === 'arrow')).toHaveLength(1); + + const arrowMiddleware = floatingUIOptions.middleware.find( + ({ name }) => name === 'arrow' + ); + expect(arrowMiddleware.options.element).toBe(userArrowElement); + }); + + it('adds the default middleware when the user supplies none', function () { + const step = createStep({ + attachTo: { element: '.floating-ui-test', on: 'right' } + }); + + const floatingUIOptions = getFloatingUIOptions( + step.options.attachTo, + step + ); + + expect(middlewareNames(floatingUIOptions)).toEqual([ + 'flip', + 'shift', + 'arrow' + ]); + expect(floatingUIOptions.placement).toBe('right'); + expect(floatingUIOptions.strategy).toBe('absolute'); + + const arrowMiddleware = floatingUIOptions.middleware.at(-1); + expect(arrowMiddleware.options.element).toBe( + stepElement.querySelector('.shepherd-arrow') + ); + // Padding only applies to edge aligned placements. + expect(arrowMiddleware.options.padding).toBe(0); + }); + + it('passes the arrow padding through for edge aligned placements', function () { + const step = createStep({ + arrow: { padding: 10 }, + attachTo: { element: '.floating-ui-test', on: 'right-start' } + }); + + const floatingUIOptions = getFloatingUIOptions( + step.options.attachTo, + step + ); + + const arrowMiddleware = floatingUIOptions.middleware.at(-1); + expect(arrowMiddleware.name).toBe('arrow'); + expect(arrowMiddleware.options.padding).toBe(10); + }); + + it('defaults the arrow padding for edge aligned placements', function () { + const step = createStep({ + attachTo: { element: '.floating-ui-test', on: 'right-start' } + }); + + const floatingUIOptions = getFloatingUIOptions( + step.options.attachTo, + step + ); + + expect(floatingUIOptions.middleware.at(-1).options.padding).toBe(4); + }); + + it('does not add any middleware for a centered step', function () { + const step = createStep({ + attachTo: { element: '.floating-ui-test' }, + floatingUIOptions: { + middleware: [offset(16)] + } + }); + + const floatingUIOptions = getFloatingUIOptions( + step.options.attachTo, + step + ); + + expect(middlewareNames(floatingUIOptions)).toEqual(['offset']); + expect(floatingUIOptions.placement).toBeUndefined(); + }); + + it('tolerates falsy entries in the user middleware', function () { + const step = createStep({ + attachTo: { element: '.floating-ui-test', on: 'right' }, + floatingUIOptions: { + middleware: [false, offset(16), null, undefined] + } + }); + + const floatingUIOptions = getFloatingUIOptions( + step.options.attachTo, + step + ); + + expect(middlewareNames(floatingUIOptions)).toEqual([ + 'flip', + 'shift', + false, + 'offset', + null, + undefined, + 'arrow' + ]); + }); + }); +});