refactor(popper)!: migrate to @floating-ui/react-dom (BREAKING)#1751
Draft
HendrikThePendric wants to merge 18 commits into
Draft
refactor(popper)!: migrate to @floating-ui/react-dom (BREAKING)#1751HendrikThePendric wants to merge 18 commits into
HendrikThePendric wants to merge 18 commits into
Conversation
Replace popper.js v2 (in maintenance) and the inlined react-popper copy with
@floating-ui/react-dom as the positioning engine. Internal code now uses
Floating UI's idiomatic API throughout.
BREAKING CHANGES
- <Popper modifiers={…}> renamed to <Popper middleware={…}>. The prop now
accepts Floating UI middleware functions (offset(), flip(), etc.) instead
of popper.js modifier config objects.
- <Popper onFirstUpdate={…}> dropped. The Popper component now forwards refs
via React.forwardRef — use <Popper ref={…}> to get the popper DOM element.
- <Popper observePopperResize> and observeReferenceResize dropped.
autoUpdate is always on (Floating UI watches both reference and floating
elements via ResizeObserver). Same removal applies to <Popover>.
- usePopper and getBaseModifiers no longer exported from @dhis2-ui/popper.
Consumers wanting low-level access should import useFloating from
@floating-ui/react-dom directly.
- placement="auto" / "auto-start" / "auto-end" no longer supported. Floating
UI uses an autoPlacement() middleware for this behavior instead.
- Tooltip's auto-hide-when-reference-hidden behavior (the
data-popper-reference-hidden CSS rule) is dropped. The original code's
author was uncertain it was working; can be restored via Floating UI's
hide() middleware if needed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
🚀 Deployed on https://pr-1751--dhis2-ui.netlify.app |
popper.js v2's preventOverflow had altAxis: true on top of the default mainAxis: true, so it shifted on BOTH axes when boundaries were hit. Floating UI's shift only does the cross-axis of placement by default (its 'mainAxis' is, confusingly, the cross-axis of the placement direction). Pass crossAxis: true to restore the previous behavior of pushing the popover into the reference when there is no space on either side — needed for the "popover overlaps reference" and "arrow hides when displaced" scenarios. Also reorder base middleware to [flip, shift] to match Floating UI's recommended ordering. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Restore the boundary: document.body option from the original popper.js config. Floating UI's rootBoundary: 'document' alone uses document.documentElement, which is the full viewport. document.body in contrast is sized to fit visible content — typically smaller than the viewport in DHIS2 layouts. Without this, scenarios where the popover should overlap the reference (no space above or below within the body) instead get flipped below because FUI thinks there is room within the document/viewport. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The popper.js v2 logic compared the shift displacement magnitude
against the reference dimension. Floating UI's shift reports different
numeric values for the same visual outcome, so the threshold-based
condition no longer fires reliably.
Replace with a direct geometric check: hide the arrow when the
popper's arrow-edge extends past the corresponding edge of the
reference. This is what the original test expects ("arrow points
past reference means hide") and works the same regardless of which
library computes the position.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Using boundary: document.body caused a feedback loop in some scenarios where the body would grow due to the positioned popover, triggering autoUpdate's layoutShift watcher, repositioning the popover, growing the body further. Floating UI's defaults (rootBoundary: 'viewport', boundary: 'clippingAncestors') are stable — the viewport doesn't grow with content. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two changes to align with popper.js v2 (master) behavior: 1. boundary: document.body — popper.js v2 used document.body as the explicit boundary. Floating UI defaults to clippingAncestors which resolves to the viewport in our DOM (no clipping ancestor in the tree above the popover). On the Hidden Arrow test scenario, body and viewport happen to be the same size (380), so the boundary itself doesn't change behavior — but for real apps with longer scrollable bodies, this preserves popper.js v2's "position based on body" semantic. 2. flip's fallbackStrategy: 'initialPlacement' — popper.js v2's flip prefers the initial placement when no fallback fits cleanly. Floating UI defaults to bestFit (pick whichever overflows less), which produced different placement choices in scenarios where both top and bottom overflow. Together these restore the expected behavior for the "Hiding the arrow" scenario: popover stays at the requested 'top' placement, gets shifted down by 82px to fit within the boundary, ends up overlapping the reference. The hideArrowWhenDisplaced middleware then detects displacement and hides the arrow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Matches popper.js v2's full preventOverflow config:
{ rootBoundary: 'document', boundary: document.body }
Without rootBoundary set, FUI defaults to 'viewport' as the outer
boundary, and the effective boundary becomes the intersection of
body and viewport. When body grows beyond viewport (e.g., scrollable
pages), that intersection clamps to viewport, preventing the popper
from scrolling out of view with its reference.
Fixes "Staying in position when the window is scrolled" tests for
SingleSelect and MultiSelect.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Allow `children` on Popper to be a function called with
{ middlewareData, placement, isPositioned } — the output of the
underlying useFloating call. This completes the middleware API:
consumers can now both pass middleware in and read the data those
middleware produce (e.g. hide, arrow, size).
Use the new render-prop in Tooltip to wire up hide() middleware:
when the reference scrolls out of its containing scroll context,
the tooltip's content gets visibility: hidden — restoring the
behavior that popper.js v2's hide modifier + CSS rule provided
on master.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CustomMiddlewareAndRenderProp showcases the full middleware lifecycle: - offset(), hide() and size() passed via the middleware prop - middlewareData read via children-as-function - visibility applied from middlewareData.hide.referenceHidden - opacity faded in via isPositioned - resolved placement displayed in the popper content Also update the popper story description to reflect Floating UI (previously referenced popper.js and react-popper). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lift the positioning logic into a usePopper hook and reduce <Popper> to a thin JSX wrapper around it. Consumers needing access to middlewareData (or custom DOM structure) use the hook directly; consumers wanting the default container keep using <Popper>. - New usePopper hook in @dhis2-ui/popper combines getBaseMiddleware, RTL-aware placement adjustment and always-on autoUpdate with the underlying useFloating call. Returns the full useFloating result (refs, floatingStyles, middlewareData, placement, isPositioned). - <Popper> now uses usePopper internally; the render-prop API introduced in the previous commit is removed (children: React.ReactNode again). - Tooltip refactored to use usePopper directly via an extracted TooltipContent sub-component (mounted only when open, so the hook's autoUpdate watchers don't run when the tooltip is closed). The visibility-on-reference-hidden behavior is now wired up inline rather than through the Popper render-prop. - Demo story renamed (CustomMiddlewareAndRenderProp → CustomMiddlewareWithHook) and rewritten to use usePopper. This makes Tooltip's coupling to Popper explicit (it's just a hook call, no children-as-function pattern) and matches Popover, which already calls useFloating directly. Popover could be refactored to usePopper too in a follow-up but is unchanged here. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The Tooltip refactor that swapped <Popper> for the usePopper hook collapsed the previous two-div structure into one, dropping the data-test='dhis2-uicore-popper' attribute that the layering cypress test relies on. Restore the outer popper-wrapper div with the data-test, keeping the inner content div (with tooltip styling and data-test='*-content') unchanged. Matches the original DOM structure that the test was written against. Also adds propTypes for the CustomPopper helper in the popper demo story to satisfy SonarCloud. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Picks up the renamed/removed props on Popper and Popover: middleware replaces modifiers, observePopperResize/observeReferenceResize/ onFirstUpdate are gone, the placement default is now 'bottom', and all popper.js-doc links are removed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previous e2e runs were cancelled by the subsequent docs commit (which itself skipped e2e due to path filtering). Need a fresh full CI run to confirm the tooltip data-test fix passes the layering spec. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
manypkg flagged a pre-existing sort order issue in these three files. Since we're already modifying them in this PR (adding @floating-ui/react-dom), apply manypkg's sort fix as well. Other unrelated sort/version issues across the repo are left for a separate cleanup. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous commit restored a two-div structure inside TooltipContent
(outer popper wrapper + inner content). The styled-jsx selector
'div { padding: 4px 6px; ... }' then matched BOTH divs, giving the
outer wrapper padding it shouldn't have and shifting the visible
tooltip ~4px from where the cypress positioning tests expect.
Use a .tooltip-body class selector and put the className on the inner
content div only via classnames cx.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
popper.js v2 accepted 'auto'/'auto-start'/'auto-end' as placement values, picking the side with most available space (with optional alignment). Floating UI doesn't accept these as a placement string — its equivalent is the autoPlacement() middleware. When usePopper detects an auto* placement, it swaps the base flip middleware for autoPlacement. flip and autoPlacement both decide the placement direction and would fight for it if combined, so per Floating UI's docs we substitute rather than add. shift stays in both paths — it operates after the placement is settled and isn't in conflict with either. The placement value passed to useFloating falls back to a concrete 'bottom' for auto* cases since FUI doesn't accept the auto string there — autoPlacement then overrides. Removes 'placement="auto"' from the breaking changes in this PR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
SonarCloud (javascript:S3358) flagged the nested ternary for selecting autoPlacement's alignment based on the placement string. Use a plain lookup object instead — clearer and lints clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Contributor
Author
|
This work will be batched together with some other work that introduces breaking changes, namely the calendar component |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Implements N/A — follow-up to #1750
Stacked on #1750. Targets
feat/add-react-19-support; will retarget master automatically when #1750 merges.Description
Migrates the floating-UI engine from popper.js v2 (in maintenance) to @floating-ui/react-dom. Internal code now uses Floating UI's idiomatic API throughout. Breaking — major version bump.
@dhis2-ui/*packages. (Note: @popperjs/core may still appear inyarn.lockbecause@dhis2/cli-app-scriptstransitively pulls in an older bundled version of @dhis2/ui via @dhis2/app-shell. That's a devDependency chain we don't control from this PR.)usePopperhook (replacing the previousreact-popper.usePopperre-export). WrapsuseFloatingwith dhis2's positioning defaults (base middleware, RTL-aware placement, autoUpdate). Use it directly when you need access tomiddlewareDataor want to compose your own DOM structure.placement="auto"/"auto-start"/"auto-end"continue to work. Floating UI doesn't accept these as placement values — whenusePoppersees an auto placement, it swaps the baseflipmiddleware forautoPlacement(which would otherwise conflict withflip, per FUI docs).shiftstays unchanged in both paths since it operates after the placement is settled.Breaking changes
<Popper modifiers={…}>renamed to<Popper middleware={…}>. Accepts Floating UI middleware functions (offset(),flip(), etc.) instead of popper.js modifier config objects.<Popper onFirstUpdate={…}>dropped. Use ref forwarding for DOM access (<Popper ref={…}>), or useusePopperdirectly and readisPositionedfor one-shot side effects.<Popper>and<Popover>no longer acceptobservePopperResize/observeReferenceResize. Floating UI'sautoUpdateis always-on with all of its defaults (ancestor scroll, ancestor resize, element resize, layout shift). We are intentionally not exposing an escape hatch: the old props were popper.js-flavored, and exposing Floating UI's option shape now would couple our public API to FUI's internals. If a consumer reports a real need to opt out, we'll add a prop then.usePopperis no longer a re-export ofreact-popper's hook. It's now a dhis2-flavored wrapper around Floating UI'suseFloating— same defaults as<Popper>. The signature and return shape are different from the previoususePopper.getBaseModifiersrenamed togetBaseMiddlewareand signature changed: now takes no arguments and returns an array of Floating UI middleware. Already applied automatically byusePopperand<Popper>; the export is for consumers usinguseFloatingfrom@floating-ui/react-domdirectly.Migration guide
modifiers→middlewareThe
modifiersprop has been renamed and now accepts Floating UI middleware functions instead of popper.js modifier config objects.If you authored custom modifiers using popper.js's mutate-state API (
fn: ({ state }) => { state.attributes.foo = … }), you'll need to port them to Floating UI's data-return API (fn: ({ rects, x, y }) => ({ data: { … } })). See the Floating UI middleware docs.onFirstUpdate→ ref forwarding, orusePopper+isPositionedFor DOM access:
For one-shot side effects after first positioning, use
usePopper:observePopperResize/observeReferenceResizeremovedThese have been dropped —
autoUpdateis now always on.usePopperAPI changedusePopperwas previously a re-export ofreact-popper's hook. It's now a dhis2 hook wrapping Floating UI'suseFloating. The API is different:Key differences:
refs.setFloating(a ref setter) instead of needing you to call useState for the popper elementfloatingStyles(apply directly) instead ofstyles.poppermiddlewareDatafor reading hide/arrow/size/etc. dataautoUpdateis always onIf you want Floating UI's
useFloatingdirectly without our defaults, import it from@floating-ui/react-dom.getBaseModifiers→getBaseMiddlewareRenamed and signature simplified. No longer takes options (autoUpdate is always-on).
Both
usePopperand<Popper>apply this automatically. The export is useful only if you're callinguseFloatingfrom@floating-ui/react-domdirectly and want to match Popper's positioning defaults.Reading middleware data (new capability)
Middleware that produces data (e.g.
hide(),arrow(),size()) was previously not consumable from thePoppercomponent. Use the newusePopperhook:See the
Popper / Custom Middleware With Hookstory for a worked example.Checklist
🤖 Generated with Claude Code