You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
svelte-kit-vice/audit/components/dialog.md

9.9 KiB

Audit: dialog

audit-version: 1 audited-at: 2026-06-26 scope: ['soma', 'sema'] (eidos recipe present though 'eidos' not in morfo scope — systemic SCOPE-DRIFT) files:

  • provider: src/uix/soma/components/dialog/dialog-provider.svelte.ts (present)
  • morfo: src/uix/morfo/components/dialog.ts (present)
  • recipe: src/uix/eidos/components/dialog/dialog.css + lib/recipes/base.ts §dialog (present)
  • demo: web/routes/uix/components/dialog (present, not inspected this pass)
  • test: src/uix/soma/components/dialog/dialog-provider.svelte.test.ts (present, jsdom)
  • readme: present (not inspected this pass)

Summary

Dialog is the reference for polymorphic close (book §5.3) and gets most of the hard stuff right: nesting via watch+untrack (loop-safe), FocusScope/ScrollLock derived from modal (A16), correct two-moments ordering (emit-before-state on close with sequence 'pre'), a thoughtfully tight data-color subset (neutral/risk/ threat), and a clean eidos→soma frontier. Two real defects stand out: the Content provider double-writes role/aria-roledescription against the syncAttrs path (an a11y race for the alertdialog variant), and the [data-dialog-trigger] CSS envelope is stale dead/conflicting chrome left behind after the trigger migrated to a composed <Button>. Plus the systemic magic z-index and jsdom-only test gap.

Counts: CRITICAL 0 · HIGH 2 · MEDIUM 2 · LOW 2.

Findings

HIGH: Content double-writes role and aria-roledescription (syncAttrs + manual props) — racy role for alertdialog

  • dimension: D, A
  • rule: active_architecture §7.7 ("lo que dom.apply escribe, Svelte no lo renderiza — una sola autoridad por atributo") + rule 7.8
  • location: dialog-provider.svelte.ts:391-397 (Content registered syncAttrs: true) vs :460-463 (props override)
  • evidence:
    this.runtimePart = this.provider.runtime.part('content', { …, syncAttrs: true });
    …
    readonly props = $derived.by(() => ({
        ...this.runtimePart.props,
        role: this.provider.opts.variant.current,      // 'dialog' | 'alertdialog'
        'aria-roledescription': undefined,             // nukes the morfo's value
        …
    }));
    
    The morfo declares Content role: 'dialog' (morfo:150) and aria-roledescription (:211-215); with syncAttrs: true the runtime writes both via dom.apply. The provider then ALSO sets role (to variant) and aria-roledescription: undefined in the Svelte-rendered props. Two authorities per attribute.
  • impact: for variant='alertdialog', dom.apply writes role="dialog" while Svelte writes role="alertdialog" — order-dependent ping-pong; the accessible role of an alert dialog is non-deterministic (a WAI-ARIA correctness break). aria-roledescription is written-then-blanked. The morfo (the cross-layer source of truth) says role='dialog' but the component renders a variant-dependent role it can't express.
  • repro: render <Dialog variant="alertdialog">, inspect Content role across effect ticks.
  • proposed-fix: make role morfo-expressible (bind a propRef('variant')/stateRef so the morfo owns the variant-dependent role), OR drop syncAttrs for Content and supply role/aria via renderProps() + a single override. Don't mix syncAttrs with manual same-attr writes.
  • fix-status: open

HIGH: [data-dialog-trigger] CSS envelope is dead/conflicting after the trigger migrated to a composed <Button>

  • dimension: E-bis, D
  • rule: THEMING §16 "no solapar" (overlapping/mis-owned styles) + COMPONENT_GUIDE §4 (compose, don't reinvent) + project memory project_dialog_button_2026-06-20
  • location: dialog.css:22-54 ([data-dialog-trigger] full button envelope) vs dialog-trigger.svelte:17-26 (renders <Button {...props}> via soma child); recipe dialog.trigger-font-size (base.ts:727).
  • evidence: the eidos <Dialog.Trigger> composes the framework <Button> through soma's child snippet, so the rendered element carries BOTH data-button AND data-dialog-trigger. The dialog recipe still defines a complete competing chrome on [data-dialog-trigger] (height/padding/border/background/hover/disabled + --dialog-trigger-font-size). Both single-attribute selectors have equal specificity → source-order decides; the two recipes fight over the same element. The CSS header comment (:15-21, "the trigger needs its own envelope here") is stale — it predates the Button migration.
  • impact: silent visual incoherence (two sources for one button's chrome), order-dependent; the --dialog-trigger-* tokens are orphan knobs. Exactly the overlap E-bis.5 forbids.
  • repro: inspect a <Dialog.Trigger> — it has data-button + data-dialog-trigger; toggle CSS order to see the chrome change.
  • proposed-fix: delete the [data-dialog-trigger] envelope + its tokens (Button owns the chrome); keep only a dialog-specific concern if any (there is none — the Close recipe is the correct model, dialog.css:319-325).
  • fix-status: open

MEDIUM: overlay-z: '70' magic z-index literal (content = calc(overlay-z + 1))

  • dimension: E-bis
  • rule: THEMING §35 Bloque C (z-index magic → --z-index-* tokens) + feedback "no intentional magic numbers"
  • location: lib/recipes/base.ts:731 ('overlay-z': '70'); dialog.css:67 + :91 (calc(var(--dialog-overlay-z) + 1)).
  • evidence: a bare 70 where a var(--z-index-modal) token belongs. Same class as select.content-z: '80' → candidate systemic finding (overlay stacking ladder should be one tokenized source, not per-recipe integers).
  • impact: the modal/popover/overlay z-ladder can't be coordinated centrally; integers drift apart.
  • proposed-fix: map dialog overlay/content to --z-index-modal/--z-index-modal-content (or the canonical ladder) and audit all overlay recipes together.
  • fix-status: open

MEDIUM: Tests pin open/close/disabled/modal-derivation; dismissal paths, polymorphic causes, nesting, focus untested; jsdom-only

  • dimension: F
  • rule: dimension-F honesty check
  • location: dialog-provider.svelte.test.ts (3 tests)
  • evidence: only dismissWith('dismiss') of the five causes is exercised (save/fail/cancel/ dismiss-outside and their distinct data-last-action + semantic.family/intent are not). No test for the Escape/interact-outside dismissal wiring, nestedOpenCount increment/decrement, or focus trap+return (jsdom can't exercise FocusScope faithfully — @vitest-environment jsdom :1).
  • impact: the polymorphic-close matrix (the component's headline feature) and the focus/nesting behavior ship largely unverified; dialog-001's role race wouldn't be caught.
  • proposed-fix: parametrize a test over all five DISMISS_CAUSES asserting data-last-action + emitted family/intent; add nesting-count tests; add a client/Playwright test for focus trap + return.
  • fix-status: open

LOW: Provider part declares data-state/data-color/data-disabled but is kind:'virtual' / defaultElement:'none'

  • dimension: A
  • rule: dimension-A (part data must reach an element) + 2-of-3
  • location: morfo dialog.ts:96-126 (Provider kind:'virtual', defaultElement:'none', yet declares 3 data attrs); provider registers it syncAttrs: true with optional ref (provider renders no DOM).
  • evidence: the DialogProvider renders only children (no host element); a virtual part with no element can't carry data-state/data-color. The same data also lives on Content (where it's actually consumed by the CSS).
  • impact: the Provider-part data declarations are likely unreachable (dead contract) unless the eidos root wrapper renders an element and applies them — needs confirmation. If dead, they mislead readers/tools.
  • proposed-fix: confirm whether any element carries data-dialog (eidos root). If not, drop the data from the virtual Provider part (Content already carries state/color), or give the provider a real host.
  • fix-status: open
  • dimension: A, D
  • rule: active_architecture §7.6 (eidos consumes declared data-*) — acknowledged exception
  • location: dialog.css:260-290 (commented "Layout shells (eidos-only, no morfo backing)").
  • evidence: pure layout containers with no ARIA/behavior, styled by the recipe but absent from the morfo. The CSS comment is explicit about it.
  • impact: minimal (no behavior/ARIA), but it's the same undeclared-part pattern as select-005 — worth a consistent disposition (eidos-only parts either get kind:'internal' morfo entries or a documented allow-list).
  • proposed-fix: adopt a project-wide convention for eidos-only decorative/layout parts (see SHARED_EXTRACTION / systemic UNDECLARED-EIDOS-PARTS).
  • fix-status: open

No-findings dimensions

  • B (behavior): two-moments ordering correct (open/close sequence 'pre' → emit precedes state via await runtime.trigger); nesting count uses watch + untrack (loop-safe, A30-spirit); FocusScope + ScrollLock derive from modal (A16); disabled guards at call-site (rule 10). Clean.
  • C (selectors): no querySelector with interpolated user values in the provider. Clean.
  • D (frontier, severe half): provider imports only soma layers + morfo — zero eidos imports. (role double-write filed under D as dialog-001, but it's a runtime/contract issue, not a cross-layer import.)
  • E (TSC): data-color declared per-part in the morfo (provider+content), CSS consumes it on content; no eager-substitution :root derived bug. Colors fully tokenized. Clean.

Style observations (opinion, non-blocking)

  • The data-color subset (neutral/risk/threat only, with the rationale "a dialog is not a hierarchy and irreversible loss should not be a dialog") is an exemplary application of THEMING §4 subset-por-componente — worth citing as the positive reference for other components.
  • The polymorphic-close DISMISS_CAUSES map is clean and readable; the only gap is test coverage of its breadth.

Powered by TurnKey Linux.