From 30ac658d712b92d108755ead890cd9ed5d0a2e5a Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 23 Mar 2026 09:42:29 +0000 Subject: [PATCH] Split selection card group owners --- .../subsystems/frontend-primitives.md | 8 + .../shared/SelectionCardGroup.test.tsx | 47 +++++ .../components/shared/SelectionCardGroup.tsx | 160 +++++++----------- .../SharedPrimitives.guardrails.test.ts | 19 ++- .../shared/selectionCardGroupModel.ts | 105 ++++++++++++ .../shared/useSelectionCardGroupState.ts | 33 ++++ .../frontendResourceTypeBoundaries.test.ts | 17 ++ 7 files changed, 285 insertions(+), 104 deletions(-) create mode 100644 frontend-modern/src/components/shared/selectionCardGroupModel.ts create mode 100644 frontend-modern/src/components/shared/useSelectionCardGroupState.ts diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index c1b3542c9..614e3c076 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -279,6 +279,14 @@ owns variant resolution plus disabled selection/change runtime, and variant class catalog, compact-label policy, and segmented button class selection. Future filter-button-group work should extend those owners instead of pushing label truncation or segmented variant policy back into the shell. +The shared selection-card primitive now follows that same owner split. +`frontend-modern/src/components/shared/SelectionCardGroup.tsx` stays the render +shell, `frontend-modern/src/components/shared/useSelectionCardGroupState.ts` +owns variant resolution plus disabled selection/change runtime, and +`frontend-modern/src/components/shared/selectionCardGroupModel.ts` owns the +tone fallback, group/button class catalog, and title/description presentation +policy. Future selection-card-group work should extend those owners instead of +pushing tone or active-card presentation logic back into the shell. The shared dialog now follows that same owner split. `frontend-modern/src/components/shared/Dialog.tsx` stays the render shell, `frontend-modern/src/components/shared/useDialogState.ts` owns focus trap, diff --git a/frontend-modern/src/components/shared/SelectionCardGroup.test.tsx b/frontend-modern/src/components/shared/SelectionCardGroup.test.tsx index 8a46fcc20..dfa15b4dd 100644 --- a/frontend-modern/src/components/shared/SelectionCardGroup.test.tsx +++ b/frontend-modern/src/components/shared/SelectionCardGroup.test.tsx @@ -1,12 +1,37 @@ import { cleanup, fireEvent, render, screen } from '@solidjs/testing-library'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { SelectionCardGroup } from './SelectionCardGroup'; +import selectionCardGroupSource from './SelectionCardGroup.tsx?raw'; +import selectionCardGroupModelSource from './selectionCardGroupModel.ts?raw'; +import selectionCardGroupStateSource from './useSelectionCardGroupState.ts?raw'; describe('SelectionCardGroup', () => { afterEach(() => { cleanup(); }); + it('keeps shell, runtime, and model owners split', () => { + expect(selectionCardGroupSource).toContain('useSelectionCardGroupState'); + expect(selectionCardGroupSource).toContain('getSelectionCardGroupClass'); + expect(selectionCardGroupSource).toContain('getSelectionCardButtonClass'); + expect(selectionCardGroupSource).toContain('getSelectionCardTitleClass'); + expect(selectionCardGroupSource).not.toContain('resolveSelectionCardTone'); + expect(selectionCardGroupSource).not.toContain('props.onChange(option.value)'); + expect(selectionCardGroupSource).not.toContain('groupClassByVariant'); + + expect(selectionCardGroupStateSource).toContain('export function useSelectionCardGroupState'); + expect(selectionCardGroupStateSource).toContain('createMemo'); + expect(selectionCardGroupStateSource).toContain('resolveSelectionCardTone'); + expect(selectionCardGroupStateSource).toContain('props.disabled || option.disabled'); + expect(selectionCardGroupStateSource).toContain('props.onChange(option.value)'); + + expect(selectionCardGroupModelSource).toContain('resolveSelectionCardGroupVariant'); + expect(selectionCardGroupModelSource).toContain('resolveSelectionCardTone'); + expect(selectionCardGroupModelSource).toContain('getSelectionCardButtonClass'); + expect(selectionCardGroupModelSource).toContain('getSelectionCardTitleClass'); + expect(selectionCardGroupModelSource).toContain("compact: 'grid grid-cols-2 gap-2'"); + }); + it('routes compact card selection changes through the shared primitive', () => { const onChange = vi.fn(); @@ -32,6 +57,28 @@ describe('SelectionCardGroup', () => { expect(onChange).toHaveBeenCalledWith('openai'); }); + it('blocks disabled selection changes in the runtime owner', () => { + const onChange = vi.fn(); + + render(() => ( + + )); + + const rcButton = screen.getByRole('button', { name: /release candidate/i }); + expect(rcButton).toBeDisabled(); + + fireEvent.click(rcButton); + expect(onChange).not.toHaveBeenCalled(); + }); + it('supports detail cards with success tone styling', () => { render(() => ( { - value: T; - title: string; - description?: string; - icon?: (props: { active: boolean }) => JSX.Element; - tone?: SelectionCardTone; - disabled?: boolean; -} - -interface SelectionCardGroupProps { - options: SelectionCardOption[]; - value: T; - onChange: (value: T) => void; - class?: string; - variant?: SelectionCardGroupVariant; - disabled?: boolean; -} - -const groupClassByVariant: Record = { - compact: 'grid grid-cols-2 gap-2', - detail: 'grid grid-cols-1 gap-3', -}; - -function activeCardClass(tone: SelectionCardTone): string { - if (tone === 'success') { - return 'border-green-500 bg-green-50 dark:bg-green-900'; - } - return 'border-blue-500 bg-blue-50 dark:bg-blue-900'; -} - -function inactiveCardClass(variant: SelectionCardGroupVariant): string { - if (variant === 'compact') { - return 'border-border hover:border-blue-300'; - } - return 'border-border hover:border-border'; -} - -function buttonClass( - variant: SelectionCardGroupVariant, - tone: SelectionCardTone, - active: boolean, - disabled: boolean, -): string { - const base = - variant === 'detail' - ? 'p-4 rounded-md border-2 transition-all text-left' - : 'p-3 rounded-md border-2 transition-all text-center'; - - return [ - base, - active ? activeCardClass(tone) : inactiveCardClass(variant), - disabled ? 'disabled:opacity-50 disabled:cursor-not-allowed' : '', - ].join(' '); -} - -function iconContainerClass(tone: SelectionCardTone, active: boolean): string { - const activeClass = - tone === 'success' ? 'bg-green-100 dark:bg-green-800' : 'bg-blue-100 dark:bg-blue-800'; - return ['p-2 rounded-md', active ? activeClass : 'bg-surface-alt'].join(' '); -} - -function titleClass( - variant: SelectionCardGroupVariant, - tone: SelectionCardTone, - active: boolean, -): string { - if (variant === 'compact') { - return 'text-sm font-medium text-base-content'; - } - if (!active) { - return 'text-sm font-semibold text-base-content'; - } - return tone === 'success' - ? 'text-sm font-semibold text-green-900 dark:text-green-100' - : 'text-sm font-semibold text-blue-900 dark:text-blue-100'; -} - -function descriptionClass(variant: SelectionCardGroupVariant): string { - return variant === 'compact' ? 'text-xs text-slate-500 mt-0.5' : 'text-xs text-muted'; -} +export type { + SelectionCardGroupProps, + SelectionCardGroupVariant, + SelectionCardOption, + SelectionCardTone, +} from './selectionCardGroupModel'; export function SelectionCardGroup(props: SelectionCardGroupProps) { - const variant = () => props.variant ?? 'detail'; + const selectionCardGroup = useSelectionCardGroupState(props); return (
{(option) => { - const isActive = () => option.value === props.value; - const isDisabled = () => props.disabled || option.disabled || false; - const tone = () => option.tone ?? 'accent'; - return (