From 6bcb7461be73b9d6b7bb89e98d691c0254a6a118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20R=C3=A9?= Date: Sun, 28 Jun 2026 18:40:08 +0200 Subject: [PATCH] Address compliance-portal review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the valid issues raised in the scaffold review. UI kit: the Button loading state now replaces only the leading icon instead of dropping the label, Button consumes the `active` variant so it no longer leaks onto the DOM, and every v2 skeleton sets aria-hidden after the prop spread so a consumer cannot override it. @probo/relay: guard the caller-supplied onCompleted/onError callbacks so a throwing callback still settles the awaitable mutation promise instead of leaving it pending. compliance-portal: normalize external website hrefs and read hostname via URL.hostname, add a localized catch-all not-found route, and widen the .gitattributes glob so colocated __generated__ artifacts at any depth are marked generated. Docs: correct the forms guide (Base UI passes plain values, Zod v3 flatten API), spread the child fragment in the permissions example, and drop references to v2 components that do not exist in the ui guide. Signed-off-by: Émile Ré --- apps/compliance-portal/.gitattributes | 3 +- .../compliance-portal/src/_locales/en-US.json | 5 +++ .../compliance-portal/src/_locales/fr-FR.json | 5 +++ .../Hero/OrganizationContactInfo.tsx | 4 +- .../compliance-portal/src/lib/url/hostname.ts | 15 ++++++- .../src/pages/NotFoundPage.tsx | 42 +++++++++++++++++++ apps/compliance-portal/src/routes.tsx | 4 ++ contrib/claude/forms.md | 6 +-- contrib/claude/permissions.md | 3 ++ contrib/claude/ui.md | 6 +-- packages/relay/src/useMutation.ts | 23 +++++++++- packages/ui/src/v2/Avatar/AvatarSkeleton.tsx | 2 +- packages/ui/src/v2/Badge/BadgeSkeleton.tsx | 2 +- packages/ui/src/v2/Button/Button.tsx | 16 +++---- packages/ui/src/v2/Button/ButtonSkeleton.tsx | 2 +- .../ui/src/v2/Callout/CalloutSkeleton.tsx | 2 +- packages/ui/src/v2/Card/CardSkeleton.tsx | 2 +- .../src/v2/IconButton/IconButtonSkeleton.tsx | 2 +- .../ui/src/v2/typography/HeadingSkeleton.tsx | 2 +- .../ui/src/v2/typography/TextSkeleton.tsx | 2 +- 20 files changed, 116 insertions(+), 32 deletions(-) create mode 100644 apps/compliance-portal/src/pages/NotFoundPage.tsx diff --git a/apps/compliance-portal/.gitattributes b/apps/compliance-portal/.gitattributes index 7a1fb9a7a..644ba1f2c 100644 --- a/apps/compliance-portal/.gitattributes +++ b/apps/compliance-portal/.gitattributes @@ -1,2 +1 @@ -__generated__/*.graphql linguist-generated -__generated__/*.js linguist-generated +**/__generated__/** linguist-generated diff --git a/apps/compliance-portal/src/_locales/en-US.json b/apps/compliance-portal/src/_locales/en-US.json index a4a7548e4..3c63de137 100644 --- a/apps/compliance-portal/src/_locales/en-US.json +++ b/apps/compliance-portal/src/_locales/en-US.json @@ -28,5 +28,10 @@ "requests": { "title": "Data Requests", "newRequest": "New Request" + }, + "notFound": { + "title": "Page not found", + "description": "The page you are looking for does not exist or has moved.", + "backHome": "Back to home" } } diff --git a/apps/compliance-portal/src/_locales/fr-FR.json b/apps/compliance-portal/src/_locales/fr-FR.json index 429514070..88c6c2ced 100644 --- a/apps/compliance-portal/src/_locales/fr-FR.json +++ b/apps/compliance-portal/src/_locales/fr-FR.json @@ -28,5 +28,10 @@ "requests": { "title": "Demandes de données", "newRequest": "Nouvelle demande" + }, + "notFound": { + "title": "Page introuvable", + "description": "La page que vous recherchez n'existe pas ou a été déplacée.", + "backHome": "Retour à l'accueil" } } diff --git a/apps/compliance-portal/src/components/Hero/OrganizationContactInfo.tsx b/apps/compliance-portal/src/components/Hero/OrganizationContactInfo.tsx index 7e2f85db4..b0bbf596d 100644 --- a/apps/compliance-portal/src/components/Hero/OrganizationContactInfo.tsx +++ b/apps/compliance-portal/src/components/Hero/OrganizationContactInfo.tsx @@ -16,7 +16,7 @@ import { EnvelopeIcon, GlobeSimpleIcon, MapPinSimpleIcon } from "@phosphor-icons import { Text } from "@probo/ui/src/v2/typography/Text"; import { graphql, useFragment } from "react-relay"; -import { hostnameOf } from "#/lib/url/hostname"; +import { externalHref, hostnameOf } from "#/lib/url/hostname"; import type { OrganizationContactInfo_organization$key } from "./__generated__/OrganizationContactInfo_organization.graphql"; import { organizationContactInfo } from "./variants"; @@ -54,7 +54,7 @@ export function OrganizationContactInfo({ organizationKey }: OrganizationContact {hasWebsite && ( diff --git a/apps/compliance-portal/src/lib/url/hostname.ts b/apps/compliance-portal/src/lib/url/hostname.ts index 6b7e69696..875632256 100644 --- a/apps/compliance-portal/src/lib/url/hostname.ts +++ b/apps/compliance-portal/src/lib/url/hostname.ts @@ -12,12 +12,25 @@ // OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +// Prepend https:// when a URL carries no http(s) scheme, so a protocol-less +// value (e.g. "blaxel.ai") parses as absolute instead of being treated as a +// relative path. +function withHttpScheme(url: string): string { + return /^https?:\/\//i.test(url) ? url : `https://${url}`; +} + // Show only the hostname for a URL (e.g. "https://blaxel.ai/x" -> "blaxel.ai"), // falling back to the raw value when it cannot be parsed. export function hostnameOf(url: string): string { try { - return new URL(url).host; + return new URL(withHttpScheme(url)).hostname; } catch { return url; } } + +// Build a safe absolute href for an external link, normalizing the scheme so a +// protocol-less value does not resolve as a relative link. +export function externalHref(url: string): string { + return withHttpScheme(url); +} diff --git a/apps/compliance-portal/src/pages/NotFoundPage.tsx b/apps/compliance-portal/src/pages/NotFoundPage.tsx new file mode 100644 index 000000000..3a5f71347 --- /dev/null +++ b/apps/compliance-portal/src/pages/NotFoundPage.tsx @@ -0,0 +1,42 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +import { Link } from "@probo/ui/src/v2/Button/Link"; +import { Heading } from "@probo/ui/src/v2/typography/Heading"; +import { Text } from "@probo/ui/src/v2/typography/Text"; +import { useTranslation } from "react-i18next"; + +import { HeaderBand } from "#/components/HeaderBand/HeaderBand"; + +// Catch-all page for portal paths that match no route, so an unknown URL renders +// an explicit not-found state inside the layout instead of an empty body. +export default function NotFoundPage() { + const { t } = useTranslation(); + + return ( + +
+ + {t("notFound.title")} + + + {t("notFound.description")} + + + {t("notFound.backHome")} + +
+ + ); +} diff --git a/apps/compliance-portal/src/routes.tsx b/apps/compliance-portal/src/routes.tsx index e97dfad47..996602e12 100644 --- a/apps/compliance-portal/src/routes.tsx +++ b/apps/compliance-portal/src/routes.tsx @@ -47,6 +47,10 @@ const routes = [ path: "requests", Component: lazy(() => import("#/pages/RequestsPage")), }, + { + path: "*", + Component: lazy(() => import("#/pages/NotFoundPage")), + }, ], }, ] satisfies AppRoute[]; diff --git a/contrib/claude/forms.md b/contrib/claude/forms.md index 475fdcc24..acf322e6c 100644 --- a/contrib/claude/forms.md +++ b/contrib/claude/forms.md @@ -93,10 +93,10 @@ export function CreateMeasureForm({ onValid }: CreateMeasureFormProps) {
{ - const result = schema.safeParse(Object.fromEntries(formData)); + onFormSubmit={(formValues) => { + const result = schema.safeParse(formValues); if (!result.success) { - setErrors(z.flattenError(result.error).fieldErrors); + setErrors(result.error.flatten().fieldErrors); return; } onValid(result.data); diff --git a/contrib/claude/permissions.md b/contrib/claude/permissions.md index bb00bb95e..e0f0c5d3f 100644 --- a/contrib/claude/permissions.md +++ b/contrib/claude/permissions.md @@ -23,12 +23,15 @@ const documentListItemFragment = graphql` title canUpdate: permission(action: "core:document:update") canDelete: permission(action: "core:document:delete") + ...EditDocumentDialog_document } `; ``` Colocate the permission with the action it gates — never drill a `canDelete` boolean down as a prop from a parent (the same data-as-props rule as everywhere else; see [`react-components.md`](react-components.md#props-are-for-configuration-and-composition-not-data)). +Spread a child's fragment (`...EditDocumentDialog_document`) when you forward the node to that child as a fragment key. `useFragment` returns plain data, but Relay keeps the spread fragment refs on it, so the resolved `document` doubles as the key the child's own `useFragment` expects. Without the spread, `documentKey={document}` would hand the child masked data with no ref and fail at runtime (see [`relay.md`](relay.md)). + ## Gate the action on the boolean Read the boolean via `useFragment` and gate the control. Default to **hiding** an action the user cannot perform; **disable** (with an explanatory tooltip) only when the action's *absence* would be confusing. diff --git a/contrib/claude/ui.md b/contrib/claude/ui.md index 2ca777b82..d621ed190 100644 --- a/contrib/claude/ui.md +++ b/contrib/claude/ui.md @@ -295,13 +295,13 @@ Components fall into two categories: **primitives** and **compound** components. ### Primitives -**Primitives** (`Text`, `Image`, form inputs, layout helpers, `ListItem`) are self-contained — they render a single semantic element with their own styling. A primitive **is its own shell**: there is no separate shell wrapper. Each primitive has a paired skeleton (`TextSkeleton`, `ImageSkeleton`) that matches its dimensions. +**Primitives** (`Text`, `Avatar`, `Badge`, form inputs, layout helpers) are self-contained — they render a single semantic element with their own styling. A primitive **is its own shell**: there is no separate shell wrapper. Each primitive has a paired skeleton (`TextSkeleton`, `AvatarSkeleton`) that matches its dimensions. ### Compound components -**Compound components** (`ImageCard`, …) assemble multiple primitives into a larger region. When logic (state, effects, data) lives inside the top-level component, a **shell** separates layout from behavior: +**Compound components** (`Card`, `Dropdown`, …) assemble multiple primitives into a larger region. When logic (state, effects, data) lives inside the top-level component, a **shell** separates layout from behavior: -- **Shell** — pure layout frame that accepts region props (`image`, `text`, …) as `ReactNode` and applies `tv` slot classes. No state, no effects, no data. +- **Shell** — pure layout frame that accepts region props (`media`, `text`, …) as `ReactNode` and applies `tv` slot classes. No state, no effects, no data. - **Root** — owns the logic and renders the shell, passing primitives into its region props. - **Skeleton** — reuses the **same shell** with skeleton primitives, so the loading placeholder is structurally identical without pulling in the logic graph. diff --git a/packages/relay/src/useMutation.ts b/packages/relay/src/useMutation.ts index 6a116d834..ac94d4c3d 100644 --- a/packages/relay/src/useMutation.ts +++ b/packages/relay/src/useMutation.ts @@ -81,11 +81,24 @@ export function createUseMutation(useNotifier: () => MutationNotifier) { ); } + function toError(value: unknown): Error { + return value instanceof Error ? value : new Error(String(value)); + } + return new Promise((resolve, reject) => { commit({ ...config, onCompleted: (response, errors) => { - config.onCompleted?.(response, errors); + // A throwing caller callback must still settle the wrapper promise, + // otherwise `await mutate()` would hang forever. + try { + config.onCompleted?.(response, errors); + } catch (callbackError) { + const error = toError(callbackError); + notifyError(error); + reject(error); + return; + } if (errors && errors.length > 0) { const [payloadError] = errors; notifyError(payloadError); @@ -102,7 +115,13 @@ export function createUseMutation(useNotifier: () => MutationNotifier) { resolve(response); }, onError: (error) => { - config.onError?.(error); + // Swallow a throwing caller callback so the original mutation error + // still flows through to the notifier and the rejection. + try { + config.onError?.(error); + } catch { + // Intentionally ignored: the mutation error below is authoritative. + } notifyError(error); reject(error); }, diff --git a/packages/ui/src/v2/Avatar/AvatarSkeleton.tsx b/packages/ui/src/v2/Avatar/AvatarSkeleton.tsx index 39d9ae3e3..2e34ccbf9 100644 --- a/packages/ui/src/v2/Avatar/AvatarSkeleton.tsx +++ b/packages/ui/src/v2/Avatar/AvatarSkeleton.tsx @@ -24,5 +24,5 @@ export type AvatarSkeletonProps = Omit, "children"> & Var export function AvatarSkeleton(props: AvatarSkeletonProps) { const { size, radius, className, ...rest } = props; - return ; + return ; } diff --git a/packages/ui/src/v2/Badge/BadgeSkeleton.tsx b/packages/ui/src/v2/Badge/BadgeSkeleton.tsx index 9eabaf544..1c857b255 100644 --- a/packages/ui/src/v2/Badge/BadgeSkeleton.tsx +++ b/packages/ui/src/v2/Badge/BadgeSkeleton.tsx @@ -24,5 +24,5 @@ export type BadgeSkeletonProps = Omit, "children"> & Vari export function BadgeSkeleton(props: BadgeSkeletonProps) { const { size, className, ...rest } = props; - return ; + return ; } diff --git a/packages/ui/src/v2/Button/Button.tsx b/packages/ui/src/v2/Button/Button.tsx index bba2ce54b..c57b5aef8 100644 --- a/packages/ui/src/v2/Button/Button.tsx +++ b/packages/ui/src/v2/Button/Button.tsx @@ -32,7 +32,7 @@ export type ButtonProps // or router link (Link) are separate components. See contrib/claude/ui.md. export function Button(props: ButtonProps) { const { - size, variant, color, highContrast, className, + size, variant, color, highContrast, active, className, iconStart, iconEnd, loading = false, disabled, type = "button", children, ...rest } = props; @@ -41,18 +41,12 @@ export function Button(props: ButtonProps) { type={type} disabled={disabled || loading} aria-busy={loading || undefined} - className={button({ size, variant, color, highContrast, className })} + className={button({ size, variant, color, highContrast, active, className })} {...rest} > - {loading - ? - : ( - <> - {iconStart} - {children} - {iconEnd} - - )} + {loading ? : iconStart} + {children} + {iconEnd} ); } diff --git a/packages/ui/src/v2/Button/ButtonSkeleton.tsx b/packages/ui/src/v2/Button/ButtonSkeleton.tsx index 35446477d..c69bb901a 100644 --- a/packages/ui/src/v2/Button/ButtonSkeleton.tsx +++ b/packages/ui/src/v2/Button/ButtonSkeleton.tsx @@ -24,5 +24,5 @@ export type ButtonSkeletonProps = Omit, "children"> & Var export function ButtonSkeleton(props: ButtonSkeletonProps) { const { size, className, ...rest } = props; - return ; + return ; } diff --git a/packages/ui/src/v2/Callout/CalloutSkeleton.tsx b/packages/ui/src/v2/Callout/CalloutSkeleton.tsx index b283ae755..7bf76d6b9 100644 --- a/packages/ui/src/v2/Callout/CalloutSkeleton.tsx +++ b/packages/ui/src/v2/Callout/CalloutSkeleton.tsx @@ -23,5 +23,5 @@ export type CalloutSkeletonProps = Omit, "children"> & Var export function CalloutSkeleton(props: CalloutSkeletonProps) { const { size, className, ...rest } = props; - return
; + return
; } diff --git a/packages/ui/src/v2/Card/CardSkeleton.tsx b/packages/ui/src/v2/Card/CardSkeleton.tsx index a58fbddeb..ac72ff7ed 100644 --- a/packages/ui/src/v2/Card/CardSkeleton.tsx +++ b/packages/ui/src/v2/Card/CardSkeleton.tsx @@ -23,5 +23,5 @@ export type CardSkeletonProps = Omit, "children"> & Varian export function CardSkeleton(props: CardSkeletonProps) { const { size, className, ...rest } = props; - return
; + return
; } diff --git a/packages/ui/src/v2/IconButton/IconButtonSkeleton.tsx b/packages/ui/src/v2/IconButton/IconButtonSkeleton.tsx index d38f7d8eb..a5e21a355 100644 --- a/packages/ui/src/v2/IconButton/IconButtonSkeleton.tsx +++ b/packages/ui/src/v2/IconButton/IconButtonSkeleton.tsx @@ -24,5 +24,5 @@ export type IconButtonSkeletonProps = Omit, "children"> & export function IconButtonSkeleton(props: IconButtonSkeletonProps) { const { size, className, ...rest } = props; - return ; + return ; } diff --git a/packages/ui/src/v2/typography/HeadingSkeleton.tsx b/packages/ui/src/v2/typography/HeadingSkeleton.tsx index 8917213fb..882dc5478 100644 --- a/packages/ui/src/v2/typography/HeadingSkeleton.tsx +++ b/packages/ui/src/v2/typography/HeadingSkeleton.tsx @@ -26,7 +26,7 @@ export function HeadingSkeleton(props: HeadingSkeletonProps) { const { size, className, ...rest } = props; return ( - + {"\u00A0"} ); diff --git a/packages/ui/src/v2/typography/TextSkeleton.tsx b/packages/ui/src/v2/typography/TextSkeleton.tsx index 55d28ef1d..5cc492833 100644 --- a/packages/ui/src/v2/typography/TextSkeleton.tsx +++ b/packages/ui/src/v2/typography/TextSkeleton.tsx @@ -26,7 +26,7 @@ export function TextSkeleton(props: TextSkeletonProps) { const { size, className, ...rest } = props; return ( - + {"\u00A0"} );