feat(popover): gen2 popover migration#6356
Draft
rubencarvalho wants to merge 33 commits into
Draft
Conversation
Adds sidebar entries for accessibility migration analysis docs on action-button, button-group, close-button, grid, and infield-button, plus the link migration plan, so they appear under their components in the Storybook navigation.
Extracts Floating UI-based positioning from 1st-gen overlay's PlacementController into a standalone reactive controller under core/controllers. Adds start/stop/recompute API, hyphenated Placement union aligned with Floating UI and swc-popover, opt-in constrainSize for picker/menu scroll cases, and VirtualTrigger support for virtual anchors. Wires the floating-ui/dom dependency and package.json export for the new controller subpath. Also lands the supporting Storybook ApiTable block plus the ConditionalAPIReference template change, and migrates the focusgroup-navigation-controller stories to the new controllerApi parameter pattern. SWC-1996
Address correctness bugs, plan-deviating defaults, an iOS regression
risk, and minor structural issues surfaced during code review.
Correctness
- toFloatingPlacement: logical-side + physical-alignment placements
(start-top, start-bottom, end-top, end-bottom) previously returned
invalid Floating UI strings (left-top, left-bottom, right-top,
right-bottom) because the logical-side branch did not consult
PHYSICAL_ALIGNMENT_TO_FLOATING. They now resolve correctly to
left-start / left-end / right-start / right-end.
- fromFloatingPlacement: Floating UI's left-start, left-end,
right-start, right-end are not in the SWC Placement union; the
function now maps them back to the physical equivalents
(left-top / left-bottom / right-top / right-bottom). The
previously unchecked cast is gone.
Defaults aligned with the popover migration plan
- DEFAULT_OFFSET: 8 to 0 (controller is now neutral; each consuming
component sets its own pattern-specific default).
- DEFAULT_CONTAINER_PADDING: 12 to 8 (matches 1st-gen
REQUIRED_DISTANCE_TO_EDGE and Spectrum guidance).
- Storybook args and argType defaults updated to match.
Behavior
- Restored an iOS WebKit visualViewport listener (passive,
rAF-coalesced) so positioning stays correct when the URL bar,
pinch-zoom, or virtual keyboard shifts the visual viewport without
triggering events that Floating UI's autoUpdate observes. The
matching offset compensation in computePlacement was already
present; only the trigger was missing.
- Comment on the single autoUpdate channel explaining the
intentional difference from 1st-gen, which closed the overlay on
ancestor scroll rather than repositioning.
Structure
- Added a reserved PlacementHostConfig interface and updated the
constructor to accept an optional config argument, so future
integration hooks (e.g. tip-element resolver for arrow middleware)
can be added without a constructor signature change.
- Cached isWebKit() per session so computePlacement no longer runs a
UA regex per autoUpdate tick.
- Removed the internal-only getFallbackPlacements re-export from
src/index.ts (it is not part of the public surface).
- Expanded JSDoc on actualPlacement (initial-value semantics),
fromFloatingPlacement (invariants), and toPlacementClassSuffix
(why the indirection exists).
Tests
- Added play-function stories covering the placement-conversion fix
end to end, the logical-side placements computing valid
coordinates, constrainSize applying max-height, shouldFlip: false
preserving the requested side, and rapid start() calls replacing
the prior session.
SWC-1996
- Drop toPlacementClassSuffix. It was a pass-through that just returned
its input; consumers can do the BEM string interpolation themselves.
- Strip {@link Foo} JSDoc annotations across placement-controller
files; they don't render in this environment. Replaced with plain
backtick references.
- Remove popover-specific phrasing from controller comments and JSDoc
so the controller stays host-agnostic. Generic mentions of popover,
picker, menu, etc. as example use cases in story descriptions are
retained where they help the reader.
- Revert the focusgroup-navigation-controller stories file. The
controllerApi parameter pattern was not required for the
placement-controller API table to render; the focusgroup keeps its
prior inline-JSDoc API documentation.
Revert the controllerApi infrastructure added to ApiTable.tsx and DocumentTemplate.mdx. Document the placement-controller's methods, readonly properties, options, and types as inline markdown tables in JSDoc on a description-only API story — matching the focusgroup-navigation-controller pattern. - ApiTable.tsx: drop the ControllerApiReference type, five controller-specific table components, and the tag-based branching added for controllers. Component CEM rendering is unchanged. - DocumentTemplate.mdx: restore ConditionalAPISection (Primary + Controls + optional stories tagged 'api') and drop the ConditionalAPIReference / ApiTable wiring. - placement-controller stories: remove parameters.controllerApi, add an API story with inline JSDoc tables tagged ['api', 'description-only'].
Replace the two side-by-side flip demos (demo-placement-flip and demo-placement-no-flip) with a single demo-placement-should-flip that has a shouldFlip checkbox. One trigger + one floating panel + one toggle isolates the feature so readers can verify the placement only moves because of the flip middleware. - demo-hosts.ts: delete DemoPlacementFlip and DemoPlacementNoFlip; add DemoPlacementShouldFlip with a should-flip attribute, a checkbox in the demo template, and an updated() hook that rebinds the controller when the toggle flips. - placement-controller.stories.ts: ShouldFlip story now renders a single demo-placement-should-flip element. Story description gained a sentence explaining the toggle. - placement-controller.test.ts: FlipReorients reads from the new element (default shouldFlip=true). NoFlipKeepsRequestedSide reuses the same element, sets shouldFlip=false, then asserts actualPlacement stays at 'bottom'.
- Virtual trigger demo: surface is a square (aspect-ratio 1, max 320px) so the click area reads as a single anchor canvas. The 'Click to move anchor' label uses Body sizeL emphasized so it reads as the demo's call to action. Added a 12px offset so the floating panel sits visibly clear of the clicked point. - Constrain-size demo: dropped the dashed outline around the surface. It implied the floating element was bounded by the surface, but the size middleware uses the viewport / clipping ancestors, so the scaffolding was misleading. - Story docs: removed prescriptive lines that named specific consumer components (picker / menu / combobox / tooltip / popover) in the description for shouldFlip, constrainSize, the meta JSDoc, and the subtitle. The controller is host-agnostic. - Documentation template: replaced the bare <ApiTable /> with a ConditionalApi helper. Controllers (tag 'controller') document their API via inline JSDoc stories tagged 'api', so they no longer trigger the 'Component not found in manifest' fallback. Components are unchanged.
Switch the virtual-trigger demo surface from a responsive square (inline-size: 100%; max-inline-size: 320px; aspect-ratio: 1) to a fixed 320x320px so the click target is the same size at any container width.
- Drop the dashed outline on the surface; replace with a subtle tinted background and a rounded border for a calmer visual. - Add 'resize: vertical' so the user can drag the bottom edge of the box to grow it. Pushing the trigger toward the bottom of the viewport makes the requested 'bottom' placement no longer fit, so toggling shouldFlip becomes meaningful — the user can watch the panel flip above when enabled and stay below (overflowing) when disabled. - Added a short hint paragraph above the surface explaining what to do.
The interactive resize-to-flip demo didn't reliably demonstrate the feature — flip depends on the trigger's viewport position, which is fragile across canvas sizes. Documenting the behavior in the story description is clearer than a demo that may not flip. - Story: ShouldFlip becomes a description-only story (no render). - Demo hosts: drop DemoPlacementShouldFlip, its tagname map entry, the FLIP_DEMO_ITEMS items, and the flipDemoStyles sheet that was only used by it. - Tests: drop FlipReorients and NoFlipKeepsRequestedSide, plus the ShouldFlip / DemoPlacementShouldFlip imports they depended on. Pure-function and constrainSize tests still exercise the controller surface.
The docs referenced `shift` in seven places as if it were an opt-in option. It isn't — `shift` runs on every compute and there's no opt-out (same as 1st-gen). Spell that out in the middleware stack section so the existing prose references read correctly.
shift is not a configurable option, so listing it alongside flip and
size in option JSDoc and story prose just makes those references read
like undocumented options. Replace the lists with neutral phrasing
("used for collision detection" / "inset from the overflow
boundary") and keep a single shift mention in the middleware-stack
section, where the always-on note belongs.
Adds six tests against a new DemoPlacementTestFixture host, which pins the trigger to a configurable viewport edge / center and can optionally swap in a 600px-tall floating panel. The fixture isn't referenced from any docs story; the test file is the only consumer. New tests: - FlipReorients — bottom placement reorients when the panel can't fit below a trigger anchored to the viewport bottom. - NoFlipKeepsRequestedSide — same setup with shouldFlip: false keeps the requested side, overflow and all. - OffsetMovesAlongPlacementAxis — offset: 40 shifts translateY by ~40 px for a 'bottom' placement. - CrossOffsetMovesAlongTriggerEdge — crossOffset: 40 shifts translateX without materially affecting translateY. - ContainerPaddingMovesPanelInward — with the trigger near the right edge, a larger containerPadding pulls the panel further inside the boundary. - OnPlacementChangeFiresOnChangeOnly — callback is silent when the computed placement matches the requested one and fires once when flip reorients. DemoPlacementTestFixture also exposes placementChanges (records each onPlacementChange invocation since the last rebind) and the controller field directly, so tests can read firing semantics or call recompute() without going through indirection.
…ways-on size Reconcile two behavioural deviations from 1st-gen that the migration plan didn't justify against the cost of breaking consumer parity. Middleware order — restored to 1st-gen: offset → shift → flip → size. The 2nd-gen had switched to offset → flip → shift → size on the rationale that it was 'Floating UI's canonical recommended order'; Floating UI doesn't actually prescribe one, and the two orders produce different positions in edge cases (trigger near a corner, panel close to viewport edge). The 1st-gen pattern is what downstream consumers have been seeing for years — no upside to changing it. size middleware — now always installed. Dropped the constrainSize option. The apply callback uses 1st-gen semantics: it tracks initialHeight from the first un-constrained compute as the baseline, writes max-width on every compute, and writes max-height only when isConstrained is true (content overflows the available space). stop() now unconditionally clears max-height / max-width and resets initialHeight. The ConstrainSize story was renamed to SizeAlwaysClamps and reframed as documentation of the always-on behaviour rather than an opt-in toggle. PlacementOptions.constrainSize is removed; the Playground demo's constrainSize property + attribute + bind entry are gone; the API table no longer lists constrainSize; the test that previously asserted 'constrainSize applies max-height' was rewritten to use the test fixture with tallFloating + triggerPosition='bottom-center' + shouldFlip=false so the overflow scenario is deterministic.
…-gen) The controller now installs Floating UI's arrow middleware when a tipElement is passed in PlacementOptions, matching 1st-gen behaviour. After every compute the controller writes inline translate on the tip element using the same pattern as 1st-gen: - top:0 reset for left/right placements (arrow on a vertical edge) - left:0 reset for top/bottom placements (arrow on a horizontal edge) - translate carries the arrow.x / arrow.y from middlewareData CSS positions the tip element relative to the floating element's edge (typically with a negative offset for the half-size of the arrow); the controller only slides it along that edge so it stays pointing at the trigger's center as shift moves the floating panel. API additions: - PlacementOptions.tipElement?: HTMLElement - PlacementOptions.tipPadding?: number (default 8) - stop() now clears the tip element's inline translate/top/left Demo + tests: - New demo-placement-arrow: trigger + floating panel with a CSS triangle tip. The host passes the tip to the controller. - New Arrow story in the docs page (section-order: 8) under Behaviors. - New ArrowMiddlewarePositionsTip test asserts the tip receives a numeric inline translate after the first compute. - Middleware-stack docs and API options table updated.
…ed trigger - Trigger button: drop the fixed 48x48 sizing and use min-block-size + inline padding so 'Trigger' fits naturally without overflowing. - Default placement: switch from 'bottom' to 'bottom-end' so the tip's computed offset from the floating panel's center is clearly visible (with 'bottom', the tip would sit at the center and there'd be nothing visually distinct from a CSS-centered tip).
Renames the references in code comments and story prose only — no behaviour change. Affects the middleware-stack JSDoc, the controller's inline comments about the order match and tip-positioning pattern, the SizeAlwaysClamps story's 'matches gen1 behaviour' note, and the appendix 'Relationship to gen1 PlacementController' section.
…ghten assertions Move the behavioral tests off the interactive demo-placement-playground host onto the lean, property-driven demo-placement-test-fixture so they no longer depend on the demo's controls, placement picker, or layout. Tighten loose assertions to the actual expected geometry: start/end alignment gap, below-trigger position, offset and cross-offset deltas, container-padding inward shift, and the flip-reorients placement value.
Replace the loose regex check on the tip's serialized translate with direct style and geometry assertions: the controller pins the tip to the floating edge (left: 0) and the arrow middleware centers it on the trigger. Read the tip and trigger rects directly and assert their centers align.
The size middleware's apply callback writes baseline state (isConstrained, initialHeight) during computePosition. A rapid start()/stop() replacement could let a stale in-flight compute bleed that state into the new session, so bail when the session has been replaced.
|
Contributor
📚 Branch Preview Links🔍 First Generation Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
Addresses PR review: the size middleware no longer writes max-width / max-height inline (which overrode a component's intended CSS max-size, e.g. Tooltip wouldn't wrap until viewport width). Instead it exposes --swc-placement-available-width / --swc-placement-available-height on the floating element; components opt in via min() so their intended size wins and overflow stays their concern (resolving the box-sizing overflow too). - stop() removes the custom properties; isConstrained kept as an informational flag (no longer drives an inline max-height) - document the contract + consumer pattern on the controller and in stories - demo-placement-constrain-size consumes the props so it clamps and scrolls - tighten the size test to assert real bounded values; no inline max-* written
Addresses PR review: logical primary sides must resolve to the correct physical side here, not in CSS, or the panel lands on the wrong side in RTL. - toFloatingPlacement takes a direction arg; start->right / end->left in RTL (and start-top -> right-start, etc.) - the controller reads getComputedStyle(trigger).direction (falling back to the floating element for a VirtualTrigger), honoring a scoped dir, not document.dir - logical alignment suffixes (bottom-start, top-end) pass through unchanged so Floating UI's own RTL handling flips them without double-flipping - actualPlacement stays physical; add RTL conversion tests
2 tasks
rubencarvalho
changed the title
# Conflicts: # 2nd-gen/packages/core/controllers/placement-controller/src/placement-controller.ts # 2nd-gen/packages/core/controllers/placement-controller/src/types.ts # 2nd-gen/packages/core/controllers/placement-controller/stories/demo-hosts.ts # 2nd-gen/packages/core/controllers/placement-controller/stories/placement-controller.stories.ts # 2nd-gen/packages/core/controllers/placement-controller/test/placement-controller.test.ts # 2nd-gen/packages/swc/.storybook/DocumentTemplate.mdx # 2nd-gen/packages/swc/.storybook/main.ts # 2nd-gen/packages/swc/.storybook/preview.ts
Collaborator
Coverage Report for CI Build 27818294657Warning No base build found for commit Coverage: 96.24%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
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
2 participants
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.
Description
Integration branch for the Popover 1st-gen → 2nd-gen migration (Epic SWC-1993). Each migration phase is opened as a separate PR targeting this branch and merges here as it completes. This PR stays in draft until all phases are merged in, then it is readied for
main.Phase PRs (target this branch)
(Former standalone Setup PR #6353 was consolidated into #6354.)
Related issue(s)
Notes
Kept in draft until all phase PRs are merged in. The combined Phase 4+5 PR (#6357) is functional but still WIP.