Added — manage it in Providers above.
)}
{addError[hit.baseUrl] && (
-
diff --git a/desktop/src/renderer/components/SyncPanel.tsx b/desktop/src/renderer/components/SyncPanel.tsx
index c15b250b..d526dec2 100644
--- a/desktop/src/renderer/components/SyncPanel.tsx
+++ b/desktop/src/renderer/components/SyncPanel.tsx
@@ -10,7 +10,7 @@
*/
import React, { useState, useEffect, useCallback, useRef } from 'react';
-import { Button, Dialog, TextInput, Toggle, LoadingState, SettingRow } from './ui';
+import { Button, Dialog, FieldError, TextInput, Toggle, LoadingState, SettingRow } from './ui';
import type { SyncWarning } from '../../main/sync-state';
import { deriveSettingsRowState, type SyncDisplayState } from '../state/sync-display-state';
import { createPortal } from 'react-dom';
@@ -1818,7 +1818,7 @@ function DevicesTab({ devices, onRename, onRemove, syncInProgress, lastSyncByDev
{/* Why the remove didn't take. Never invents a cause: the handler's own
reason when it gave one, otherwise non-committal. */}
{removeNote?.id === d.id && (
-
{removeNote.text}
+
{removeNote.text}
)}
);
diff --git a/desktop/src/renderer/components/SyncSetupWizard.tsx b/desktop/src/renderer/components/SyncSetupWizard.tsx
index f06666c5..bb7e80a5 100644
--- a/desktop/src/renderer/components/SyncSetupWizard.tsx
+++ b/desktop/src/renderer/components/SyncSetupWizard.tsx
@@ -10,7 +10,7 @@
*/
import { useState, useEffect, useCallback } from 'react';
-import { Button, CloseButton, TextInput, Toggle, Radio, RadioGroup, Callout } from './ui';
+import { Button, CloseButton, FieldError, TextInput, Toggle, Radio, RadioGroup, Callout } from './ui';
import { isAndroid as checkIsAndroid } from '../platform';
import { useEscClose } from '../hooks/use-esc-close';
import { useScrollFade } from '../hooks/useScrollFade';
@@ -846,9 +846,9 @@ function GhInstallHelp({ onRecheck }: { onRecheck: () => void }) {
)}
{installError && (
-
+
Couldn't install it automatically: {installError}
-
+
)}
{/* Manual instructions: only after an automated attempt failed (or on a
diff --git a/desktop/src/renderer/components/ui/states.tsx b/desktop/src/renderer/components/ui/states.tsx
index a2cb359c..285c3fe2 100644
--- a/desktop/src/renderer/components/ui/states.tsx
+++ b/desktop/src/renderer/components/ui/states.tsx
@@ -164,13 +164,30 @@ export function ErrorState(props: ErrorStateProps) {
export type FieldErrorProps = {
children: React.ReactNode;
className?: string;
+ /** Type step. The app has always used both: 19 of the 25 hand-rolled copies
+ * this primitive replaced were `text-3xs`, 6 were `text-2xs`. It is a PROP
+ * rather than something a caller passes through `className` because this
+ * component CONCATENATES className onto the base — and Tailwind resolves two
+ * competing utilities by CSS SOURCE ORDER, not by the order they appear in
+ * the attribute, so `className="text-2xs"` would silently keep rendering at
+ * 3xs (the same trap that made Button's pills render as rectangles). */
+ size?: '3xs' | '2xs';
+ /** Element to render. Default `span` (inline) matches how the primitive
+ * shipped. Pass `p`/`div` where the line is a BLOCK under a field: vertical
+ * margin and padding (`mt-1`, `pb-2`) do not lay out on an inline element,
+ * so a `
` swapped to a bare span would silently lose
+ * its gap wherever the parent is not a flex/grid container. */
+ as?: 'span' | 'p' | 'div';
};
/** Field-level errors stay short lines under the input — not cards. */
-export function FieldError({ children, className = '' }: FieldErrorProps) {
+export function FieldError({ children, className = '', size = '3xs', as: Tag = 'span' }: FieldErrorProps) {
+ // Literal class strings, not `text-${size}` — Tailwind scans source text for
+ // whole class names and never sees an interpolated one.
+ const sizeClass = size === '2xs' ? 'text-2xs' : 'text-3xs';
return (
-
+
{children}
-
+
);
}
diff --git a/desktop/tests/field-error-adoption.test.ts b/desktop/tests/field-error-adoption.test.ts
new file mode 100644
index 00000000..ce2f8f05
--- /dev/null
+++ b/desktop/tests/field-error-adoption.test.ts
@@ -0,0 +1,98 @@
+import { describe, it, expect } from 'vitest';
+import { readFileSync, readdirSync, statSync } from 'node:fs';
+import { join } from 'node:path';
+
+const RENDERER = join(__dirname, '..', 'src', 'renderer');
+
+function walk(dir: string, out: string[] = []): string[] {
+ for (const e of readdirSync(dir)) {
+ const p = join(dir, e);
+ if (statSync(p).isDirectory()) walk(p, out);
+ else if (p.endsWith('.tsx') && !p.endsWith('.test.tsx')) out.push(p);
+ }
+ return out;
+}
+
+/**
+ * ROADMAP "Adopt the `FieldError` primitive": 25 sites across 14 files wrote the
+ * primitive's exact markup by hand. They are gone; this keeps them gone.
+ *
+ * The check is on the CLASS PAIR, because that pair *is* the primitive's body —
+ * anything that renders it by hand is a copy, and the copies drift (the sweep
+ * found the app split between text-3xs and text-2xs, which is why the primitive
+ * gained a `size` prop rather than silently resizing six lines).
+ *
+ * Each exemption below is a site that matches the pair but is NOT a field error.
+ * A new one needs a reason here, not just a name.
+ */
+const EXEMPT: Record = {
+ // Four copies of one static caption under the skip-permissions toggle. Not a
+ // failure report — it is always-on warning copy, and FieldError carries
+ // role="alert", which would make a screen reader interrupt with it every time
+ // the toggle flips. Their real problem is that there are four of them; the fix
+ // is a shared warning component, not this primitive.
+ 'App.tsx': { count: 1, why: 'skip-permissions caption, not a field error' },
+ 'SessionStrip.tsx': { count: 1, why: 'skip-permissions caption, not a field error' },
+ 'ResumeBrowser.tsx': { count: 1, why: 'skip-permissions caption, not a field error' },
+ 'ResumeOptionsPopover.tsx': { count: 1, why: 'skip-permissions caption, not a field error' },
+ // NOT here: SettingsPanel's confirm-dialog prose. It is dimmed to
+ // `text-destructive-fg/80`, and the pattern below excludes the opacity
+ // variants on purpose — an opacity modifier is prose styling, not this
+ // primitive's body. So it needs no exemption, and granting one would have
+ // exempted a file full of real fields.
+ // A destructive text BUTTON (hover fill, padding, rounded), not an error line.
+ 'GitReviewView.tsx': { count: 1, why: 'destructive text button' },
+ // Deliberately role="status" (polite): a failed update check must not
+ // interrupt what the user is reading. FieldError is role="alert".
+ 'UpdateButton.tsx': { count: 1, why: 'role="status" by design' },
+};
+
+/** How many times a file writes the primitive's class pair by hand. The pattern
+ * is deliberately NOT anchored to a tag: a copy is a copy whether it lands on a
+ * , a or a . `text-destructive-fg/80` (an opacity variant) is
+ * excluded — that is prose styling, not this primitive. */
+function handRolledCount(src: string): number {
+ return (src.match(/text-[23]xs text-destructive-fg(?![/\w-])/g) ?? []).length;
+}
+
+describe('FieldError adoption', () => {
+ it('no file hand-rolls the primitive markup', () => {
+ const offenders: string[] = [];
+ for (const file of walk(RENDERER)) {
+ if (file.endsWith(join('ui', 'states.tsx'))) continue; // the primitive itself
+ const base = file.split(/[\\/]/).pop()!;
+ if (base in EXEMPT) continue;
+ if (handRolledCount(readFileSync(file, 'utf8')) > 0) offenders.push(base);
+ }
+ expect(
+ offenders,
+ 'Use
(components/ui/states.tsx). If the site is not a field '
+ + 'error, add it to EXEMPT above with the reason.',
+ ).toEqual([]);
+ });
+
+ // COUNTS, not just names. Exempting a whole FILE would let the next
+ // hand-rolled copy hide inside one — SettingsPanel is exempt for exactly one
+ // paragraph and holds plenty of real fields, so "SettingsPanel is allowed to
+ // match" is too coarse a permission to grant.
+ it('an exemption covers exactly the occurrences it was granted for', () => {
+ const counts = new Map(
+ walk(RENDERER).map((f) => [f.split(/[\\/]/).pop()!, handRolledCount(readFileSync(f, 'utf8'))]),
+ );
+ for (const [name, { count, why }] of Object.entries(EXEMPT)) {
+ expect(counts.get(name) ?? 0, `${name} (exempt: ${why})`).toBe(count);
+ }
+ });
+
+ it('every exemption still matches something', () => {
+ // An exemption that stops being true is a place for the next copy to hide.
+ const seen = new Set(
+ walk(RENDERER)
+ .filter((f) => handRolledCount(readFileSync(f, 'utf8')) > 0)
+ .map((f) => f.split(/[\\/]/).pop()!),
+ );
+ for (const name of Object.keys(EXEMPT)) {
+ expect(seen.has(name), `${name} no longer hand-rolls it — drop the exemption.`).toBe(true);
+ }
+ });
+});
diff --git a/desktop/tests/ui-primitives.test.tsx b/desktop/tests/ui-primitives.test.tsx
index ce6aa066..7ed4f7a7 100644
--- a/desktop/tests/ui-primitives.test.tsx
+++ b/desktop/tests/ui-primitives.test.tsx
@@ -667,5 +667,28 @@ describe('state family', () => {
expect(el.className).toContain('text-destructive');
expect(el.className).toContain('text-3xs');
expect(el.className).not.toContain('text-red-500');
+ expect(el.tagName).toBe('SPAN');
+ });
+
+ // The adoption sweep found the app split between two type steps and a mix of
+ // block/inline hosts, so the primitive takes both as PROPS. Neither could be a
+ // className pass-through: this component concatenates className onto its base,
+ // and Tailwind resolves competing utilities by CSS source order, so a caller's
+ // `text-2xs` would silently keep rendering at 3xs.
+ it('size="2xs" replaces the base step rather than piling on next to it', () => {
+ render(Too short);
+ const el = screen.getByText('Too short');
+ expect(el.className).toContain('text-2xs');
+ expect(el.className).not.toContain('text-3xs');
+ });
+
+ it('as="p" renders a block host so vertical margin still lays out', () => {
+ // `mt-1` on an inline element does nothing; 21 of the swapped sites were
+ // carrying exactly that kind of spacing class.
+ render(Nope);
+ const el = screen.getByText('Nope');
+ expect(el.tagName).toBe('P');
+ expect(el.className).toContain('mt-1');
+ expect(el.getAttribute('role')).toBe('alert');
});
});