From afa346918243302911ddcb3981653659e7dda4e2 Mon Sep 17 00:00:00 2001 From: simonfrvr Date: Wed, 21 Oct 2026 08:46:09 +0000 Subject: [PATCH] fix(a11y,api,config): close assigned issues #993 #997 #998 #999 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #999: ImageUpload now asks the uploader for alt text and keeps it editable after upload; file names are used only as a last resort, and meaningless descriptions ("IMG_0001", data:/blob: URLs) are never read aloud. #998: every API failure normalizes to { code, message, statusCode, timestamp } via ApiErrorResponse, ApiError.toJSON, toApiErrorResponse and withErrorHandling; plain-text and { error } envelopes are handled too. #997: env validation is extracted into a pure validateEnv() so the fail-fast rules can be tested, with coverage for missing, invalid and non-HTTPS values. #993: the portfolio grid now exposes its arrow-key/Enter controls through aria-describedby and includes item status in each cell's accessible name. Also repairs two merge-corrupted modules (duplicate useQuery import, duplicate test helper) that blocked the build and the test suite. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- docs/architecture.md | 16 ++ src/components/ImageUpload/ImageUpload.tsx | 100 ++++++++-- src/components/PortfolioGrid.tsx | 138 +++++++------ src/config/env.test.ts | 133 +++++++++++++ src/config/env.ts | 46 +++-- src/config/index.ts | 19 +- src/features/payments/PaymentResultScreen.tsx | 1 - src/lib/altText.test.ts | 87 +++++++++ src/lib/altText.ts | 132 +++++++++++++ src/lib/analytics.test.ts | 6 - src/lib/api-errors.ts | 53 +++++ src/lib/index.ts | 2 + src/services/http.ts | 56 +++++- src/services/index.ts | 1 + src/test/ApiError.test.ts | 183 ++++++++++++++++++ src/test/ImageUpload.test.tsx | 108 +++++++++++ src/test/PortfolioGrid.test.tsx | 39 ++++ src/types/api-error.ts | 49 +++++ src/types/index.ts | 1 + 19 files changed, 1054 insertions(+), 116 deletions(-) create mode 100644 src/config/env.test.ts create mode 100644 src/lib/altText.test.ts create mode 100644 src/lib/altText.ts create mode 100644 src/lib/api-errors.ts create mode 100644 src/test/ApiError.test.ts create mode 100644 src/test/ImageUpload.test.tsx create mode 100644 src/types/api-error.ts diff --git a/docs/architecture.md b/docs/architecture.md index b324968c..8a85716d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -41,6 +41,22 @@ promoted down a layer (`lib/`, `hooks/`, `types/`, `components/ui/`) instead of 5. **Undocumented locations need a doc update first.** Where new code goes should never be a guess; if no row in "Where Does New Code Go?" fits, the table is extended in the same PR. +## API errors + +Every failed API call is normalized into one shape (`ApiErrorResponse` in `src/types/api-error.ts`): + +```ts +{ code: string; message: string; statusCode: number | null; timestamp: string } +``` + +`services/http.ts` produces it from the envelopes the backend and its gateways actually send (`{ message }`, +`{ error }`, short plain text, HTML error pages), and `ApiError` implements the contract so `code`, `message`, +`statusCode` and `timestamp` are always present. Errors that never reached a server use `statusCode: null`. + +At a boundary that may throw (`toApiErrorResponse`, `withErrorHandling` in `src/lib/api-errors.ts`), call +`withErrorHandling` to get a discriminated result instead of a thrown error, and `toApiErrorResponse` to log or +report the canonical payload. Do not format errors ad hoc in components; read `message` from the normalized error. + ## Known exceptions These seven deep imports (they bypass a feature barrel) exist today. (`main.tsx` and `services/http.ts` no longer reach into feature internals.) They are tolerated, not endorsed: each lists why it diff --git a/src/components/ImageUpload/ImageUpload.tsx b/src/components/ImageUpload/ImageUpload.tsx index 3fe4182e..2a047d38 100644 --- a/src/components/ImageUpload/ImageUpload.tsx +++ b/src/components/ImageUpload/ImageUpload.tsx @@ -1,5 +1,6 @@ -import { useState, useCallback, useRef } from 'react' +import { useState, useCallback, useId, useRef } from 'react' import { Spinner } from '@/components/ui' +import { MAX_ALT_TEXT_LENGTH, resolveAltText } from '@/lib/altText' interface ImageUploadProps { /** Current image URL to display */ @@ -24,6 +25,12 @@ interface ImageUploadProps { error?: string | null /** Disabled state */ disabled?: boolean + /** Saved alt text for the current image; prefills the editable field. */ + altText?: string + /** Called with the new value whenever the user edits the alt text. */ + onAltTextChange?: (altText: string) => void + /** Wording used when neither the user nor the file name describes the image. */ + altFallback?: string } const DEFAULT_MAX_SIZE = 10 * 1024 * 1024 // 10MB @@ -41,9 +48,31 @@ export default function ImageUpload({ uploadProgress = 0, error = null, disabled = false, + altText, + onAltTextChange, + altFallback, }: ImageUploadProps) { const [dragActive, setDragActive] = useState(false) const fileInputRef = useRef(null) + const altInputId = useId() + const altHintId = `${altInputId}-hint` + const fileInputId = useId() + + // The field starts from the saved value; once the user types, their draft wins + // so an upload completing mid-edit cannot overwrite it. + const [draftAltText, setDraftAltText] = useState(null) + const fallbackAlt = altFallback ?? `${label} image` + const altTextValue = + draftAltText ?? altText ?? (currentImage ? resolveAltText('', currentImage, fallbackAlt) : '') + const effectiveAltText = resolveAltText(altTextValue, currentImage, fallbackAlt) + + const handleAltTextChange = useCallback( + (e: React.ChangeEvent) => { + setDraftAltText(e.target.value) + onAltTextChange?.(e.target.value) + }, + [onAltTextChange], + ) const validateFile = (file: File): string | null => { if (!acceptedTypes.includes(file.type)) { @@ -71,7 +100,7 @@ export default function ImageUpload({ onFileSelect(file) }, - [acceptedTypes, maxSize, onFileSelect] + [acceptedTypes, maxSize, onFileSelect], ) const handleDrop = useCallback( @@ -83,16 +112,19 @@ export default function ImageUpload({ handleFiles(e.dataTransfer.files) } }, - [disabled, handleFiles] + [disabled, handleFiles], ) - const handleDragOver = useCallback((e: React.DragEvent) => { - e.preventDefault() - e.stopPropagation() - if (!disabled) { - setDragActive(true) - } - }, [disabled]) + const handleDragOver = useCallback( + (e: React.DragEvent) => { + e.preventDefault() + e.stopPropagation() + if (!disabled) { + setDragActive(true) + } + }, + [disabled], + ) const handleDragLeave = useCallback((e: React.DragEvent) => { e.preventDefault() @@ -106,7 +138,7 @@ export default function ImageUpload({ handleFiles(e.target.files) } }, - [handleFiles] + [handleFiles], ) const handleClick = useCallback(() => { @@ -120,7 +152,7 @@ export default function ImageUpload({ e.stopPropagation() onRemove?.() }, - [onRemove] + [onRemove], ) const formatFileSize = (bytes: number): string => { @@ -132,7 +164,9 @@ export default function ImageUpload({ return (
- + {currentImage && onRemove && !isUploading && ( + + , + ) + + const cells = screen.getAllByRole('gridcell') + await user.tab() + expect(screen.getByRole('button', { name: 'Before grid' })).toHaveFocus() + + await user.tab() + expect(cells[0]).toHaveFocus() + + await user.tab() + expect(cells[0]).not.toHaveFocus() + }) + it('keeps exactly one cell in the tab order and moves it with the arrow keys', async () => { const user = userEvent.setup() render() diff --git a/src/types/api-error.ts b/src/types/api-error.ts new file mode 100644 index 00000000..290707d2 --- /dev/null +++ b/src/types/api-error.ts @@ -0,0 +1,49 @@ +/** + * Canonical API error contract (#998). + * + * Every failed API call is normalized into this shape before it reaches a + * caller, so components, logging and tests never have to cope with the + * different envelopes the backend and its gateways produce + * (`{ message }`, `{ error }`, plain text, HTML error pages). + */ +export interface ApiErrorResponse { + /** Stable, machine-readable code, e.g. `VALIDATION_FAILED`. */ + code: string + /** User-readable copy. Already curated, so it is safe to render. */ + message: string + /** HTTP status, or `null` when the request never reached the server. */ + statusCode: number | null + /** ISO-8601 timestamp of when the failure was normalized. */ + timestamp: string +} + +/** + * Shape of an error body before normalization. Mirrors the wire DTO but is + * intentionally loose: proxies and gateways send partial envelopes. + */ +export interface ApiErrorPayload { + code?: string + message?: string + error?: string + errors?: + | Record + | Array<{ + field?: string + path?: string | string[] + message?: string + } | null> + requestId?: string +} + +/** Type guard for the canonical error shape. */ +export function isApiErrorResponse(value: unknown): value is ApiErrorResponse { + if (typeof value !== 'object' || value === null) return false + const candidate = value as Partial + return ( + typeof candidate.code === 'string' && + typeof candidate.message === 'string' && + (typeof candidate.statusCode === 'number' || candidate.statusCode === null) && + typeof candidate.timestamp === 'string' && + !Number.isNaN(Date.parse(candidate.timestamp)) + ) +} diff --git a/src/types/index.ts b/src/types/index.ts index fbdc0519..f42763c9 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -4,4 +4,5 @@ * Types that belong to a single domain live inside that feature folder. */ export * from './api' +export * from './api-error' export type * from './views'