-
Notifications
You must be signed in to change notification settings - Fork 15
refactor(core): split widget prop types by family #277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| import type { FocusConfig } from "../../focus/styles.js"; | ||
| import type { WidgetSize, WidgetTone, WidgetVariant } from "../../ui/designTokens.js"; | ||
| import type { TextStyle } from "../style.js"; | ||
| import type { VNode } from "../types.js"; | ||
|
|
||
| /* ========== Form Widgets (GitHub issue #119) ========== */ | ||
|
|
||
| /** Option for select and radio group widgets. */ | ||
| export type SelectOption = Readonly<{ | ||
| /** Option value used in form state. */ | ||
| value: string; | ||
| /** Display label for the option. */ | ||
| label: string; | ||
| /** Whether this option is disabled. */ | ||
| disabled?: boolean; | ||
| }>; | ||
|
|
||
| /** Props for field wrapper widget. Wraps an input with label, error, and hint. */ | ||
| export type FieldProps = Readonly<{ | ||
| key?: string; | ||
| /** Field label displayed above the input. */ | ||
| label: string; | ||
| /** Error message to display below the input. */ | ||
| error?: string; | ||
| /** Whether the field is required (shows asterisk). */ | ||
| required?: boolean; | ||
| /** Help text displayed below the input. */ | ||
| hint?: string; | ||
| /** The wrapped input widget. */ | ||
| children: VNode; | ||
| }>; | ||
|
|
||
| /** Props for select dropdown widget. */ | ||
| export type SelectProps = Readonly<{ | ||
| id: string; | ||
| key?: string; | ||
| /** Opt out of Tab focus order while keeping id-based routing available. */ | ||
| focusable?: boolean; | ||
| /** Optional semantic label used for accessibility/debug announcements. */ | ||
| accessibleLabel?: string; | ||
| /** Currently selected value. */ | ||
| value: string; | ||
| /** Available options. */ | ||
| options: readonly SelectOption[]; | ||
| /** Callback when selection changes. */ | ||
| onChange?: (value: string) => void; | ||
| /** Whether the select is disabled. */ | ||
| disabled?: boolean; | ||
| /** Placeholder text when no value is selected. */ | ||
| placeholder?: string; | ||
| /** Whether to show the select in an error state. */ | ||
| error?: boolean; | ||
| /** Optional focus appearance configuration. */ | ||
| focusConfig?: FocusConfig; | ||
| /** Design system: visual variant (reserved for future select recipes). */ | ||
| dsVariant?: WidgetVariant; | ||
| /** Design system: tone (reserved for future select recipes). */ | ||
| dsTone?: WidgetTone; | ||
| /** Design system: size preset. */ | ||
| dsSize?: WidgetSize; | ||
| }>; | ||
|
|
||
| /** Props for slider widget. */ | ||
| export type SliderProps = Readonly<{ | ||
| id: string; | ||
| key?: string; | ||
| /** Opt out of Tab focus order while keeping id-based routing available. */ | ||
| focusable?: boolean; | ||
| /** Optional semantic label used for accessibility/debug announcements. */ | ||
| accessibleLabel?: string; | ||
| /** Current slider value. */ | ||
| value: number; | ||
| /** Minimum value (default: 0). */ | ||
| min?: number; | ||
| /** Maximum value (default: 100). */ | ||
| max?: number; | ||
| /** Step increment for keyboard changes (default: 1). */ | ||
| step?: number; | ||
| /** Optional fixed track width in cells (default: fills available width). */ | ||
| width?: number; | ||
| /** Optional label shown before the track. */ | ||
| label?: string; | ||
| /** Show numeric value text (default: true). */ | ||
| showValue?: boolean; | ||
| /** Callback when value changes. */ | ||
| onChange?: (value: number) => void; | ||
| /** Whether the slider is disabled. */ | ||
| disabled?: boolean; | ||
| /** Whether the slider is read-only (focusable but non-editable). */ | ||
| readOnly?: boolean; | ||
| /** Optional style applied to label/value text. */ | ||
| style?: TextStyle; | ||
| /** Optional focus appearance configuration. */ | ||
| focusConfig?: FocusConfig; | ||
| }>; | ||
|
|
||
| /** Props for checkbox widget. */ | ||
| export type CheckboxProps = Readonly<{ | ||
| id: string; | ||
| key?: string; | ||
| /** Opt out of Tab focus order while keeping id-based routing available. */ | ||
| focusable?: boolean; | ||
| /** Optional semantic label used for accessibility/debug announcements. */ | ||
| accessibleLabel?: string; | ||
| /** Whether the checkbox is checked. */ | ||
| checked: boolean; | ||
| /** Label displayed next to the checkbox. */ | ||
| label?: string; | ||
| /** Callback when checked state changes. */ | ||
| onChange?: (checked: boolean) => void; | ||
| /** Whether the checkbox is disabled. */ | ||
| disabled?: boolean; | ||
| /** Optional focus appearance configuration. */ | ||
| focusConfig?: FocusConfig; | ||
| /** Design system: tone for checked/focus rendering. */ | ||
| dsTone?: WidgetTone; | ||
| /** Design system: size preset. */ | ||
| dsSize?: WidgetSize; | ||
| }>; | ||
|
|
||
| /** Props for radio group widget. */ | ||
| export type RadioGroupProps = Readonly<{ | ||
| id: string; | ||
| key?: string; | ||
| /** Opt out of Tab focus order while keeping id-based routing available. */ | ||
| focusable?: boolean; | ||
| /** Optional semantic label used for accessibility/debug announcements. */ | ||
| accessibleLabel?: string; | ||
| /** Currently selected value. */ | ||
| value: string; | ||
| /** Available options. */ | ||
| options: readonly SelectOption[]; | ||
| /** Callback when selection changes. */ | ||
| onChange?: (value: string) => void; | ||
| /** Layout direction. */ | ||
| direction?: "horizontal" | "vertical"; | ||
| /** Whether the radio group is disabled. */ | ||
| disabled?: boolean; | ||
| /** Optional focus appearance configuration. */ | ||
| focusConfig?: FocusConfig; | ||
| /** Design system: tone for selected/focus rendering. */ | ||
| dsTone?: WidgetTone; | ||
| /** Design system: size preset. */ | ||
| dsSize?: WidgetSize; | ||
| }>; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,93 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { WidgetSize, WidgetTone, WidgetVariant } from "../../ui/designTokens.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { VNode } from "../types.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* ========== Navigation Widgets ========== */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Tabs visual style variant. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type TabsVariant = "line" | "enclosed" | "pills"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Tabs bar position relative to content. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type TabsPosition = "top" | "bottom"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Tab item descriptor. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type TabsItem = Readonly<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| content: VNode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Props for tabs widget. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type TabsProps = Readonly<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tabs: readonly TabsItem[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| activeTab: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange: (key: string) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| variant?: TabsVariant; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| position?: TabsPosition; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: visual variant. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsVariant?: WidgetVariant; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: color tone. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsTone?: WidgetTone; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: size preset. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsSize?: WidgetSize; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Accordion item descriptor. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type AccordionItem = Readonly<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| title: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| content: VNode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Props for accordion widget. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type AccordionProps = Readonly<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| items: readonly AccordionItem[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expanded: readonly string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange: (expanded: readonly string[]) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowMultiple?: boolean; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: visual variant. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsVariant?: WidgetVariant; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: color tone. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsTone?: WidgetTone; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: size preset. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsSize?: WidgetSize; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Breadcrumb item descriptor. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type BreadcrumbItem = Readonly<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onPress?: () => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Props for breadcrumb widget. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type BreadcrumbProps = Readonly<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| items: readonly BreadcrumbItem[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| separator?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: visual variant. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsVariant?: WidgetVariant; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: color tone. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsTone?: WidgetTone; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: size preset. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsSize?: WidgetSize; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+59
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actionable breadcrumbs need a stable widget id. Once any Suggested fix export type BreadcrumbProps = Readonly<{
- id?: string;
+ id: string;
key?: string;
items: readonly BreadcrumbItem[];
separator?: string;📝 Committable suggestion
Suggested change
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checked the existing implementation before changing anything. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm recording this pattern for future reviews. ✏️ Learnings added
🧠 Learnings used |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Props for pagination widget. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type PaginationProps = Readonly<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page: number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| totalPages: number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange: (page: number) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| showFirstLast?: boolean; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: visual variant. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsVariant?: WidgetVariant; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: color tone. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsTone?: WidgetTone; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Design system: size preset. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dsSize?: WidgetSize; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep a single source of truth for
fieldchildren.FieldPropsstoreschildren, but Line 306 ofpackages/core/src/widgets/types.tsalready modelskind: "field"with a separatechildrenarray. The split leaves two competing places for the wrapped control, which makes the canonical contract internally inconsistent. Based on learnings, "Canonical API: preserve prop/callback names and signatures; these are the source of truth for widget contracts."🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified against
origin/mainand the current widget implementation.FieldProps.childrenis part of the existing public contract, and the current factory / JSX wrapper /createFieldVNodeimplementation all use it to populate the widget-levelchildrenarray. Removing it here would be a real API change, not a split-only fix, so I’m preserving the existing contract in this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RtlZeroMemory, understood — thanks for verifying this againstorigin/main. TheFieldProps.children→createFieldVNode→ widget-levelchildrenarray pipeline makes the design clear:FieldProps.childrenis the public input surface, not a duplicate. Preserving it as-is in this split-only PR is the right call. I'll withdraw my concern.( ´・‿・`) ~♡
✏️ Learnings added
🧠 Learnings used