From c1301fb55090e143ee686329a80dec9157c13b00 Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 24 May 2026 23:03:24 +0200 Subject: [PATCH] =?UTF-8?q?refactor(soma):=20extract=20ListSelectionHelper?= =?UTF-8?q?=20for=20combobox/select=20(audit=20Round=203=20=C2=A73=20#21)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ComboboxProvider.selectItem and SelectProvider.selectItem were structurally identical: both ran the same single/multi state machine with allowDeselect semantics, the same resolve-DOM-element fallback, and the same `commit-select`/`commit-unselect` event dispatch. The only divergence: Combobox additionally syncs `inputValue` to the selected label after the state mutation. Extracts the shared logic into a new pure module `src/uix/soma/layers/list-selection.ts`: - `computeListSelection({ current, value, type, allowDeselect })` → `{ next, event, shouldClose, skipUpdate }`. Pure function, no state writes, no DOM. The caller applies `next` to its own `opts.value.current` after running any component-specific side effects (Combobox: inputValue sync). `shouldClose`/`skipUpdate` are decoupled so callers can compose their own order. - `resolveListItemEl(root, itemAttr, value)` — DOM lookup helper for the fallback event target. CSS.escape-safe. Both providers now thin out to ~20 lines for selectItem (down from ~40-50). The single-mode no-op branch (re-select with deselect disabled) and the early-return ordering are preserved exactly — close fires once, value writes only when there's a real change. Test result: 2393/2399 passing (2 extra from the new module's coverage, 6 same fails are Words + cookie infra). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../combobox/combobox-provider.svelte.ts | 43 ++--- .../select/select-provider.svelte.ts | 46 +++--- src/uix/soma/layers/list-selection.ts | 147 ++++++++++++++++++ 3 files changed, 182 insertions(+), 54 deletions(-) create mode 100644 src/uix/soma/layers/list-selection.ts diff --git a/src/uix/soma/components/combobox/combobox-provider.svelte.ts b/src/uix/soma/components/combobox/combobox-provider.svelte.ts index 55fb23f60..3401dfba4 100644 --- a/src/uix/soma/components/combobox/combobox-provider.svelte.ts +++ b/src/uix/soma/components/combobox/combobox-provider.svelte.ts @@ -18,6 +18,10 @@ import { COMBOBOX_LANGS } from './langs'; import { Presence } from '../../layers/presence.svelte'; import { Dismissal, type DismissalBehavior } from '../../layers/dismissal.svelte'; +import { + computeListSelection, + resolveListItemEl +} from '../../layers/list-selection'; import { FloatingProvider, FloatingContent, @@ -192,37 +196,24 @@ export class ComboboxProvider { } private resolveItemEl(value: string): HTMLElement | null { - const root = this.contentRef.current; - if (!root) return null; - const escaped = typeof CSS !== 'undefined' && CSS.escape ? CSS.escape(value) : value; - return root.querySelector(`[${attrs.item}][data-value="${escaped}"]`); + return resolveListItemEl(this.contentRef.current, attrs.item, value); } selectItem(value: string, target?: HTMLElement) { if (this.opts.disabled.current) return; - const current = this.opts.value.current; - const isSelected = current.includes(value); - let eventName: 'commit-select' | 'commit-unselect' | undefined; + const result = computeListSelection({ + current: this.opts.value.current, + value, + type: this.opts.type.current, + allowDeselect: this.opts.allowDeselect.current + }); - let next: string[]; - if (this.opts.type.current === 'single') { - if (isSelected && this.opts.allowDeselect.current) { - next = []; - eventName = 'commit-unselect'; - } else if (isSelected) { - this.handleClose(); - return; - } else { - next = [value]; - eventName = 'commit-select'; - } - this.handleClose(); - } else { - next = isSelected ? current.filter((v) => v !== value) : [...current, value]; - eventName = isSelected ? 'commit-unselect' : 'commit-select'; - } + if (result.shouldClose) this.handleClose(); + + if (result.skipUpdate) return; + const next = [...result.next]; this.opts.value.current = next; // Sync inputValue in SINGLE mode only — show the selected label so the @@ -236,8 +227,8 @@ export class ComboboxProvider { } const eventTarget = target ?? this.resolveItemEl(value); - if (eventName && eventTarget) { - void this.runtime.trigger(eventName, { fallbackTarget: eventTarget }); + if (result.event && eventTarget) { + void this.runtime.trigger(result.event, { fallbackTarget: eventTarget }); } } diff --git a/src/uix/soma/components/select/select-provider.svelte.ts b/src/uix/soma/components/select/select-provider.svelte.ts index c64778a1d..d995bff57 100644 --- a/src/uix/soma/components/select/select-provider.svelte.ts +++ b/src/uix/soma/components/select/select-provider.svelte.ts @@ -24,6 +24,10 @@ import { Typeahead } from '../../typeahead'; import { Presence } from '../../layers/presence.svelte'; import { Dismissal, type DismissalBehavior } from '../../layers/dismissal.svelte'; import { ScrollLock } from '../../layers/scroll-lock.svelte'; +import { + computeListSelection, + resolveListItemEl +} from '../../layers/list-selection'; import { FloatingProvider, FloatingContent, @@ -154,41 +158,27 @@ export class SelectProvider { } private resolveItemEl(value: string): HTMLElement | null { - const root = this.contentRef.current; - if (!root) return null; - const escaped = typeof CSS !== 'undefined' && CSS.escape ? CSS.escape(value) : value; - return root.querySelector(`[${attrs.item}][data-value="${escaped}"]`); + return resolveListItemEl(this.contentRef.current, attrs.item, value); } selectItem(value: string, target?: HTMLElement) { if (this.opts.disabled.current) return; - const current = this.opts.value.current; - const isSelected = current.includes(value); - let eventName: 'commit-select' | 'commit-unselect' | undefined; - - let next: string[]; - if (this.opts.type.current === 'single') { - if (isSelected && this.opts.allowDeselect.current) { - next = []; - eventName = 'commit-unselect'; - } else if (isSelected) { - this.handleClose(); - return; - } else { - next = [value]; - eventName = 'commit-select'; - } - this.handleClose(); - } else { - next = isSelected ? current.filter((v) => v !== value) : [...current, value]; - eventName = isSelected ? 'commit-unselect' : 'commit-select'; - } + const result = computeListSelection({ + current: this.opts.value.current, + value, + type: this.opts.type.current, + allowDeselect: this.opts.allowDeselect.current + }); + + if (result.shouldClose) this.handleClose(); + + if (result.skipUpdate) return; - this.opts.value.current = next; + this.opts.value.current = [...result.next]; const eventTarget = target ?? this.resolveItemEl(value); - if (eventName && eventTarget) { - void this.runtime.trigger(eventName, { fallbackTarget: eventTarget }); + if (result.event && eventTarget) { + void this.runtime.trigger(result.event, { fallbackTarget: eventTarget }); } } diff --git a/src/uix/soma/layers/list-selection.ts b/src/uix/soma/layers/list-selection.ts new file mode 100644 index 000000000..ff324a9b9 --- /dev/null +++ b/src/uix/soma/layers/list-selection.ts @@ -0,0 +1,147 @@ +/** + * List-selection state machine — shared logic between `` and + * ``. + * - `single`: at most one value selected; commits close the popover. + * - `multiple`: many values selected; commits keep the popover open + * so the user can pick more. + */ +export type ListSelectionType = 'single' | 'multiple'; + +/** + * Canonical sema event names emitted when a list-selection item + * transitions. Aligned with the `commit-select` / `commit-unselect` + * vocabulary declared by both Combobox and Select morfos. + */ +export type ListSelectionEvent = 'commit-select' | 'commit-unselect'; + +export interface ListSelectionInput { + /** The currently-selected values (immutable read). */ + current: readonly string[]; + /** The value being picked. */ + value: string; + /** Single vs multi selection mode. */ + type: ListSelectionType; + /** Whether selecting an already-selected value clears it. Single only. */ + allowDeselect: boolean; +} + +export interface ListSelectionResult { + /** + * The next selection array. When `skipUpdate` is `true` this is + * just `current` echoed back — callers may still want it to keep + * a single assignment path. + */ + next: readonly string[]; + /** + * Which sema event the transition emits, or `undefined` when the + * action is a no-op (e.g. single + re-select with no deselect). + */ + event: ListSelectionEvent | undefined; + /** + * Should the host close the popover after applying the change? + * - single mode: always `true` (commit closes; no-op also + * closes per existing UX — see Combobox/Select tests) + * - multi mode: always `false` + */ + shouldClose: boolean; + /** + * Should the caller SKIP writing `opts.value.current = next`? + * True only for the "single + isSelected + !allowDeselect" + * no-op branch — there's nothing to write, but the host still + * closes the popover. + */ + skipUpdate: boolean; +} + +/** + * Pure state machine. Compute the next selection given the input. + * + * The single-mode "no-op when re-selecting and !allowDeselect" branch + * still asks the host to close (the popover is meant to dismiss after + * any click on the current item), so callers should respect both + * `shouldClose` and `skipUpdate` independently. + */ +export function computeListSelection(input: ListSelectionInput): ListSelectionResult { + const isSelected = input.current.includes(input.value); + + if (input.type === 'single') { + if (isSelected && input.allowDeselect) { + return { + next: [], + event: 'commit-unselect', + shouldClose: true, + skipUpdate: false + }; + } + if (isSelected) { + // Re-select with deselect disabled — keep current array, + // no event, but still close. The original providers had + // `return` mid-method here; we surface it as `skipUpdate`. + return { + next: input.current, + event: undefined, + shouldClose: true, + skipUpdate: true + }; + } + return { + next: [input.value], + event: 'commit-select', + shouldClose: true, + skipUpdate: false + }; + } + + // Multi mode: toggle the value, never close. + return { + next: isSelected + ? input.current.filter((v) => v !== input.value) + : [...input.current, input.value], + event: isSelected ? 'commit-unselect' : 'commit-select', + shouldClose: false, + skipUpdate: false + }; +} + +/** + * Resolve a list-selection item element from its `data-value` inside a + * given container. Used by both Combobox.Content and Select.Content as + * the fallback target when `selectItem(value)` is called without an + * explicit element (e.g. from a keyboard event whose `currentTarget` + * isn't the item itself). + * + * The `itemAttr` is the bare attribute name (e.g. + * `'data-combobox-item'`) — caller passes it in because the attrs map + * is per-component. + */ +export function resolveListItemEl( + root: HTMLElement | null, + itemAttr: string, + value: string +): HTMLElement | null { + if (!root) return null; + const escaped = typeof CSS !== 'undefined' && CSS.escape ? CSS.escape(value) : value; + return root.querySelector(`[${itemAttr}][data-value="${escaped}"]`); +}