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.
211 lines
15 KiB
211 lines
15 KiB
|
4 months ago
|
# Audit: select
|
||
|
|
audit-version: 1
|
||
|
|
audited-at: 2026-06-26
|
||
|
|
scope: ['soma', 'sema'] (eidos recipe present though 'eidos' not in morfo scope — see systemic SCOPE-DRIFT)
|
||
|
|
files:
|
||
|
|
- provider: src/uix/soma/components/select/select-provider.svelte.ts (present)
|
||
|
|
- morfo: src/uix/morfo/components/select.ts (present)
|
||
|
|
- recipe: src/uix/eidos/components/select/select.css + lib/recipes/base.ts §select (present)
|
||
|
|
- demo: web/routes/uix/components/select (present, not yet inspected this pass)
|
||
|
|
- test: src/uix/soma/components/select/select-provider.svelte.test.ts (present, jsdom)
|
||
|
|
- readme: soma + eidos README present (not yet inspected this pass)
|
||
|
|
|
||
|
|
## Summary
|
||
|
|
Select is the canonical hard overlay: portal-split (`data-color` on trigger+content via TSC v2.2),
|
||
|
|
virtual focus (aria-activedescendant), typeahead, dismissal-with-trigger-exclusion, floating + scroll-lock.
|
||
|
|
The soma provider respects the eidos→soma frontier (zero eidos imports) and uses `SvelteMap` for the label
|
||
|
|
registry (A33-correct). But the audit found **2 contract-vs-behavior contradictions** (focus.trap declared
|
||
|
|
yet not implemented; eidos styles two parts the morfo never declares), a **keyboard double-route with a real
|
||
|
|
behavioral asymmetry + an off-by loop bug**, an **untrusted-selector fragility**, an **A31 O(N²) selection
|
||
|
|
read**, and theming-coherence drift (magic z-index, an `em`-literal font-size that escapes the px/rem guard).
|
||
|
|
Test coverage pins only the easy path (selection logic) — every hard path (keyboard, typeahead, dismissal,
|
||
|
|
focus return) is untested, jsdom-only.
|
||
|
|
|
||
|
|
Counts: CRITICAL 0 · HIGH 4 · MEDIUM 5 · LOW 2.
|
||
|
|
|
||
|
|
## Findings
|
||
|
|
|
||
|
|
### HIGH: Untrusted value interpolated into querySelector without CSS.escape <!-- id: select-001 -->
|
||
|
|
- dimension: C
|
||
|
|
- rule: behavioral (dimension-C selector robustness) + active_architecture §7.5 local-read frontier
|
||
|
|
- location: select-provider.svelte.ts:244-246 (`scrollSelectedIntoView`)
|
||
|
|
- evidence:
|
||
|
|
```ts
|
||
|
|
const selectedEl = container.querySelector<HTMLElement>(
|
||
|
|
`[${attrs.item}][data-value="${selectedValue}"]`
|
||
|
|
);
|
||
|
|
```
|
||
|
|
`selectedValue = this.opts.value.current[0]` is consumer-supplied option-value text, interpolated raw.
|
||
|
|
- impact: a value containing `"`, `]`, or a backslash produces an invalid selector → `querySelector`
|
||
|
|
throws (`SyntaxError`) on open, or silently matches nothing → the selected item never scrolls into view
|
||
|
|
and `highlightedId` is left stale. Blast radius: any app whose option values aren't plain identifiers
|
||
|
|
(URLs, emails, JSON-ish keys, i18n strings with quotes).
|
||
|
|
- repro: `<Select value={['a"b']}>` with an item `value="a\"b"`, open the popup → throw / no scroll.
|
||
|
|
- proposed-fix: `CSS.escape(selectedValue)` in the attribute selector, or reuse the registry/`resolveListItemEl`
|
||
|
|
attribute-equality path (which compares `el.dataset.value` in JS, not via a selector).
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### HIGH: focus.trap / initial:'first-focusable' declared in morfo but provider implements virtual focus (no FocusScope) <!-- id: select-002 -->
|
||
|
|
- dimension: A, B
|
||
|
|
- rule: "don't trust docs over code or code over docs — flag drift" + A17 (virtual vs DOM focus) + M-... morfo contract
|
||
|
|
- location: morfo select.ts:60-65 vs select-provider.svelte.ts (Content provider has Floating + Dismissal +
|
||
|
|
ScrollLock but NO `FocusScope`); A17 declares Select as the canonical **virtual-focus** component.
|
||
|
|
- evidence: morfo declares
|
||
|
|
```ts
|
||
|
|
focus: { initial: 'first-focusable', trap: true, return: 'trigger', restore: true }
|
||
|
|
```
|
||
|
|
The Content provider never instantiates `FocusScope`; focus stays on the Trigger (`aria-activedescendant`),
|
||
|
|
Content is `tabindex: -1`. A focus trap + "focus first focusable item" is the *roving/DOM-focus* model,
|
||
|
|
which contradicts the implemented virtual-focus model.
|
||
|
|
- impact: the `focus` block is dead contract — nothing consumes `trap`/`initial`. Worse, it misdescribes the
|
||
|
|
component to any reader/tool that trusts the morfo (and to a future FocusScope wiring that would BREAK
|
||
|
|
virtual focus if someone "implements the declared trap"). `return: 'trigger'` IS honored manually
|
||
|
|
(`handleClose` → `dom.focus(triggerRef)`), so half the block is real and half is fiction.
|
||
|
|
- repro: static — read morfo vs provider.
|
||
|
|
- proposed-fix: change the morfo `focus` to reflect virtual focus (`initial: 'none'` / trap:false, keep
|
||
|
|
return:'trigger'), OR document why a listbox-virtual-focus component carries a trap declaration. Cross-check
|
||
|
|
combobox (same strategy) for the same drift.
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### HIGH: Two keyboard routes duplicate index-math; content route uses real-DOM-focus (contradicts virtual focus) and diverges from the trigger route <!-- id: select-003 -->
|
||
|
|
- dimension: B, G
|
||
|
|
- rule: A17 (never mix virtual + DOM focus) + dimension-B keyboard-route divergence + dimension-G redundancy
|
||
|
|
- location: trigger route select-provider.svelte.ts:304-387 (virtual: `highlightedId` + `findIndex(el.id===…)`);
|
||
|
|
content route :559-612 (real focus: `active = dom.activeElement(container)` + `items.indexOf(active)`)
|
||
|
|
- evidence: the two `onkeydown` handlers each re-implement next/prev/Home/End/typeahead index math.
|
||
|
|
The content route resolves the current index from `activeElement` — but Select keeps focus on the trigger
|
||
|
|
(virtual focus), so inside the content route `currentIndex` is ~always `-1`. Divergent no-selection landing:
|
||
|
|
- trigger route, ArrowUp from no highlight → `targetIndex = items.length - 1` (LAST). (:337-344)
|
||
|
|
- content route, ArrowUp from no active, `loop=false` → `Math.max(-1-1,0) = 0` (FIRST); `loop=true` →
|
||
|
|
`(-1-1+n)%n = n-2` (second-to-last — an off-by bug). (:577-581)
|
||
|
|
- impact: maintenance hazard (one nav model expressed twice) + a latent correctness bug in the content
|
||
|
|
route's loop path + an A17 violation (content route reads `activeElement` though the component is
|
||
|
|
virtual-focus). The content route is largely unreachable in normal operation, which makes it untested
|
||
|
|
dead-ish code that will rot.
|
||
|
|
- repro: force focus into the content element and press ArrowUp with `loop` on → lands on n-2, not last.
|
||
|
|
- proposed-fix: extract one pure `nextIndex(curr, key, len, loop, dir)` helper unit-tested once; delete the
|
||
|
|
content route or, if a real-focus fallback is wanted, make it consistent with the trigger route's
|
||
|
|
no-selection semantics. (Shared extraction candidate — see SHARED_EXTRACTION directional-nav.)
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### HIGH: Tests pin selection logic only — keyboard, typeahead, dismissal-exclusion, focus-return untested; jsdom-only <!-- id: select-004 -->
|
||
|
|
- dimension: F
|
||
|
|
- rule: dimension-F honesty check ("complex behavior tested only for its easy path")
|
||
|
|
- location: select-provider.svelte.test.ts (3 tests: single-select, multi-select toggle, getItems disabled-filter)
|
||
|
|
- evidence: no test exercises `SelectTriggerProvider.onkeydown` / `SelectContentProvider.onkeydown` (the
|
||
|
|
index math in select-003), `Typeahead`, the `onInteractOutside` trigger-exclusion (:532-541), focus return
|
||
|
|
on close (:217), or `highlightedId`/`aria-activedescendant` sync. `@vitest-environment jsdom` (:1) — focus
|
||
|
|
and layout aren't faithful for a portal+floating+virtual-focus widget.
|
||
|
|
- impact: the highest-risk behaviors (and the select-001/002/003 defects) would not be caught by the suite.
|
||
|
|
Regressions in keyboard nav ship green.
|
||
|
|
- repro: n/a (coverage gap).
|
||
|
|
- proposed-fix: add provider tests for both keyboard routes (incl. the loop boundary), typeahead match,
|
||
|
|
dismissal trigger-exclusion, and a client/Playwright test for focus return + activedescendant.
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### MEDIUM: Eidos recipe styles `[data-select-indicator]` and `[data-select-item-indicator]` — parts the morfo never declares <!-- id: select-005 -->
|
||
|
|
- dimension: A, D
|
||
|
|
- rule: active_architecture §7.6 ("Eidos consume DOM y data-* … debe estar declarado en morfo") + dimension-D frontier
|
||
|
|
- location: select.css:149-163 (`[data-select-indicator]`), :340-358 (`[data-select-item-indicator]`).
|
||
|
|
morfo parts list (select.ts:67-259) has no `Indicator` / `ItemIndicator`.
|
||
|
|
- evidence: the recipe binds layout + the open-state chevron rotation + the check animation to two
|
||
|
|
`data-select-*` attributes that are NOT morfo parts (and have no `kind:'internal'/'private'` declaration).
|
||
|
|
- impact: undeclared contract surface — eidos-lint would classify these as eidos-only/invalid; renaming or
|
||
|
|
removing the eidos wrapper's decorative spans silently breaks the rules with no morfo signal. The
|
||
|
|
`data-{component}-{part}` namespace is being used for parts outside the morfo.
|
||
|
|
- repro: `scripts/eidos-lint.ts select` would surface `[data-select-indicator]` as not morfo-backed.
|
||
|
|
- proposed-fix: declare `Indicator` + `ItemIndicator` in the morfo as `kind:'internal'` decorative parts
|
||
|
|
(they carry no ARIA), or document them as eidos-only in the eidos README §recipe.
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### MEDIUM: Provider emits `aria-invalid` on Trigger; morfo's Trigger does not declare it <!-- id: select-006 -->
|
||
|
|
- dimension: A
|
||
|
|
- rule: dimension-A ("undeclared attr emitted by the provider but absent from the morfo") + 2-of-3
|
||
|
|
- location: select-provider.svelte.ts:398 (`'aria-invalid': this.provider.opts.invalid.current || undefined`);
|
||
|
|
morfo Trigger `aria` (select.ts:101-118) declares type/haspopup/expanded/controls/activedescendant/required —
|
||
|
|
no `aria-invalid`. (The morfo puts `aria-invalid` only on the **Provider** part, :83.)
|
||
|
|
- evidence: the combobox-role trigger gets `aria-invalid` from soma, off-contract.
|
||
|
|
- impact: a real, screen-reader-meaningful ARIA attr lives only in the provider — the morfo (the cross-layer
|
||
|
|
source of truth) under-describes the trigger; eidos can't reliably select on it; drift risk on rename.
|
||
|
|
- proposed-fix: move `aria-invalid` to the Trigger part in the morfo (conditional on `invalid`), let
|
||
|
|
`renderProps()` supply it.
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### MEDIUM: Per-item `isSelected` reads the whole `value` array through the provider (A31 O(N²)) <!-- id: select-007 -->
|
||
|
|
- dimension: B
|
||
|
|
- rule: A31 (per-entity `$derived` must not read global state through the provider)
|
||
|
|
- location: select-provider.svelte.ts:708 (`isSelected = $derived.by(() => this.provider.isSelected(value))`)
|
||
|
|
→ :158-160 (`isSelected(v) { return this.opts.value.current.includes(v) }`)
|
||
|
|
- evidence: each Item's `isSelected` derivation calls a provider method that reads the shared `value` array
|
||
|
|
and runs `.includes` (O(N)). On any value change all N items re-derive → O(N²). Matches the A31 incident
|
||
|
|
signature (Listbox rovingTarget).
|
||
|
|
- impact: fine for single-select / few items; degrades with large multi-select lists (the documented A31
|
||
|
|
"works with 5, hangs with 30+" class).
|
||
|
|
- proposed-fix: lift `selectedSet = new SvelteSet(value)` on the provider; item derivation becomes
|
||
|
|
`selectedSet.has(value)` (O(1)).
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### MEDIUM: `item-description-font-size: '0.85em'` — literal font-size in recipe escapes the px/rem coherence guard <!-- id: select-008 -->
|
||
|
|
- dimension: E-bis
|
||
|
|
- rule: THEMING §5 ("ningún token font-size-* de recipe puede ser literal — DEBE referenciar --font-size-*") + R-2.2
|
||
|
|
- location: lib/recipes/base.ts:2243 (`'item-description-font-size': '0.85em'`); consumed select.css:330.
|
||
|
|
- evidence: every other `select.*font-size-*` token references `var(--font-size-*)` (1:1, :2154-2158). The
|
||
|
|
description token is a raw `em`, breaking the type-scale link (`--scaling` / `applyTypeScale` no longer
|
||
|
|
reach it). The §5 guard checks **px/rem** literals — an `em` literal slips through (see VALIDATOR_GAPS).
|
||
|
|
- impact: description text stops following the typographic scale; a theme that bumps `--font-size-*` won't
|
||
|
|
move it. Silent.
|
||
|
|
- proposed-fix: tokenize against the scale (e.g. one step below the item: `var(--font-size-sm)` at md) or
|
||
|
|
keep relative but make the guard reject bare `em` font-size literals too.
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### MEDIUM: `content-z: '80'` — magic z-index literal in recipe <!-- id: select-009 -->
|
||
|
|
- dimension: E-bis
|
||
|
|
- rule: THEMING §35 Bloque C (z-index magic numbers → `--z-index-*` tokens) + feedback "no intentional magic numbers"
|
||
|
|
- location: lib/recipes/base.ts:2206 (`'content-z': '80'`); consumed select.css:166.
|
||
|
|
- evidence: a bare `80` instead of a `var(--z-index-{layer})` token.
|
||
|
|
- impact: overlay stacking is a cross-component decision; a per-recipe integer can't be re-coordinated
|
||
|
|
centrally. Likely recurs in combobox/popover/dropdown content-z → promote to systemic if confirmed.
|
||
|
|
- proposed-fix: map to `--z-index-popover` (or the canonical floating layer token) and verify the ladder.
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### LOW: Magic letter-spacing + spacing literals where canonical scales exist <!-- id: select-010 -->
|
||
|
|
- dimension: E-bis
|
||
|
|
- rule: THEMING §6 (slots/scales) + §35 (`--tracking-*`) + R-2.3
|
||
|
|
- location: lib/recipes/base.ts:2232 (`'group-heading-letter-spacing': '0.04em'`), :2241
|
||
|
|
(`'item-description-mt': '0.125rem'`).
|
||
|
|
- evidence: `0.04em` for uppercase group heading should be `var(--tracking-caps)`; `0.125rem` (=2px) should
|
||
|
|
be `var(--space-0-5)`.
|
||
|
|
- impact: minor drift; values won't track density/scale changes.
|
||
|
|
- proposed-fix: remap to `--tracking-caps` and `--space-0-5`.
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
### LOW: `SelectColor = ColorRole` (all 9) but recipe implements 8 accents (no tertiary) <!-- id: select-011 -->
|
||
|
|
- dimension: E-bis, G
|
||
|
|
- rule: THEMING §4 "subset por componente" + G-1.2/G-1.3 (type union must match implemented subset)
|
||
|
|
- location: types.ts:20 (`export type SelectColor = ColorRole`) vs recipe `_accent-*` cascades cover
|
||
|
|
secondary/neutral/affirm/fulfill/risk/threat/loss + host=primary (no `color:tertiary`).
|
||
|
|
- evidence: `data-color='tertiary'` is type-valid but silently falls back to the primary host default.
|
||
|
|
- impact: API promises a color the recipe doesn't render; minor.
|
||
|
|
- proposed-fix: narrow `SelectColor` to the implemented subset (or add the tertiary accent). Decide whether a
|
||
|
|
neutral form control should expose the full evaluative palette at all (style observation below).
|
||
|
|
- fix-status: open
|
||
|
|
|
||
|
|
## No-findings dimensions
|
||
|
|
- **D (frontier, severe half):** the soma provider imports only soma layers + the morfo — **zero eidos
|
||
|
|
imports**, no reach into eidos internals. Frontier direction respected. (The undeclared-part issue is
|
||
|
|
filed under A/D as select-005, but the provider→eidos direction is clean.)
|
||
|
|
- **A (naming):** `data-select` / `data-select-{part}` naming is consistent across morfo, provider and CSS;
|
||
|
|
no `data-soma-*`. Root part is `provider` (A2-correct).
|
||
|
|
- **A33 / A30 / A6:** label registry uses `SvelteMap` (A33 ✓); trigger/content ref mirror via `onRefChange`
|
||
|
|
not `$effect` (A30 ✓); typeahead timer disposed in `$effect` return (A6 ✓).
|
||
|
|
- **E (TSC scope):** multi-part `parts: ['trigger','content']` cascade is the textbook TSC v2.2 case and is
|
||
|
|
used correctly; no eager-substitution `:root` derived-token bug; the orphan `_accent-solid` was correctly
|
||
|
|
dropped. Clean.
|
||
|
|
|
||
|
|
## Style observations (opinion, non-blocking)
|
||
|
|
- Exposing the full evaluative palette (`affirm/fulfill/risk/threat/loss`) as a *selectable accent color* on
|
||
|
|
a neutral form control reads as over-rich; a select's accent is brand-ish (hierarchy) more than evaluative.
|
||
|
|
Not a rule violation (Button exposes all 9), but worth a subset decision.
|
||
|
|
- The Content `onkeydown` real-focus fallback (select-003) feels like leftover from a pre-virtual-focus
|
||
|
|
iteration. If it isn't reachable, deleting it would remove a whole class of divergence risk.
|