refactor(soma): extract ListSelectionHelper for combobox/select (audit Round 3 §3 #21)
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) <noreply@anthropic.com>
active-uix
parent
00cd3a2c61
commit
c1301fb550
@ -0,0 +1,147 @@
|
||||
/**
|
||||
* List-selection state machine — shared logic between `<Combobox>` and
|
||||
* `<Select>` providers. Both components implement an identical
|
||||
* single/multi selection flow with `allowDeselect` semantics; this
|
||||
* module extracts that flow into pure functions so the providers stay
|
||||
* thin and consistent.
|
||||
*
|
||||
* The helper does NOT touch state directly — it computes the *next*
|
||||
* state from the current one + the picked value + policy flags, and
|
||||
* returns a structured result the caller applies to its own
|
||||
* `opts.value.current`, runs its component-specific side effects on
|
||||
* (e.g. Combobox's `inputValue` sync), and dispatches the resolved
|
||||
* event against the resolved DOM element.
|
||||
*
|
||||
* Design rationale:
|
||||
* - Keep the logic pure so the same machine drives both providers.
|
||||
* - Surface `shouldClose` and `skipUpdate` explicitly so the caller
|
||||
* decides what to do; avoid embedding `this.handleClose()` calls
|
||||
* in the helper.
|
||||
* - Use plain string identifiers, not coupled to any `OnChangeFn`
|
||||
* or `ActiveProps` shape; the providers feed in the raw values.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Selection mode shared by `<Combobox>` and `<Select>`.
|
||||
* - `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<HTMLElement>(`[${itemAttr}][data-value="${escaped}"]`);
|
||||
}
|
||||
Loading…
Reference in new issue