diff --git a/.cursor/rules/relay-connection-item-components.mdc b/.cursor/rules/relay-connection-item-components.mdc index 2058c99d7..c4ab2d0c7 100644 --- a/.cursor/rules/relay-connection-item-components.mdc +++ b/.cursor/rules/relay-connection-item-components.mdc @@ -12,27 +12,27 @@ inline the rendering of node fields directly in the parent's `.map()` body. This ensures: - Data requirements are colocated with the rendering component -- Adding/removing fields in a row doesn't bloat the parent's fragment -- The row component is independently testable and reusable +- Adding/removing fields in an item doesn't bloat the parent's fragment +- The item component is independently testable and reusable ## Pattern ```tsx -// _components/ThingRow.tsx — owns its fragment -const thingRowFragment = graphql` - fragment ThingRow_thing on Thing { +// _components/ThingListItem.tsx — owns its fragment +const thingListItemFragment = graphql` + fragment ThingListItem_thing on Thing { id name status } `; -interface ThingRowProps { - thingKey: ThingRow_thing$key; +interface ThingListItemProps { + thingKey: ThingListItem_thing$key; } -export function ThingRow({ thingKey }: ThingRowProps) { - const thing = useFragment(thingRowFragment, thingKey); +export function ThingListItem({ thingKey }: ThingListItemProps) { + const thing = useFragment(thingListItemFragment, thingKey); return ( {thing.name} @@ -43,7 +43,7 @@ export function ThingRow({ thingKey }: ThingRowProps) { ``` ```tsx -// Parent — spreads the row fragment in its connection and renders the component +// Parent — spreads the item fragment in its connection and renders the component const parentFragment = graphql` fragment ParentPage_things on Query @refetchable(queryName: "ParentPageRefetchQuery") { @@ -51,7 +51,7 @@ const parentFragment = graphql` edges { node { id - ...ThingRow_thing + ...ThingListItem_thing } } } @@ -60,13 +60,17 @@ const parentFragment = graphql` // In JSX: {things.map(thing => ( - + ))} ``` ## Naming -- File: `_components/Row.tsx` (for table rows) or - `_components/Card.tsx` (for card lists) -- Fragment: `_` (e.g. `ThingRow_thing`) +- File: `_components/ListItem.tsx` — the canonical connection-item + suffix, regardless of whether the item renders as a table row, a card, or a + plain list entry (the layout is internal to the component, not its identity). + Do **not** use `*Row` or `*Card` suffixes. +- Fragment: `_` (e.g. `ThingListItem_thing`) - Prop: `Key` (e.g. `thingKey`) + +See `contrib/claude/react-components.md` for the full suffix taxonomy. diff --git a/AGENTS.md b/AGENTS.md index d56223331..44782df94 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,11 +20,13 @@ Detailed guides for specific subsystems live in `contrib/claude/`: - [`contrib/claude/validation.md`](contrib/claude/validation.md) — Validation framework (fluent API, validators, error codes, propagation) - [`contrib/claude/e2e.md`](contrib/claude/e2e.md) — End-to-end testing (factory builders, RBAC tests, tenant isolation, assertions) - [`contrib/claude/agent.md`](contrib/claude/agent.md) — Agent orchestration framework (tools, handoffs, execution) -- [`contrib/claude/app-arborescence.md`](contrib/claude/app-arborescence.md) — Frontend app folder layout (pages, routes, loaders, skeletons, _components) +- [`contrib/claude/app-arborescence.md`](contrib/claude/app-arborescence.md) — Frontend app folder layout (pages, routes at resource folders, loaders, skeletons, _components, _lib, _locales) - [`contrib/claude/relay.md`](contrib/claude/relay.md) — Frontend Relay client (queries, fragments, mutations, pagination) -- [`contrib/claude/react-components.md`](contrib/claude/react-components.md) — React component shape (file/export, props, configure vs data via hooks) -- [`contrib/claude/ui.md`](contrib/claude/ui.md) — @probo/ui, Tailwind, tailwind-variants, folders, skeletons, compound components +- [`contrib/claude/react-components.md`](contrib/claude/react-components.md) — React component shape (file/export, props, configure vs data via hooks, naming/suffix taxonomy, error props) +- [`contrib/claude/ui.md`](contrib/claude/ui.md) — @probo/ui v2 kit (Base UI headless, Tailwind, tailwind-variants, flat folders, bundle-safe skeletons) - [`contrib/claude/v2-colors.md`](contrib/claude/v2-colors.md) — v2 color system (Radix scale, step usage, do/don't) +- [`contrib/claude/error-handling.md`](contrib/claude/error-handling.md) — Frontend error handling (ErrorBoundary at any level, error/fallback props, async try/catch) +- [`contrib/claude/i18n.md`](contrib/claude/i18n.md) — Frontend i18next (key-based translations, _locales/.json per routes.ts) - [`contrib/claude/config.md`](contrib/claude/config.md) — Configuration propagation (all files to update when config changes) - [`contrib/claude/file-naming.md`](contrib/claude/file-naming.md) — File naming conventions (template files, extensions) - [`contrib/claude/prompt-style.md`](contrib/claude/prompt-style.md) — Agent prompt template structure (role/task/instructions XML style) diff --git a/contrib/claude/app-arborescence.md b/contrib/claude/app-arborescence.md index de0621957..68d632b02 100644 --- a/contrib/claude/app-arborescence.md +++ b/contrib/claude/app-arborescence.md @@ -1,16 +1,18 @@ # App arborescence (folder and file layout) -Conventions for organising pages, routes, and supporting files in Probo frontend apps (`apps/console`). The guiding principle is **one arborescence**: the route hierarchy is expressed once, through the `pages/` folder tree, and everything related to a route lives next to it. +Conventions for organising pages, routes, and supporting files in Probo frontend apps (`apps/compliance-portal`, `apps/console`). The guiding principle is **one arborescence**: the route hierarchy is expressed once, through the `pages/` folder tree, and everything related to a route lives next to it. -**The codebase does not fully match these rules yet.** Some route definitions still live in a separate `src/routes/` folder. Treat this guide as the target for new work and refactors. +These rules are the **source of truth**. Where existing code disagrees (e.g. `apps/console` still keeps some route definitions in a separate `src/routes/` folder), the code is non-compliant and should be migrated — it is not precedent. ## Related guides | Topic | Guide | | -------------------------------------------------------------------------------- | ----------------------------------------------------------- | | `@probo/ui`, Tailwind, `tailwind-variants`, folders, skeletons, compound modules | [`contrib/claude/ui.md`](ui.md) | -| React component shape, props, file/export conventions | [`contrib/claude/react-components.md`](react-components.md) | +| React component shape, props, file/export conventions, naming/suffixes | [`contrib/claude/react-components.md`](react-components.md) | | Relay queries, fragments, loaders, `queryRef` | [`contrib/claude/relay.md`](relay.md) | +| Error boundaries at any level, error/fallback props | [`contrib/claude/error-handling.md`](error-handling.md) | +| i18next translations and `_locales` folders | [`contrib/claude/i18n.md`](i18n.md) | ## Single arborescence principle @@ -66,8 +68,10 @@ Each page folder may contain a subset of these files. Names use PascalCase match | `MyPageLoader.tsx` | Bundle entry point imported by `lazy()` in the route. **Default export.** loads data via Relay, renders a skeleton while loading, then mounts the page with `queryRef`. Only needed when the page reads data. | | `MyPage.tsx` | The actual page component. Receives `queryRef` from the loader (when data is loaded), or is the **default export** directly imported by `lazy()` when no data is needed. | | `MyPageSkeleton.tsx` | `Suspense` fallback rendered while the page is still receiving data. Also used as the route-level `Fallback`. Only needed when the page reads data. | -| `MyPageError.tsx` | Error boundary rendering component for this page's error state. | +| `MyPageError.tsx` | Error UI rendered by a boundary for this page (see [`error-handling.md`](error-handling.md)). An `*Error` file may exist at **any** level, not just the route root. | | `_components/` | Sub-components scoped to this page (see [below](#_components-folder)). | +| `_lib/` | Non-component helpers scoped to this subtree: hooks, utilities, constants, types (see [below](#_lib-folder)). | +| `_locales/` | i18next translation catalogs for this route segment, one file per locale (`en-US.json`). Colocated with `routes.ts` (see [below](#_locales-folder)). | ### Layout vs Page naming @@ -244,7 +248,7 @@ export function ThirdPartiesPageError() { ## File naming -Component files (`.tsx` that export a React component) use **PascalCase**: `ThirdPartiesPage.tsx`, `ThirdPartyContactRow.tsx`, `ThirdPartiesPageSkeleton.tsx`. +Component files (`.tsx` that export a React component) use **PascalCase**: `ThirdPartiesPage.tsx`, `ThirdPartyContactListItem.tsx`, `ThirdPartiesPageSkeleton.tsx`. All other helper files (utilities, hooks, constants, configuration) use **camelCase**: `routes.ts`, `useThirdPartyFilters.ts`, `formatCurrency.ts`, `constants.ts`. @@ -332,10 +336,53 @@ pages/organizations/_components/StatusBadge.tsx ```text // Bad — page-specific helper placed in a global folder -src/components/ThirdPartyContactRow.tsx # only used by ThirdPartyContactsTab +src/components/ThirdPartyContactListItem.tsx # only used by the third-parties feature // Good — scoped to the page that uses it -pages/organizations/third-parties/_components/ThirdPartyContactRow.tsx +pages/organizations/third-parties/_components/ThirdPartyContactListItem.tsx +``` + +## `_lib` folder + +Non-component code scoped to a subtree — hooks, utilities, constants, types — lives in a `_lib/` folder next to the pages that use it. The same hoisting rule as `_components/` applies: shared helpers move to the nearest common ancestor's `_lib/`; truly global helpers live in `src/lib/`. Files in `_lib/` use camelCase (`useThirdPartyFilters.ts`, `formatCurrency.ts`, `constants.ts`). + +```text +// Good — feature-scoped helpers under _lib +pages/organizations/third-parties/ + _lib/ + useThirdPartyFilters.ts + formatThirdPartyStatus.ts + _components/ + ThirdPartyListItem.tsx +``` + +## `_locales` folder + +Translations are i18next catalogs in a `_locales/` folder, **one file per locale**, named by locale tag: `en-US.json`, `fr-FR.json`. See [`contrib/claude/i18n.md`](i18n.md) for key conventions and setup. + +`_locales/` is colocated with a `routes.ts`. The rule of thumb: + +- A `_locales/` folder belongs at a folder that **names a resource and owns a `routes.ts`** (e.g. `organizations/routes.ts` + `organizations/_locales/`, `organizations/measures/routes.ts` + `organizations/measures/_locales/`). +- There must be **no more `_locales/` folders than there are `routes.ts` files.** If a folder has no `routes.ts`, it does not get its own `_locales/` — its strings live in the nearest ancestor that does. + +```text +// Good — _locales sits beside routes.ts at each resource boundary +pages/organizations/ + routes.ts + _locales/ + en-US.json + fr-FR.json + measures/ + routes.ts + _locales/ + en-US.json + fr-FR.json + MeasuresPage.tsx + +// Bad — a _locales folder with no sibling routes.ts (strings belong to the parent resource) +pages/organizations/measures/_components/ + _locales/ + en-US.json ``` ## Child-route folder naming @@ -368,16 +415,22 @@ Target layout for a `third-parties` feature under `pages/organizations/`: ```text pages/organizations/third-parties/ routes.ts # route definitions for third parties + _locales/ # i18next catalogs (one _locales per routes.ts) + en-US.json + fr-FR.json ThirdPartiesPageLoader.tsx # lazy entry — providers + Suspense + query loader ThirdPartiesPage.tsx # page component (usePreloadedQuery) ThirdPartiesPageSkeleton.tsx # loading fallback + ThirdPartiesPageError.tsx # error UI for this page's boundary ThirdPartyDetailLayoutLoader.tsx # lazy entry for detail layout ThirdPartyDetailLayout.tsx # layout — breadcrumbs, tabs, ThirdPartyDetailLayoutSkeleton.tsx # detail loading fallback NewThirdPartyPage.tsx # mutation-only page — default export, wraps itself in the Relay provider + _lib/ # hooks / utils / constants scoped to third party pages + useThirdPartyFilters.ts _components/ # sub-components used only by third party pages - ThirdPartyContactRow.tsx - ThirdPartyRiskSummary.tsx + ThirdPartyContactListItem.tsx + ThirdPartyRiskSummarySection.tsx overview/ # child route: /third-parties/:thirdPartyId/overview ThirdPartyOverviewPage.tsx compliance/ # child route: /third-parties/:thirdPartyId/compliance diff --git a/contrib/claude/error-handling.md b/contrib/claude/error-handling.md new file mode 100644 index 000000000..28f1ebd05 --- /dev/null +++ b/contrib/claude/error-handling.md @@ -0,0 +1,164 @@ +# Error handling (frontend) + +Errors must be **containable at any level** of the tree, not only at the route root. A failure in one section, list, or widget should be able to render a local fallback without taking down the rest of the page. This guide covers the reusable `ErrorBoundary`, the error/fallback props that let any subtree opt in, and how to handle the errors boundaries **cannot** catch (async work and event handlers). + +## Related guides + +| Topic | Guide | +|-------|--------| +| Error/fallback props as configuration | [`contrib/claude/react-components.md`](react-components.md#error-and-fallback-props) | +| Where `*Error` files live in the tree | [`contrib/claude/app-arborescence.md`](app-arborescence.md) | +| UI for error states (`ErrorLayout`, …) | [`contrib/claude/ui.md`](ui.md) | + +## Two kinds of errors + +React error boundaries only catch errors thrown **during rendering, in lifecycle methods, and in the constructors of the tree below them**. They do **not** catch: + +- errors in **event handlers** (`onClick`, `onSubmit`, …), +- errors in **async** code (`await`, `.then`, `setTimeout`), +- errors thrown in the boundary itself. + +So there are two complementary tools: + +1. **`ErrorBoundary`** — for render-time failures (including Relay/Suspense errors thrown while reading data). Place it at the level where you want the blast radius to stop. +2. **`try`/`catch`** — for event handlers and async work. Surface the result through a toast and/or by storing the error in state. + +## `ErrorBoundary` + +A single reusable class component (the sanctioned use of a class — see [`react-components.md`](react-components.md#component-shape)) is the only error boundary primitive. It is generic and works at route, section, or component level. + +```tsx +// packages/ui/src/v2/ErrorBoundary/ErrorBoundary.tsx +import { Component, type ErrorInfo, type ReactNode } from "react"; + +export interface ErrorBoundaryProps { + children: ReactNode; + // A node, or a render function that receives the caught error + a reset fn. + fallback?: ReactNode | ((error: Error, reset: () => void) => ReactNode); + onError?: (error: Error, info: ErrorInfo) => void; +} + +interface ErrorBoundaryState { + error: Error | null; +} + +export class ErrorBoundary extends Component { + state: ErrorBoundaryState = { error: null }; + + static getDerivedStateFromError(error: Error): ErrorBoundaryState { + return { error }; + } + + componentDidCatch(error: Error, info: ErrorInfo) { + this.props.onError?.(error, info); + } + + reset = () => this.setState({ error: null }); + + render() { + const { error } = this.state; + if (error) { + const { fallback } = this.props; + if (typeof fallback === "function") { + return fallback(error, this.reset); + } + return fallback ?? null; + } + return this.props.children; + } +} +``` + +### Trigger at any level + +The same boundary wraps a whole route or a single widget — only the placement and the `fallback` differ. + +```tsx +// Route level — a page's *Error file is the fallback +}> + + +``` + +```tsx +// Section level — one failing section, the rest of the page survives + ( + + )} + onError={reportError} +> + + +``` + +In a router context, route boundaries are wired through the router's `ErrorBoundary` slot (e.g. `RootErrorBoundary` reading `useRouteError()`); `ErrorBoundary` above is for **in-page** boundaries below the route level. + +### Components expose error/fallback props + +Any component that owns a fallible region should let the surrounding subtree decide the fallback, by accepting `fallback` / `onError` and wrapping its risky content itself. These are configuration/composition props (a slot + a callback) — they never carry fetched data. + +```tsx +interface RiskSummarySectionProps { + fallback?: ReactNode; + onError?: (error: Error) => void; +} + +export function RiskSummarySection({ fallback, onError }: RiskSummarySectionProps) { + return ( + + + + ); +} +``` + +## `try`/`catch` for events and async work + +Boundaries will not catch a rejected promise in a submit handler. Wrap the risky call in `try`/`catch`, report via toast, and keep the UI responsive. + +```tsx +// Good — async event handler guards itself; the boundary above can't help here +function PublishButton() { + const { __ } = useTranslate(); + const { toast } = useToast(); + const [isPublishing, setIsPublishing] = useState(false); + + async function onPublish() { + setIsPublishing(true); + try { + await publishReport(); + toast({ title: __("Published"), variant: "success" }); + } catch (error) { + toast({ + title: __("Publish failed"), + description: error instanceof Error ? error.message : __("Unknown error"), + variant: "error", + }); + } finally { + setIsPublishing(false); + } + } + + return ; +} +``` + +```tsx +// Bad — relying on an ErrorBoundary to catch an async rejection (it never will) +function PublishButton() { + async function onPublish() { + await publishReport(); // throws → unhandled rejection, boundary does not fire + } + return ; +} +``` + +For Relay mutations, prefer the built-in `onCompleted` / `onError` callbacks (see [`relay.md`](relay.md)) over a manual `try`/`catch`; use `try`/`catch` for non-Relay async work (fetch, parsing, third-party SDKs). + +## Placement guidance + +- **Route root** — one boundary so an unhandled failure shows a full-page error instead of a blank screen. +- **Section / list / widget** — add a boundary around any independently-loaded region (especially Relay `Suspense` subtrees) so one failure degrades gracefully. +- **Interaction surfaces** (dropdowns, dialogs that load data on open) — wrap the lazily-loaded content so opening a broken menu doesn't crash the page. +- **Event/async paths** — `try`/`catch` + toast, never a boundary. diff --git a/contrib/claude/i18n.md b/contrib/claude/i18n.md new file mode 100644 index 000000000..0428beeff --- /dev/null +++ b/contrib/claude/i18n.md @@ -0,0 +1,101 @@ +# Internationalization (i18next) + +The v2 apps (starting with `apps/compliance-portal`) localize with [i18next](https://www.i18next.com/). Translations are **key-based**: code references a stable key, and each locale supplies the human-readable string in a JSON catalog. Catalogs live in `_locales/` folders colocated with routes. + +> This differs from the legacy `@probo/i18n` translator used by `apps/console`, where the **English source string itself is the key** (`__("Save changes")`). New code uses i18next with explicit keys; do not copy the source-string-as-key pattern into v2 apps. + +## Related guides + +| Topic | Guide | +|-------|--------| +| Where `_locales/` folders live in the tree | [`contrib/claude/app-arborescence.md`](app-arborescence.md#_locales-folder) | +| Routes and resource folders | [`contrib/claude/app-arborescence.md`](app-arborescence.md) | + +## Catalog files + +- One JSON file **per locale**, named by its BCP 47 locale tag: `en-US.json`, `fr-FR.json`. +- The **filename is the locale** — there is no locale field inside the file; the file is the namespace's catalog for that locale. +- Keys are stable, descriptive identifiers (not the English text). Nest by feature/component to avoid collisions. + +```jsonc +// pages/organizations/measures/_locales/en-US.json +{ + "measures": { + "title": "Measures", + "empty": "No measures yet", + "actions": { + "create": "New measure" + }, + "count_one": "{{count}} measure", + "count_other": "{{count}} measures" + } +} +``` + +```jsonc +// pages/organizations/measures/_locales/fr-FR.json +{ + "measures": { + "title": "Mesures", + "empty": "Aucune mesure pour le moment", + "actions": { + "create": "Nouvelle mesure" + }, + "count_one": "{{count}} mesure", + "count_other": "{{count}} mesures" + } +} +``` + +## Where catalogs live + +`_locales/` is colocated with a `routes.ts`, at the folder that **names a resource**. The constraint is exact: + +- **No more `_locales/` folders than `routes.ts` files.** A folder without a `routes.ts` does not get its own `_locales/`; its strings belong to the nearest ancestor resource that has one. +- Resource boundaries own their translations: `organizations/routes.ts` → `organizations/_locales/`; `organizations/measures/routes.ts` → `organizations/measures/_locales/`. + +See [app-arborescence.md](app-arborescence.md#_locales-folder) for the folder-tree examples. + +## Using translations in components + +Read translations with the i18next hook and a key. Keep keys close to where they are defined (the feature namespace), and pass interpolation values as the second argument. + +```tsx +import { useTranslation } from "react-i18next"; + +export function MeasuresPage() { + const { t } = useTranslation(); + return ( +
+

{t("measures.title")}

+

{t("measures.count", { count })}

+
+ ); +} +``` + +### Do / don't: keys, not source strings + +```tsx +// Bad — English source string as the key (legacy @probo/i18n pattern) +__("No measures yet"); + +// Good — stable key resolved from the locale catalog +t("measures.empty"); +``` + +### Do / don't: no string building + +Never assemble translated sentences by concatenation — it breaks word order in other languages. Use interpolation and pluralization keys instead. + +```tsx +// Bad — concatenation +`${count} ${t("measures.unit")}`; + +// Good — pluralized key with interpolation +t("measures.count", { count }); +``` + +## Loading catalogs + +Catalogs are loaded into i18next per locale at app startup (or lazily per route). Because each `_locales/` folder maps to a resource segment, catalogs can be code-split alongside the route bundle that needs them — keep a catalog scoped to the feature it serves rather than one global megafile. diff --git a/contrib/claude/react-components.md b/contrib/claude/react-components.md index fde2e2d67..b73cad8be 100644 --- a/contrib/claude/react-components.md +++ b/contrib/claude/react-components.md @@ -1,8 +1,8 @@ # React component conventions -This document describes **how to define and shape** React components in Probo frontends (`apps/console`, [`packages/ui`](../../packages/ui), and related apps). It complements styling and package layout in [`contrib/claude/ui.md`](ui.md) and data loading in [`contrib/claude/relay.md`](relay.md). +This document describes **how to define and shape** React components in Probo frontends (`apps/compliance-portal`, [`packages/ui`](../../packages/ui), and related apps). It complements styling and package layout in [`contrib/claude/ui.md`](ui.md) and data loading in [`contrib/claude/relay.md`](relay.md). -**The codebase does not fully match these rules yet.** Treat this guide as the target for new work and refactors. +These rules are the **source of truth**. Where existing code (e.g. `apps/console` or the legacy `@probo/ui` `Atoms/`/`Molecules/` tree) disagrees, the code is non-compliant and should be migrated — it is not precedent. ## Related guides @@ -10,6 +10,9 @@ This document describes **how to define and shape** React components in Probo fr |-------|--------| | `@probo/ui`, Tailwind, `tailwind-variants`, folders, skeletons, compound modules | [`contrib/claude/ui.md`](ui.md) | | Relay queries, fragments, loaders, `queryRef` | [`contrib/claude/relay.md`](relay.md) | +| App folder layout, route segments, special folders | [`contrib/claude/app-arborescence.md`](app-arborescence.md) | +| Error boundaries, error/fallback props, async `try`/`catch` | [`contrib/claude/error-handling.md`](error-handling.md) | +| i18next, `_locales`, translation keys | [`contrib/claude/i18n.md`](i18n.md) | ## Destructuring @@ -148,6 +151,53 @@ export default function ThirdPartiesPage({ queryRef }: ThirdPartiesPageProps) { } ``` +## Naming and suffixes + +Component names are built from a **base** (the resource or concept) plus a **role suffix**. The suffix tells you what kind of component it is at a glance, and mirrors the file's role in the route tree. UI-kit primitives are the exception — they use bare names (see [`contrib/claude/ui.md`](ui.md)). + +### Route / tree-level suffixes + +| Suffix | Role | +|--------|------| +| `*Page` | A leaf page rendering final content (no ``). Default export when it is the `lazy()` entry. | +| `*Loader` | The lazy bundle entry: sets up providers, triggers the Relay query, renders a skeleton, then mounts the page. | +| `*Layout` | A layout route that renders shared chrome and an ``. | +| `*Skeleton` | The loading placeholder for a page, section, or component. | +| `*Error` | The error UI rendered by a boundary at this level (see [`contrib/claude/error-handling.md`](error-handling.md)). | +| `*Provider` | A context / Relay provider wrapper. | + +### Content / section-level suffixes + +| Suffix | Role | +|--------|------| +| `*Section` | A logical section of a page (owns its own fragment). | +| `*List` | The component that renders a **collection** (the index/list region, including its empty + loading states). | +| `*ListItem` | A **single item** within a `*List`. This is the canonical connection-item suffix. | +| `*Form` | A form (owns its fields + submit wiring). | +| `*Field` | A single form field. | +| `*Empty` | An empty-state placeholder. | + +### Overlay suffixes + +`*Dialog`, `*Drawer`, `*Menu` — overlay surfaces, styled from the headless primitive (see [`contrib/claude/ui.md`](ui.md)). + +### Do / don't: `List` / `ListItem` over `Table` / `Row` + +A collection's identity is "a list of things"; whether it is laid out as a table, cards, or rows is a **presentation detail** that can change. Name after the data, not the current layout. + +```text +// Bad — named after the current visual treatment +ThirdPartiesTable.tsx +ThirdPartyRow.tsx +ThirdPartyCard.tsx + +// Good — named after the collection and its item +ThirdPartyList.tsx +ThirdPartyListItem.tsx +``` + +`*ListItem` replaces both the old `*Row` (table) and `*Card` (card list) connection-item suffixes. See [Connection items are components](#connection-items-are-components). + ## Props ordering Within `ComponentNameProps`, order members as follows: @@ -333,6 +383,31 @@ export function PageSection({ className, title, icon, children }: PageSectionPro } ``` +## Error and fallback props + +Error handling is not reserved for route boundaries. A component that performs work which can fail (a query, a parse, a risky render) should be **wrappable in a boundary at any level**, and components that own a fallible region may expose error-handling props so the surrounding subtree can render its own error UI instead of taking down the whole page. + +- `fallback` / `errorFallback` — a `ReactNode` (or render function receiving the error) shown when the wrapped content fails. +- `onError` — a callback invoked when the boundary catches, for logging / toasts. + +These are **configuration / composition** props (UI slots and callbacks), so they obey the same rules as the rest of this guide — they never carry fetched domain data. The full pattern (the reusable `ErrorBoundary`, where to place it, and the `async` event-handler `try`/`catch` that boundaries cannot catch) lives in [`contrib/claude/error-handling.md`](error-handling.md). + +```tsx +// Good — a section that can fail accepts a fallback slot and an onError callback +interface RiskSummarySectionProps { + fallback?: ReactNode; + onError?: (error: Error) => void; +} + +export function RiskSummarySection({ fallback, onError }: RiskSummarySectionProps) { + return ( + + + + ); +} +``` + ## Interaction-triggered data Data that is only needed after a user interaction (opening a dropdown, clicking a button, hovering) must **not** be fetched at page load. Instead, the parent component owns the query lifecycle with `useQueryLoader`, triggers `loadQuery` in the interaction event handler, and passes `queryRef` to a child component that reads data with `usePreloadedQuery`. @@ -450,18 +525,18 @@ For sections that own a paginated connection, use `usePaginationFragment` with a ## Connection items are components -When rendering items from a Relay connection (e.g. table rows via `edges.map(…)`), each item **must** be a dedicated component with its own colocated fragment — never inline the rendering of node fields directly in the parent's `.map()` body. +When rendering items from a Relay connection (e.g. `edges.map(…)`), each item **must** be a dedicated component with its own colocated fragment — never inline the rendering of node fields directly in the parent's `.map()` body. -Place the item component in `_components/` adjacent to the page. Name it after the GraphQL type it renders (e.g. `DetectedTrackerRow.tsx`, `ThirdPartyCard.tsx`). The component receives a single fragment key prop (e.g. `detectedTrackerKey: DetectedTrackerRow_detectedTracker$key`) and calls `useFragment` internally. +Place the item component in `_components/` adjacent to the page. Name it `ListItem` after the GraphQL type it renders (e.g. `DetectedTrackerListItem.tsx`, `ThirdPartyListItem.tsx`) — never `*Row` or `*Card` (see [Naming and suffixes](#naming-and-suffixes)). The component receives a single fragment key prop (e.g. `detectedTrackerKey: DetectedTrackerListItem_detectedTracker$key`) and calls `useFragment` internally. ```tsx -// Parent (page) — spreads the child fragment in the connection: -edges { node { id ...DetectedTrackerRow_detectedTracker } } +// Parent (the *List component) — spreads the item fragment in the connection: +edges { node { id ...DetectedTrackerListItem_detectedTracker } } // Parent JSX: {trackers.map(tracker => ( - + ))} ``` -This ensures field additions/removals in the row never modify the parent's fragment, and keeps the item independently testable. +This ensures field additions/removals in the item never modify the parent's fragment, and keeps the item independently testable. The layout the item renders (a table row, a card, a plain `
  • `) is internal to the component and does not affect its name. diff --git a/contrib/claude/relay.md b/contrib/claude/relay.md index fd0f9a6db..79a42ad07 100644 --- a/contrib/claude/relay.md +++ b/contrib/claude/relay.md @@ -191,7 +191,7 @@ Fragments colocate data requirements with the component that reads them: ```tsx const contactFragment = graphql` - fragment ContactRow_contactFragment on ThirdPartyContact { + fragment ContactListItem_contactFragment on ThirdPartyContact { id fullName email @@ -204,7 +204,7 @@ const contactFragment = graphql` } `; -function ContactRow(props: { contactKey: ContactRow_contactFragment$key }) { +function ContactListItem(props: { contactKey: ContactListItem_contactFragment$key }) { const contact = useFragment(contactFragment, props.contactKey); // ... } @@ -539,9 +539,10 @@ pages/organizations/third-parties/ _components/ CreateContactDialog.tsx # create mutation EditContactDialog.tsx # update mutation - tabs/ - ThirdPartyContactsTab.tsx # refetchable fragment + item fragment - ThirdPartyComplianceTab.tsx + ThirdPartyContactListItem.tsx # connection-item fragment + contacts/ + ThirdPartyContactsPage.tsx # refetchable fragment + list section + ThirdPartyContactsSection.tsx # section fragment ``` Component-specific operations (queries, fragments, mutations) are defined inline in the component file that uses them. Shared sub-components live in `_components/` next to the page (scoped to the nearest common ancestor). \ No newline at end of file diff --git a/contrib/claude/ui.md b/contrib/claude/ui.md index 303fb36eb..f879b37be 100644 --- a/contrib/claude/ui.md +++ b/contrib/claude/ui.md @@ -1,63 +1,121 @@ -# UI system (`@probo/ui`) +# UI system (`@probo/ui` v2 kit) -Shared React UI for Probo apps lives in the **`@probo/ui`** workspace package ([`packages/ui`](../../packages/ui)), as well as ad-hoc components created in apps (under `apps/*/src`). This document describes **target** conventions for building and styling those components, whether shared or app-local. +Shared React UI for Probo apps lives in the **`@probo/ui`** workspace package ([`packages/ui`](../../packages/ui)). The **v2 kit** ([`packages/ui/src/v2`](../../packages/ui/src/v2)) is the target system: a flat set of components styled on top of a headless primitive library, consuming the Radix-scale v2 theme. This document describes how to build and style those components. -**Today's codebase does not fully match these rules.** The tree still uses layouts like `Atoms/`, `Molecules/`, and `Layouts/`, and many files mix ad-hoc Tailwind on `className` with `tailwind-variants`. Treat this guide as the direction for new work and refactors, not as a description of the current tree. +These rules are the **source of truth**. The legacy tree (`Atoms/`, `Molecules/`, `Layouts/`, `clsx`-mixed `className`, imperative `DialogRef`) is non-compliant code to migrate, not precedent. -For data loading and GraphQL on the console, see [`contrib/claude/relay.md`](relay.md). +## Related guides + +| Topic | Guide | +|-------|--------| +| Component shape, props, naming/suffixes | [`contrib/claude/react-components.md`](react-components.md) | +| v2 color system (Radix scale) | [`contrib/claude/v2-colors.md`](v2-colors.md) | +| App folder layout and special folders | [`contrib/claude/app-arborescence.md`](app-arborescence.md) | +| Error boundaries and error/fallback props | [`contrib/claude/error-handling.md`](error-handling.md) | +| Relay data loading | [`contrib/claude/relay.md`](relay.md) | ## Package and tooling | Item | Convention | |------|------------| -| Package | **`@probo/ui`** — import shared components from this package in apps. | -| Styling | **Tailwind** (project uses Tailwind v4 in `packages/ui`). | -| Variants API | **`tailwind-variants`** — `import { tv } from "tailwind-variants"` to define component styles and slot class names. | +| Package | **`@probo/ui`** — v2 components under `src/v2`. Apps opt into v2 by importing the v2 theme (see [`v2-colors.md`](v2-colors.md)). | +| Styling | **Tailwind v4** with the Radix-scale tokens (`bg-sand-3`, `text-sand-12`, `rounded-3`, `text-4`, …). | +| Variants API | **`tailwind-variants`** only — `import { tv } from "tailwind-variants"`. | +| Class composition | **Do not use `clsx` or `tailwind-merge`.** All conditional styling goes through `tv` variants and slots. | +| Headless primitives | **Base UI** (`@base-ui-components/react`). We **style** these primitives; we do not re-implement their behavior. | -Preview components with Storybook from `packages/ui`: `npm run dev` (Storybook on port 6006 per `package.json`). +Preview components with Storybook from `packages/ui`: `npm run dev` (Storybook on port 6006). -## Props typing +> Base UI is migrating its package name from `@base-ui-components/react` to `@base-ui/react`. Import from whichever name the installed version publishes; examples below use `@base-ui-components/react`. -When a component renders a native HTML element as its top-level node (not a custom component), **merge the component's own props with that element's intrinsic props** via `ComponentProps`. Destructure custom props and spread the rest onto the element so callers can pass standard HTML attributes (`id`, `className`, `aria-*`, event handlers, etc.) without wrapper boilerplate. +## Headless primitives: style, don't re-implement -### Do / don't: props merging +Interactive components (dialogs, popovers, menus, selects, tabs, tooltips) are **Base UI primitives with our styling applied** — nothing more. The job of a v2 component is to bind `tv` classes to the primitive's parts. Do **not** add a custom behavior layer on top. + +Rules: + +- **Use the primitive's controlled API as-is.** A dialog is controlled with `open` / `onOpenChange` — the exact same API Base UI exposes. Opening *our* dialog is opening *the lib's* dialog. +- **No custom imperative ref API.** Never invent `useDialogRef()` / `ref.current.open()` / `ref.current.close()`. If imperative control is genuinely needed, use the primitive's own mechanism (e.g. Base UI's `Dialog.createHandle()` / `actionsRef`), never a hand-rolled `useRef` + `useEffect` shim. +- **No `cloneElement` / `Children.map` plumbing.** Compose with the primitive's parts and `asChild`-style props the library provides, not by cloning children to inject className/handlers. +- **No local mirror state.** Don't copy `open` into `useState` and sync it with `useEffect`; pass `open`/`onOpenChange` straight through, or let the primitive stay uncontrolled. + +### Do / don't: dialog wrapper ```tsx -// Good — own props merged with the native element's props, rest spread onto -type MyProps = ComponentProps<"span"> & { myPropName: string }; +// Bad — hand-rolled imperative ref, mirrored open state, cloneElement plumbing +export const useDialogRef = () => useRef(null); -export function MyComponent(props: MyProps) { - const { myPropName, ...spanProps } = props; - - return {myPropName}; +export function Dialog({ trigger, ref, children }: Props) { + const [open, setOpen] = useState(false); + useEffect(() => { + if (ref) ref.current = { open: () => setOpen(true), close: () => setOpen(false) }; + }); + // ... Children.map / cloneElement to inject classes ... + return {/* … */}; } ``` ```tsx -// Bad — only custom props accepted; callers cannot set id, className, aria-*, etc. -type MyProps = { myPropName: string }; +// Good — thin styling over Base UI; consumers use the lib's open/onOpenChange directly +import { Dialog as BaseDialog } from "@base-ui-components/react/dialog"; +import { tv } from "tailwind-variants"; -export function MyComponent(props: MyProps) { - return {props.myPropName}; +const dialog = tv({ + slots: { + backdrop: "fixed inset-0 bg-sand-12/40", + popup: "fixed left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2 rounded-4 bg-sand-2 p-6 shadow-4", + title: "text-4 font-medium text-sand-12", + }, +}); + +export type DialogProps = ComponentProps; + +export function Dialog(props: DialogProps) { + return ; } + +export function DialogPopup({ children, ...props }: ComponentProps) { + const { backdrop, popup } = dialog(); + return ( + + + + {children} + + + ); +} +``` + +```tsx +// Good — consumer controls it the same way they'd control the Base UI dialog +const [open, setOpen] = useState(false); + + + Open + + Delete third party + {/* … */} + + ``` ## `tailwind-variants` and `className` -In a **single component file**, do **not** mix arbitrary Tailwind utility strings on `className` with `tailwind-variants` for the same styling concerns. Put layout and look in **`tv` variants and `slots`** (and the APIs `tv` exposes for overrides). If consumers need extensibility, expose it through variant props or documented slot/class hooks—not by sprinkling raw utilities beside `tv()` output in the same file. +In a **single component file**, do **not** mix arbitrary Tailwind utility strings on `className` with `tailwind-variants` for the same styling concern. Layout and look live in **`tv` slots and variants** (and the override APIs `tv` exposes). Extensibility is exposed through variant props or documented slot/class hooks — never by sprinkling raw utilities (or `clsx`) beside `tv()` output. -For **compound / multi-slot** components, define `tv` in a **dedicated module** (see [Variants file](#variants-file)) so loading-only code paths can import styles without pulling the full interactive implementation. +For **compound / multi-slot** components, define `tv` in a **dedicated `variants.ts` module** (see [Variants file](#variants-file)) so loading-only code paths can import styles without pulling the full interactive implementation. ### Do / don't: `tv` vs raw `className` ```tsx -// Bad — same file mixes tv() output with ad-hoc Tailwind on className (clsx shown for the anti-pattern) +// Bad — same file mixes tv() output with ad-hoc Tailwind / clsx on className import { clsx } from "clsx"; import { tv } from "tailwind-variants"; const row = tv({ base: "flex items-center gap-2" }); -export function Row({ children }: { children: React.ReactNode }) { - return
    {children}
    ; +export function Row({ children }: { children: ReactNode }) { + return
    {children}
    ; } ``` @@ -66,45 +124,115 @@ export function Row({ children }: { children: React.ReactNode }) { import { tv } from "tailwind-variants"; const row = tv({ - base: "flex items-center gap-2 rounded-md border border-border-low", + base: "flex items-center gap-2 rounded-3 border border-sand-6", }); -export function Row({ children }: { children: React.ReactNode }) { +export function Row({ children }: { children: ReactNode }) { return
    {children}
    ; } ``` ```tsx -// Good — optional styling toggles use tv variants, not extra className strings in this file +// Good — optional styling toggles use tv variants, not extra className strings import { tv } from "tailwind-variants"; const row = tv({ base: "flex items-center gap-2", variants: { - bordered: { true: "rounded-md border border-border-low", false: "" }, + bordered: { true: "rounded-3 border border-sand-6", false: "" }, }, defaultVariants: { bordered: true }, }); -export function Row({ bordered, children }: { bordered?: boolean; children: React.ReactNode }) { +export function Row({ bordered, children }: { bordered?: boolean; children: ReactNode }) { return
    {children}
    ; } ``` +## No structure-changing variants + +Variants tune **look** (size, tone, density) — they must not change a component's **structure, semantics, or prop contract**. When a "variant" would render a different element, accept different props, or fork the behavior, build a **separate component** instead. This keeps each component's typing simple and its rendered element predictable. + +The clearest case is the button family: a clickable action, a styled ``, and a router link are three components, not one `Button` with an `as`/`href`/`to` union. + +### Do / don't: separate components over polymorphic props + +```tsx +// Bad — one component forks structure on props; typing becomes a union mess +type ButtonProps = + | { as?: "button"; onClick: () => void } + | { as: "a"; href: string } + | { as: "link"; to: string }; + +export function Button(props: ButtonProps) { + if (props.as === "a") return ; + if (props.as === "link") return ; + return