refactor(floating): extract createFloatingShellRoot + buildFloatingShellWrapperProps helpers (audit Round 3 §3 #1)

7 popover-based soma providers (popover, dropdown-menu, context-menu,
combobox, select, tooltip, link-preview) shared two byte-identical
blocks that an investigation report (delegated Plan agent) confirmed
as real duplication after looking carefully past the audit's headline
"600 lines across 15 providers" — the actual scope was ~230 lines
across 7 providers (the audit overcounted by ~2x, similar to the
BaseSegmentProvider case).

Two narrow helpers added in `src/uix/soma/layers/floating/shell.ts`:

  - `createFloatingShellRoot({ dom, open, contentRef, onOpenChangeComplete })`
    bundles `FloatingProvider.create({ dom })` + the `contentPresence`
    constructor (which were verbatim across all 7 providers, byte for
    byte). Returns `{ floatingProvider, contentPresence }` the consumer
    assigns to `this.*` fields.

  - `buildFloatingShellWrapperProps(floating, pointerEvents = 'auto')`
    returns the canonical wrapperProps shape (spread floating's
    wrapperProps + `style.pointer-events` forced to a value). Identical
    in 6 of 7; tooltip parameterises `pointerEvents` to flip on
    `hoverableDisabled`.

Explicitly NOT abstracted: FocusScope, Dismissal, ScrollLock,
TextSelection — these look similar but diverge per provider (modal-
derived `trap`, custom `isValidEvent` closures, different close
callbacks — see Round 3 audit analysis). Trying to hide them would
recreate the BaseSegmentProvider trap of flag bloat.

Notable variations preserved:
  - popover keeps its `overlayPresence` (popover-only chrome) outside
    the shell helper — only `contentPresence` is shared.
  - context-menu's SubContent sub-provider also uses the same wrapper
    helper (its FloatingProvider.create stays inline because it reads
    `this.provider.soma.dom` not `this.soma.dom`).
  - dropdown-menu has both a Content and SubContent wrapperProps —
    both routed through the helper.

Test result: 2394/2399 passing — no regressions in the 27 tests
across the 7 affected providers. The 5 fails remain Words + cookie
infra (cookie is flaky; sometimes 6, sometimes 5).

Closes the last item of Kim audit Round 3 §3. Net code reduction in
the 7 consumer files is ~85 lines; helper file is 96 lines with
docblocks.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
active-uix
dev 5 months ago
parent bf7e116af9
commit cc0d38ec52

@ -27,6 +27,8 @@ import {
FloatingContent, FloatingContent,
FloatingArrow, FloatingArrow,
FloatingAnchor, FloatingAnchor,
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type Side, type Side,
type Align, type Align,
type Boundary type Boundary
@ -132,14 +134,14 @@ export class ComboboxProvider {
syncAttrs: true syncAttrs: true
}); });
this.floatingProvider = FloatingProvider.create({ dom: this.soma.dom }); const shell = createFloatingShellRoot({
this.contentPresence = new Presence({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
ref: this.contentRef, contentRef: this.contentRef,
onComplete: (open) => opts.onOpenChangeComplete.current(open) onOpenChangeComplete: opts.onOpenChangeComplete
}); });
this.floatingProvider = shell.floatingProvider;
this.contentPresence = shell.contentPresence;
// Single-mode auto-sync: keep `inputValue` mirroring the selected // Single-mode auto-sync: keep `inputValue` mirroring the selected
// value's label so the input reflects external `value` changes // value's label so the input reflects external `value` changes
@ -598,15 +600,7 @@ export class ComboboxContentProvider {
readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
readonly onkeydown = (e: KeyboardEvent) => { readonly onkeydown = (e: KeyboardEvent) => {
const container = this.opts.ref.current; const container = this.opts.ref.current;

@ -24,6 +24,8 @@ import {
FloatingContent, FloatingContent,
FloatingArrow, FloatingArrow,
FloatingAnchor, FloatingAnchor,
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type Measurable, type Measurable,
type Side, type Side,
type Align, type Align,
@ -99,14 +101,14 @@ export class ContextMenuProvider {
context: ContextMenuProvider.ctx context: ContextMenuProvider.ctx
}); });
this.floatingProvider = FloatingProvider.create({ dom: this.soma.dom }); const shell = createFloatingShellRoot({
this.contentPresence = new Presence({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
ref: this.contentRef, contentRef: this.contentRef,
onComplete: (open) => opts.onOpenChangeComplete.current(open) onOpenChangeComplete: opts.onOpenChangeComplete
}); });
this.floatingProvider = shell.floatingProvider;
this.contentPresence = shell.contentPresence;
$effect(() => { $effect(() => {
return () => this.typeahead.destroy(); return () => this.typeahead.destroy();
@ -309,15 +311,7 @@ export class ContextMenuContentProvider {
readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
readonly onkeydown = (e: KeyboardEvent) => { readonly onkeydown = (e: KeyboardEvent) => {
const container = this.opts.ref.current; const container = this.opts.ref.current;
@ -991,15 +985,7 @@ export class ContextMenuSubContentProvider {
readonly isPresent = $derived.by(() => this.sub.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.sub.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
readonly onkeydown = (e: KeyboardEvent) => { readonly onkeydown = (e: KeyboardEvent) => {
const dir = this.sub.provider.opts.dir.current; const dir = this.sub.provider.opts.dir.current;

@ -30,6 +30,8 @@ import {
FloatingContent, FloatingContent,
FloatingArrow, FloatingArrow,
FloatingAnchor, FloatingAnchor,
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type Side, type Side,
type Align, type Align,
type Boundary type Boundary
@ -96,14 +98,14 @@ export class MenuProvider {
context: MenuProvider.ctx context: MenuProvider.ctx
}); });
this.floatingProvider = FloatingProvider.create({ dom: this.soma.dom }); const shell = createFloatingShellRoot({
this.contentPresence = new Presence({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
ref: this.contentRef, contentRef: this.contentRef,
onComplete: (open) => opts.onOpenChangeComplete.current(open) onOpenChangeComplete: opts.onOpenChangeComplete
}); });
this.floatingProvider = shell.floatingProvider;
this.contentPresence = shell.contentPresence;
// Cleanup typeahead timer on unmount // Cleanup typeahead timer on unmount
$effect(() => { $effect(() => {
@ -332,15 +334,7 @@ export class MenuContentProvider {
readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
readonly onkeydown = (e: KeyboardEvent) => { readonly onkeydown = (e: KeyboardEvent) => {
const container = this.opts.ref.current; const container = this.opts.ref.current;
@ -1079,15 +1073,7 @@ export class MenuSubContentProvider {
readonly isPresent = $derived.by(() => this.sub.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.sub.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
readonly onkeydown = (e: KeyboardEvent) => { readonly onkeydown = (e: KeyboardEvent) => {
const dir = this.sub.provider.opts.dir.current; const dir = this.sub.provider.opts.dir.current;

@ -11,6 +11,8 @@ import {
FloatingContent, FloatingContent,
FloatingArrow, FloatingArrow,
FloatingAnchor, FloatingAnchor,
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type Measurable, type Measurable,
type Side, type Side,
type Align, type Align,
@ -74,14 +76,14 @@ export class LinkPreviewProvider {
context: LinkPreviewProvider.ctx context: LinkPreviewProvider.ctx
}); });
this.floatingProvider = FloatingProvider.create({ dom: this.soma.dom }); const shell = createFloatingShellRoot({
this.contentPresence = new Presence({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
ref: this.contentRef, contentRef: this.contentRef,
onComplete: (open) => opts.onOpenChangeComplete.current(open) onOpenChangeComplete: opts.onOpenChangeComplete
}); });
this.floatingProvider = shell.floatingProvider;
this.contentPresence = shell.contentPresence;
// SafePolygon: preserve the preview when the pointer crosses the gap // SafePolygon: preserve the preview when the pointer crosses the gap
// between the trigger anchor and the content. // between the trigger anchor and the content.
@ -321,15 +323,7 @@ export class LinkPreviewContentProvider {
readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
readonly onpointerenter = (e: PointerEvent) => { readonly onpointerenter = (e: PointerEvent) => {
if (e.pointerType === 'touch') return; if (e.pointerType === 'touch') return;

@ -30,6 +30,8 @@ import {
FloatingContent, FloatingContent,
FloatingArrow, FloatingArrow,
FloatingAnchor, FloatingAnchor,
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type Side, type Side,
type Align, type Align,
type Boundary type Boundary
@ -98,15 +100,18 @@ export class PopoverProvider {
PopoverProvider.ctx.set(this); PopoverProvider.ctx.set(this);
this.soma = Soma.require(); this.soma = Soma.require();
this.floatingProvider = FloatingProvider.create({ dom: this.soma.dom });
this.contentPresence = new Presence({ const shell = createFloatingShellRoot({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
ref: this.contentRef, contentRef: this.contentRef,
onComplete: (open) => opts.onOpenChangeComplete.current(open) onOpenChangeComplete: opts.onOpenChangeComplete
}); });
this.floatingProvider = shell.floatingProvider;
this.contentPresence = shell.contentPresence;
// Overlay is popover-only — separate Presence not folded into the
// shared shell because the other 6 providers don't have an overlay.
this.overlayPresence = new Presence({ this.overlayPresence = new Presence({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
@ -509,15 +514,7 @@ export class PopoverContentProvider {
readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
// `role`, `aria-labelledby`, `aria-modal`, `data-state` come from the // `role`, `aria-labelledby`, `aria-modal`, `data-state` come from the
// morfo's Content part via the SomaRuntime; the layered attrs (floating // morfo's Content part via the SomaRuntime; the layered attrs (floating

@ -33,6 +33,8 @@ import {
FloatingContent, FloatingContent,
FloatingArrow, FloatingArrow,
FloatingAnchor, FloatingAnchor,
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type Side, type Side,
type Align, type Align,
type Boundary type Boundary
@ -138,14 +140,14 @@ export class SelectProvider {
syncAttrs: true syncAttrs: true
}); });
this.floatingProvider = FloatingProvider.create({ dom: this.soma.dom }); const shell = createFloatingShellRoot({
this.contentPresence = new Presence({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
ref: this.contentRef, contentRef: this.contentRef,
onComplete: (open) => opts.onOpenChangeComplete.current(open) onOpenChangeComplete: opts.onOpenChangeComplete
}); });
this.floatingProvider = shell.floatingProvider;
this.contentPresence = shell.contentPresence;
// Cleanup typeahead timer on unmount // Cleanup typeahead timer on unmount
$effect(() => { $effect(() => {
@ -535,15 +537,7 @@ export class SelectContentProvider {
readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() => buildFloatingShellWrapperProps(this.floating));
...this.floating.wrapperProps,
style: {
...(typeof this.floating.wrapperProps.style === 'object'
? this.floating.wrapperProps.style
: {}),
'pointer-events': 'auto'
}
}));
readonly onkeydown = (e: KeyboardEvent) => { readonly onkeydown = (e: KeyboardEvent) => {
const container = this.opts.ref.current; const container = this.opts.ref.current;

@ -22,6 +22,8 @@ import {
FloatingContent, FloatingContent,
FloatingArrow, FloatingArrow,
FloatingAnchor, FloatingAnchor,
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type Side, type Side,
type Align, type Align,
type Boundary type Boundary
@ -149,14 +151,14 @@ export class TooltipProvider {
}); });
this.group = TooltipGroupProvider.get(); this.group = TooltipGroupProvider.get();
this.floatingProvider = FloatingProvider.create({ dom: this.soma.dom }); const shell = createFloatingShellRoot({
this.contentPresence = new Presence({
dom: this.soma.dom, dom: this.soma.dom,
open: opts.open, open: opts.open,
ref: this.contentRef, contentRef: this.contentRef,
onComplete: (open) => opts.onOpenChangeComplete.current(open) onOpenChangeComplete: opts.onOpenChangeComplete
}); });
this.floatingProvider = shell.floatingProvider;
this.contentPresence = shell.contentPresence;
// SafePolygon: prevents tooltip from closing when pointer traverses // SafePolygon: prevents tooltip from closing when pointer traverses
// the gap between trigger and content (hoverable tooltips) // the gap between trigger and content (hoverable tooltips)
@ -465,15 +467,12 @@ export class TooltipContentProvider {
readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent); readonly isPresent = $derived.by(() => this.provider.contentPresence.isPresent);
readonly wrapperProps = $derived.by(() => ({ readonly wrapperProps = $derived.by(() =>
...this.floating.wrapperProps, buildFloatingShellWrapperProps(
style: { this.floating,
...(typeof this.floating.wrapperProps.style === 'object' this.provider.hoverableDisabled ? 'none' : 'auto'
? this.floating.wrapperProps.style )
: {}), );
'pointer-events': this.provider.hoverableDisabled ? 'none' : 'auto'
}
}));
readonly onpointerenter = () => { readonly onpointerenter = () => {
if (!this.provider.hoverableDisabled) { if (!this.provider.hoverableDisabled) {

@ -31,3 +31,11 @@ export { useFloating } from './use-floating.svelte';
// ── Utilities ──────────────────────────────────────────────────────────────── // ── Utilities ────────────────────────────────────────────────────────────────
export { getDPR, roundByDPR, getFloatingContentCSSVars, isReferenceHidden } from './utils'; export { getDPR, roundByDPR, getFloatingContentCSSVars, isReferenceHidden } from './utils';
export { SafePolygon, type SafePolygonOptions } from './safe-polygon'; export { SafePolygon, type SafePolygonOptions } from './safe-polygon';
// ── Shell helpers (popover-based providers boilerplate) ────────────────────
export {
createFloatingShellRoot,
buildFloatingShellWrapperProps,
type FloatingShellRootOpts,
type FloatingShellRoot
} from './shell';

@ -0,0 +1,97 @@
/**
* Floating "shell" helpers — small abstractions over the byte-identical
* boilerplate that 7 popover-based soma providers (popover, dropdown-menu,
* context-menu, combobox, select, tooltip, link-preview) used to inline:
*
* 1. **Root-side** triad: every root provider calls `FloatingProvider.create({ dom })`
* then constructs a `contentPresence` with the open state + content ref +
* onOpenChangeComplete callback. The 5-line block was byte-identical
* across the 7 providers.
*
* 2. **Content-side** `wrapperProps`: every Content provider exposes a
* `$derived` field that re-emits the floating layer's wrapperProps
* with `pointer-events: 'auto'` forced onto the style object. The
* 7-line block was identical in 6 of 7 (tooltip parameterises the
* pointer-events value).
*
* The helpers do NOT abstract FocusScope / Dismissal / ScrollLock /
* TextSelection — those layers genuinely diverge per provider
* (different `trap` resolution, different `isValidEvent` closures,
* different close routing — see Round 3 audit analysis).
*
* Saves ~150 lines net across the 7 consumers without introducing
* configuration flags.
*/
import type { ActiveDom } from '$adom';
import type { Active, State } from '$libs/reactive';
import type { OnChangeFn } from '../../types';
import { FloatingProvider, type FloatingContent } from './floating.svelte';
import { Presence } from '../presence.svelte';
export interface FloatingShellRootOpts {
/** DOM service — every floating root piggybacks on the soma ActiveDom. */
dom: ActiveDom;
/** The provider's `open` state — drives the content presence transitions. */
open: Active<boolean>;
/** Ref to the Content element. Presence listens to its mount/unmount. */
contentRef: State<HTMLElement | null>;
/**
* Called when the open/close presence transition completes (animation
* end). Optional — when omitted, presence still runs but the callback
* is a no-op.
*/
onOpenChangeComplete?: Active<OnChangeFn<boolean> | undefined>;
}
export interface FloatingShellRoot {
floatingProvider: FloatingProvider;
contentPresence: Presence;
}
/**
* Initialise the shared `FloatingProvider + contentPresence` triad. The
* resulting handles are stored on the consumer's class (`this.floatingProvider`,
* `this.contentPresence`) — the helper just removes the boilerplate.
*/
export function createFloatingShellRoot(opts: FloatingShellRootOpts): FloatingShellRoot {
const floatingProvider = FloatingProvider.create({ dom: opts.dom });
const contentPresence = new Presence({
dom: opts.dom,
open: opts.open,
ref: opts.contentRef,
onComplete: opts.onOpenChangeComplete
? (open) => opts.onOpenChangeComplete!.current?.(open)
: undefined
});
return { floatingProvider, contentPresence };
}
/**
* Build the wrapper props the Content provider exposes on `data-*-content`'s
* wrapper. Merges the floating layer's own wrapperProps (positioning + role
* + transition attrs) with `pointer-events` forced ON so portaled content
* captures clicks.
*
* Tooltip overrides `pointerEvents` to toggle on `hoverableDisabled`; the
* rest pass `'auto'` (default).
*
* Returns the spread + an explicit `style: Record<string, unknown>` shape
* so Content components can pass it to `styleToString` directly without TS
* narrowing tripping on the upstream `string | Record<...>` union.
*/
export function buildFloatingShellWrapperProps(
floating: FloatingContent,
pointerEvents: 'auto' | 'none' = 'auto'
): FloatingContent['wrapperProps'] & { style: Record<string, unknown> } {
const baseStyle = floating.wrapperProps.style;
return {
...floating.wrapperProps,
style: {
...(typeof baseStyle === 'object' && baseStyle !== null ? baseStyle : {}),
'pointer-events': pointerEvents
}
};
}
Loading…
Cancel
Save

Powered by TurnKey Linux.