diff --git a/.github/workflows/design-patterns-check.yaml b/.github/workflows/design-patterns-check.yaml new file mode 100644 index 0000000000..cfee10c594 --- /dev/null +++ b/.github/workflows/design-patterns-check.yaml @@ -0,0 +1,85 @@ +name: Picasso Design Patterns Check + +on: + pull_request: + types: + - opened + - synchronize + - reopened + - ready_for_review + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: write + issues: write + id-token: write + +jobs: + design-patterns: + name: Picasso design patterns + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@ff7abcd0c3c05ccf6adc123a8cd1fd4fb30fb493 + with: + fetch-depth: 0 + + - name: Run Claude design-patterns review + uses: anthropics/claude-code-action@v1 + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + claude_args: '--allowed-tools "Bash(git diff:*),Bash(git fetch:*),Read,Write"' + prompt: | + You are reviewing this pull request against the Picasso component + library design-pattern rules. Follow these steps: + + 1. Read `./PICASSO_COMPONENT_DESIGN_PATTERNS.md`. It contains: + - Rules under `## Patterns` (apply to all components). + - Rules under `## Form components` (apply only to + form/field-style components: inputs, selects, checkboxes, + radios, date pickers, file uploaders, etc.). Label these + rules F1, F2, F3 in the table. + 2. Run `git fetch --no-tags --depth=1 origin ${{ github.base_ref }}` + and then `git diff origin/${{ github.base_ref }}...HEAD -- + 'packages/**/*.tsx' 'packages/**/*.ts' + ':(exclude)**/*.test.*' ':(exclude)**/*.spec.*' + ':(exclude)**/*.stories.*'` to obtain the PR diff. Evaluate + ONLY the added/modified component code in that diff. + 3. Skip rules that are not relevant to the diff. Skip the F-rules + for non-form components. + 4. Write your review to `design-patterns-report.md` as a single + Markdown table — and ONLY that table. No preamble, no notes, + no summary, no surrounding prose. + + Table format: + + | Rule | Status | + | --- | --- | + | | :white_check_mark: OR :x: | + + - Use the rule's id followed by an em-dash and short label + (e.g., `1 — Optimize defaults for the common case`). + - One row per evaluated rule. Use `:white_check_mark:` for pass, + `:x: ` for fail. + - If every evaluated rule passes, every row is + `:white_check_mark:`. + + 5. After writing the file, also output the exact same table as + your final message so it appears in the PR comment. + + - name: Fail if any rule is unsatisfied + run: | + if [ ! -f design-patterns-report.md ]; then + echo "No report produced — failing." + exit 1 + fi + if grep -q ':x:' design-patterns-report.md; then + echo "One or more design pattern rules failed. See the table in the PR comment or the step above." + exit 1 + fi + echo "All design pattern rules satisfied." diff --git a/PICASSO_COMPONENT_DESIGN_PATTERNS.md b/PICASSO_COMPONENT_DESIGN_PATTERNS.md new file mode 100644 index 0000000000..52ef22ef19 --- /dev/null +++ b/PICASSO_COMPONENT_DESIGN_PATTERNS.md @@ -0,0 +1,90 @@ +# Picasso Component Design Patterns + +This document lists the design patterns that every Picasso component in this repository must follow. It is intended to be the source of truth for a CI workflow (GitHub Actions) that validates each component added or modified in a pull request. + +## Patterns + +1. **Optimize defaults for the common case.** Design the props API so the most frequently used look of a component requires zero or near-zero props. Pick sensible defaults for `variant`, `size`, `color`, etc., so consumers only pass props when deviating from the common case. +2. **Reuse prop names across components.** The same concept must use the same prop name everywhere. If `collapsed` denotes a collapsed state in one component, every other component expressing the same concept must also use `collapsed` — not `isCollapsed`, `folded`, or `minimized`. Reuse before inventing. +3. **Keep prop names short and simple.** Prop names should be as short as possible while staying readable. Prefer `size` over `sizeText`, `color` over `colorValue`, `label` over `labelString`. Drop redundant suffixes that restate the type or context. +4. **Mirror native HTML prop names.** When a prop corresponds to a native HTML attribute, use the native name verbatim (e.g., `name`, `value`, `type`, `disabled`, `placeholder`, `checked`, `readOnly`, `autoFocus`, `href`, `target`, `rel`, `alt`, `src`). Do not rename, prefix, or alias native attributes (avoid `inputName`, `fieldValue`, `isDisabled`, `linkHref`). For event handlers, keep the native name (`onChange`, `onBlur`, `onFocus`, `onClick`, …), but the callback signature may diverge from the native one when a simpler shape is more ergonomic — e.g., `onChange?: (value: string) => void` is preferred over passing the raw event when only the value is needed. +5. **Style overrides only via `className` or `style`.** Consumers may customize a component's appearance exclusively through the `className` and `style` props. Do not expose other styling hooks such as `classes`, `styles`, `sx`, `css`, theme overrides, slot-level class props, or styled-component wrappers as part of the public API. +6. **Prefer `children` over content props.** For simple components, pass content through `children` rather than a dedicated prop. Use ``, not ` + + + ``` +16. **Use `testIds` for multi-part test selectors.** When a component has multiple independently testable parts and the root `data-testid` is not enough, expose a single optional `testIds` prop — an object whose keys map to each addressable part. Each key is itself optional, and the component should fall back to sensible defaults or skip the attribute when unset. Do not add per-part `data-testid` props at the top level. + + ```ts + // Shape — keys depend on the component's parts: + testIds?: { + [partName: string]: string | undefined + } + ``` + +## Form components + +Rules in this section apply only to form components (inputs, selects, checkboxes, radios, date pickers, file uploaders, and similar field-style components). + +1. **Extend `FieldProps` (or a descendant).** Every form component's props interface must extend `FieldProps` — the project's extended version of `final-form`'s field props — or a type that itself extends `FieldProps` (e.g., `InputProps`, `SelectProps`). This guarantees a consistent contract for `value`, `onChange`, validation state, error messaging, and form integration across every field-style component. +2. **Honor the standard form-field props.** Form components must accept and respect the full set of standard field props provided by `final-form` / `FieldProps` — including `name`, `value`, `defaultValue`, `required`, `disabled`, `onChange`, `onBlur`, `onFocus`, and any others surfaced by `FieldProps`. Do not selectively omit or rename them. +3. **Render through `PicassoField` (or a descendant).** Internally, every form component must use `PicassoField` — or a wrapper that itself builds on `PicassoField` (e.g., `OutlinedInput`, `InputBase`-style descendants) — to render its field chrome. This centralizes label, hint, error, required-marker, and layout behavior so all fields look and behave consistently. Do not reimplement field chrome ad hoc.