From 21b7de57a123af9f289ab818575e2b03a666ca88 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 23 Jul 2026 18:48:15 -0300 Subject: [PATCH] chore(frontend): enforce maintainable text rendering --- docs/FRONTEND_MAINTAINABILITY.md | 66 +++++++++++++ eslint.config.js | 20 ++++ .../(auth)/_components/auth-card-header.tsx | 18 ++-- src/app/(auth)/login.tsx | 9 +- src/app/v/[token]/_layout.tsx | 11 ++- src/app/v/[token]/dashboard.tsx | 7 +- src/app/v/[token]/dispatch/[id].tsx | 27 +++-- src/app/v/[token]/pos.tsx | 7 +- src/components/ui/page-header.tsx | 28 +++--- src/components/ui/text.tsx | 99 +++++++++++++++++++ src/test/components/ui/text.test.tsx | 44 +++++++++ 11 files changed, 297 insertions(+), 39 deletions(-) create mode 100644 docs/FRONTEND_MAINTAINABILITY.md create mode 100644 src/components/ui/text.tsx create mode 100644 src/test/components/ui/text.test.tsx diff --git a/docs/FRONTEND_MAINTAINABILITY.md b/docs/FRONTEND_MAINTAINABILITY.md new file mode 100644 index 00000000..cd7863da --- /dev/null +++ b/docs/FRONTEND_MAINTAINABILITY.md @@ -0,0 +1,66 @@ +# Frontend maintainability conventions + +## Conditional rendering + +Use logical `&&` or the `when` prop on `Text` when JSX has only a rendered state and an empty +state. Use a ternary only when both branches render meaningful alternatives. + +```tsx +{ + error && {error.message}; +} + + + {description} +; +``` + +ESLint rejects `condition ? : null`. This keeps one-sided conditions visually +distinct from real either-or UI decisions. + +## Typography and feedback + +Use `Text` from `@/components/ui/text` for headings, paragraphs, descriptions, labels, captions, +code, and asynchronous feedback. It owns: + +- semantic HTML for each visual variant; +- the display, body, and monospace font families; +- default, muted, error, success, and warning tones; +- accessible `alert` and `status` live regions for error and feedback text; +- one-sided conditional text through `when`. + +ESLint rejects raw paragraph and heading elements. Existing MUI `Typography` usages remain valid, +but new shared UI should prefer `Text` so semantics and design tokens do not drift. + +## Forms and mutations + +Use the libraries already established in the application: + +- React Hook Form owns field registration, touched/dirty state, and client form lifecycle. +- Zod owns form validation and inferred form value types. +- TanStack Query owns server reads and mutations, including pending/error state, cache + invalidation, and retries where safe. + +Do not add TanStack Form alongside React Hook Form. It would create two form conventions without +removing any current dependency. Reconsider only as a deliberate repository-wide migration with +benchmarks, a codemod plan, and an approved deprecation path. + +File uploads are not ordinary form fields. Keep file selection and client validation in a focused +component, and use a TanStack Query mutation for upload progress, errors, completion refresh, and +retry state. Do not place upload orchestration in a route-sized page component. + +## Page state + +Pages should compose focused state components instead of accumulating unrelated booleans: + +- query loading, error, and empty states stay adjacent to the query result; +- mutation pending/error state belongs to the component that initiated the mutation; +- route pages coordinate sections and navigation; +- reusable sections own their interaction details; +- errors render inline with accessible feedback, with toasts reserved for cross-page outcomes. + +## Enforcement and rollout + +The lint rules are repository-wide and the initial violations were migrated in the same change. +`npm run lint`, `npm run build`, and the `Text` behavior tests are required gates. Future +maintainability rules must also land with a green migration rather than a warning-only backlog. diff --git a/eslint.config.js b/eslint.config.js index 8d1f29c2..f88e2a81 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -62,6 +62,26 @@ export default tseslint.config( "react-refresh/only-export-components": ["warn", { allowConstantExport: true }], "@typescript-eslint/no-unused-vars": ["error", { argsIgnorePattern: "^_" }], "@typescript-eslint/no-explicit-any": "warn", + "no-restricted-syntax": [ + "error", + { + selector: + "JSXExpressionContainer > ConditionalExpression[alternate.type='Literal'][alternate.value=null]", + message: + "Use logical AND for one-sided JSX rendering instead of `condition ? element : null`.", + }, + { + selector: + ":matches(JSXOpeningElement[name.name='p'], JSXOpeningElement[name.name='h1'], JSXOpeningElement[name.name='h2'], JSXOpeningElement[name.name='h3'], JSXOpeningElement[name.name='h4'], JSXOpeningElement[name.name='h5'], JSXOpeningElement[name.name='h6'])", + message: + "Use the shared Text component so typography semantics, family, tone, and feedback behavior stay consistent.", + }, + { + selector: + "JSXOpeningElement[name.name='div'] > JSXAttribute[name.name='className'][value.value='vp-error']", + message: 'Use Text variant="error" for accessible, consistent error feedback.', + }, + ], }, }, { diff --git a/src/app/(auth)/_components/auth-card-header.tsx b/src/app/(auth)/_components/auth-card-header.tsx index 795ca0a7..aaf95cf6 100644 --- a/src/app/(auth)/_components/auth-card-header.tsx +++ b/src/app/(auth)/_components/auth-card-header.tsx @@ -1,5 +1,6 @@ import type { ComponentPropsWithoutRef } from "react"; +import { Text } from "@/components/ui/text"; import { cn } from "@/lib/utils"; export type AuthCardHeaderProps = ComponentPropsWithoutRef<"div"> & { @@ -10,12 +11,17 @@ export type AuthCardHeaderProps = ComponentPropsWithoutRef<"div"> & { export function AuthCardHeader({ title, subtitle, className, ...props }: AuthCardHeaderProps) { return (
-

{title}

- {subtitle && ( -

- {subtitle} -

- )} + + {title} + + + {subtitle} +
); } diff --git a/src/app/(auth)/login.tsx b/src/app/(auth)/login.tsx index 9149bc8d..7d1c4c74 100644 --- a/src/app/(auth)/login.tsx +++ b/src/app/(auth)/login.tsx @@ -5,6 +5,7 @@ import { AuthCardHeader } from "@/app/(auth)/_components/auth-card-header"; import { AuthPageShell } from "@/app/(auth)/_components/auth-page-shell"; import { LoginForm } from "@/app/(auth)/_components/login-form"; import { BrandLockup } from "@/components/common/brand-lockup"; +import { Text } from "@/components/ui/text"; import { loginSchema, type LoginFormValues } from "@/domain/auth/schemas/login-schema"; import { useAuthContext } from "@/providers/auth-context"; @@ -37,9 +38,13 @@ export default function LoginPage() { isLoggingIn={isLoggingIn} loginError={loginError} /> -

+ Having trouble? Contact your administrator. -

+ ); } diff --git a/src/app/v/[token]/_layout.tsx b/src/app/v/[token]/_layout.tsx index 548f4ed3..3b4e7880 100644 --- a/src/app/v/[token]/_layout.tsx +++ b/src/app/v/[token]/_layout.tsx @@ -1,6 +1,7 @@ import { NavLink, Outlet, useLocation, useParams } from "react-router"; import { useVendorPortal } from "@/app/v/_components/vendor-portal-context"; import { VendorPortalProvider } from "@/app/v/_components/vendor-portal-provider"; +import { Text } from "@/components/ui/text"; import "@/app/v/_components/vendor-portal.css"; function VendorPortalHeader() { @@ -55,13 +56,13 @@ function VendorPortalBody() { if (status === "error") { return (
-
-

Access Denied

-

+

+ Access Denied + {error?.message || "This link is invalid or has expired. Please contact your dispatcher for a new link."} -

-
+ +
); } diff --git a/src/app/v/[token]/dashboard.tsx b/src/app/v/[token]/dashboard.tsx index a7dc74f9..eeb222a2 100644 --- a/src/app/v/[token]/dashboard.tsx +++ b/src/app/v/[token]/dashboard.tsx @@ -2,6 +2,7 @@ import { useMemo, useState } from "react"; import { useNavigate } from "react-router"; import { useQuery } from "@tanstack/react-query"; import { useVendorPortal } from "@/app/v/_components/vendor-portal-context"; +import { Text } from "@/components/ui/text"; import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; import { formatVendorPortalDateTime, @@ -54,7 +55,7 @@ export default function VendorPortalDashboardPage() { return ( <>
-

Your Work

+ Your Work
Active dispatches and work orders assigned to your company.
@@ -76,7 +77,9 @@ export default function VendorPortalDashboardPage() {
{isLoading &&
Loading dispatches…
} - {error &&
{error.message}
} + + {error?.message} + {!isLoading && !error && filtered.length === 0 && (
No dispatches in this view.
diff --git a/src/app/v/[token]/dispatch/[id].tsx b/src/app/v/[token]/dispatch/[id].tsx index 2edae8a1..854631b9 100644 --- a/src/app/v/[token]/dispatch/[id].tsx +++ b/src/app/v/[token]/dispatch/[id].tsx @@ -3,6 +3,7 @@ import { Link, useParams } from "react-router"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { SignaturePad } from "@/app/v/_components/signature-pad"; import { useVendorPortal } from "@/app/v/_components/vendor-portal-context"; +import { Text } from "@/components/ui/text"; import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; import { formatVendorPortalDateTime, @@ -429,7 +430,11 @@ export default function VendorPortalDispatchPage() { } if (error && !data) { - return
{error.message}
; + return ( + + {error.message} + + ); } if (!data) return null; @@ -457,7 +462,7 @@ export default function VendorPortalDispatchPage() {
-

{data.dispatchNumber}

+ {data.dispatchNumber}
{data.workOrder?.workerOrderTitle}
{data.status} @@ -492,7 +497,9 @@ export default function VendorPortalDispatchPage() { {data.location && ( <> -

Location

+ + Location +
{data.location.name}
{[ @@ -509,7 +516,9 @@ export default function VendorPortalDispatchPage() { {(data.description || data.workOrder?.description) && ( <> -

Description

+ + Description +
{data.description || data.workOrder?.description}
)} @@ -570,7 +579,7 @@ export default function VendorPortalDispatchPage() {
-

Checklist

+ Checklist
-

NTE Uplift Requests

+ NTE Uplift Requests
-

Customer Signoff

+ Customer Signoff
-

Vendor Signoff

+ Vendor Signoff
-

Comments

+ Comments
-

Your Purchase Orders

+ Your Purchase Orders
Every PO issued to your company, with current status and dollar value.
@@ -113,7 +114,9 @@ export default function VendorPortalPosPage() {
{isLoading &&
Loading POs…
} - {error &&
{error.message}
} + + {error?.message} + {!isLoading && !error && visible.length === 0 && (
No POs match this view.
diff --git a/src/components/ui/page-header.tsx b/src/components/ui/page-header.tsx index a43b99e3..dd691a80 100644 --- a/src/components/ui/page-header.tsx +++ b/src/components/ui/page-header.tsx @@ -1,6 +1,7 @@ import type { ReactNode } from "react"; -import { Stack, Typography } from "@mui/material"; +import { Stack } from "@mui/material"; +import { Text } from "@/components/ui/text"; import { cn } from "@/lib/utils"; type PageHeaderProps = { @@ -35,24 +36,25 @@ export function PageHeader({ }} > - {eyebrow && ( -

- {eyebrow} -

- )} -

+ {eyebrow} + + {title} -

- {subtitle && ( - - {subtitle} - - )} + + + {subtitle} + {actions && ( diff --git a/src/components/ui/text.tsx b/src/components/ui/text.tsx new file mode 100644 index 00000000..2628d808 --- /dev/null +++ b/src/components/ui/text.tsx @@ -0,0 +1,99 @@ +import type { ElementType, ReactNode } from "react"; +import { + Typography as MuiTypography, + type TypographyProps as MuiTypographyProps, +} from "@mui/material"; + +type TextVariant = + | "display" + | "title" + | "heading" + | "body" + | "description" + | "label" + | "feedback" + | "error" + | "caption" + | "code"; + +type TextTone = "default" | "muted" | "error" | "success" | "warning"; +type TextFamily = "display" | "body" | "mono"; + +export type TextProps = Omit & { + children: ReactNode; + as?: ElementType; + family?: TextFamily; + tone?: TextTone; + variant?: TextVariant; + when?: boolean; +}; + +const variantConfig: Record< + TextVariant, + { element: ElementType; muiVariant: MuiTypographyProps["variant"]; family: TextFamily } +> = { + display: { element: "h1", muiVariant: "h3", family: "display" }, + title: { element: "h2", muiVariant: "h5", family: "display" }, + heading: { element: "h3", muiVariant: "h6", family: "display" }, + body: { element: "p", muiVariant: "body1", family: "body" }, + description: { element: "p", muiVariant: "body2", family: "body" }, + label: { element: "span", muiVariant: "subtitle2", family: "body" }, + feedback: { element: "p", muiVariant: "body2", family: "body" }, + error: { element: "p", muiVariant: "body2", family: "body" }, + caption: { element: "span", muiVariant: "caption", family: "body" }, + code: { element: "code", muiVariant: "body2", family: "mono" }, +}; + +const toneColor: Record = { + default: "text.primary", + muted: "text.secondary", + error: "error", + success: "success.main", + warning: "warning.main", +}; + +const familyValue: Record = { + display: "var(--font-display)", + body: "var(--font-sans)", + mono: "var(--font-mono)", +}; + +export function Text({ + as, + children, + family, + tone, + variant = "body", + when = true, + sx, + ...props +}: TextProps) { + if (!when) { + return null; + } + + const config = variantConfig[variant]; + const resolvedTone = tone ?? (variant === "error" ? "error" : "default"); + let liveProps = {}; + if (variant === "error") { + liveProps = { role: "alert", "aria-live": "assertive" as const }; + } else if (variant === "feedback") { + liveProps = { role: "status", "aria-live": "polite" as const }; + } + + return ( + + {children} + + ); +} diff --git a/src/test/components/ui/text.test.tsx b/src/test/components/ui/text.test.tsx new file mode 100644 index 00000000..59b4ce93 --- /dev/null +++ b/src/test/components/ui/text.test.tsx @@ -0,0 +1,44 @@ +import { screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { Text } from "@/components/ui/text"; +import { renderWithProviders } from "@/test/test-utils"; + +describe("Text", () => { + it("maps visual variants to semantic elements", () => { + renderWithProviders( + <> + Page title + Supporting copy + , + { withAuth: false }, + ); + + expect(screen.getByRole("heading", { level: 1, name: "Page title" })).toBeInTheDocument(); + expect(screen.getByText("Supporting copy").tagName).toBe("P"); + }); + + it("does not render conditional text when its condition is false", () => { + renderWithProviders( + + Saved + , + { withAuth: false }, + ); + + expect(screen.queryByText("Saved")).not.toBeInTheDocument(); + }); + + it("gives error and feedback messages accessible live-region semantics", () => { + renderWithProviders( + <> + Upload failed + Uploading + , + { withAuth: false }, + ); + + expect(screen.getByRole("alert")).toHaveTextContent("Upload failed"); + expect(screen.getByRole("status")).toHaveTextContent("Uploading"); + }); +});