From a0684b919a90cae04108d928c51792b0c359745d Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 03:00:20 +0000 Subject: [PATCH] refactor: extract shared ui/ProgressBar and migrate six hand-rolled meters (#4138) --- .changelog/next/changed-issue-4138.md | 1 + .../meatspace/post/MemoryPractice.jsx | 36 +++-- .../meatspace/post/PostLlmDrillRunner.jsx | 61 ++++---- .../pipeline/AutopilotMilestones.jsx | 21 ++- .../manuscript/ManuscriptReadAloud.jsx | 5 +- .../pipeline/stages/EpisodeVideoStage.jsx | 12 +- client/src/components/ui/ProgressBar.jsx | 114 +++++++++++++++ client/src/components/ui/ProgressBar.test.jsx | 130 ++++++++++++++++++ client/src/components/ui/README.md | 48 +++++++ client/src/pages/Loras.jsx | 8 +- 10 files changed, 367 insertions(+), 69 deletions(-) create mode 100644 .changelog/next/changed-issue-4138.md create mode 100644 client/src/components/ui/ProgressBar.jsx create mode 100644 client/src/components/ui/ProgressBar.test.jsx create mode 100644 client/src/components/ui/README.md diff --git a/.changelog/next/changed-issue-4138.md b/.changelog/next/changed-issue-4138.md new file mode 100644 index 0000000000..3cf6c03b36 --- /dev/null +++ b/.changelog/next/changed-issue-4138.md @@ -0,0 +1 @@ +- Shared ui/ProgressBar primitive replaces six hand-rolled progress meters; every meter now carries role=progressbar with aria-valuenow/min/max and an accessible name diff --git a/client/src/components/meatspace/post/MemoryPractice.jsx b/client/src/components/meatspace/post/MemoryPractice.jsx index 92422a9408..0b71c0577d 100644 --- a/client/src/components/meatspace/post/MemoryPractice.jsx +++ b/client/src/components/meatspace/post/MemoryPractice.jsx @@ -1,6 +1,7 @@ import { useState, useRef, useEffect, useMemo } from 'react'; import { ChevronLeft, Check, X, SkipForward, RotateCcw, Target, ChevronDown, Loader, ShieldCheck } from 'lucide-react'; import { submitMemoryPractice, getChunkMastery, getMemoryItem, attestMemoryMastery } from '../../../services/api'; +import ProgressBar from '../../ui/ProgressBar'; import PostCompletionActions from './PostCompletionActions'; import { startRetryableSave } from './completionSave'; @@ -386,7 +387,7 @@ function MemoryPracticeRunner({ item, mode, onSelectMode, onExitMode, onBack, on - + {/* Chunk info */}
@@ -480,7 +481,7 @@ function MemoryPracticeRunner({ item, mode, onSelectMode, onExitMode, onBack, on {currentIdx + 1} / {lines.length}
- +
@@ -547,7 +548,7 @@ function MemoryPracticeRunner({ item, mode, onSelectMode, onExitMode, onBack, on {currentIdx + 1} / {lines.length - 1}
- +
Current line:
@@ -629,7 +630,7 @@ function MemoryPracticeRunner({ item, mode, onSelectMode, onExitMode, onBack, on {currentIdx + 1} / {lines.length}
- +
{displayText}
@@ -904,14 +905,18 @@ function ChunkMasteryOverview({ item }) { const accuracy = typeof stats?.masteredAt === 'string' ? 100 : stats?.attempts > 0 ? Math.round((stats.correct / stats.attempts) * 100) : 0; - const barColor = accuracy >= 80 ? 'bg-port-success' : accuracy >= 40 ? 'bg-port-warning' : 'bg-gray-600'; + const barTone = accuracy >= 80 ? 'success' : accuracy >= 40 ? 'warning' : 'muted'; return (
{chunk.label} -
-
-
+ {accuracy}%
); @@ -962,12 +967,17 @@ function SpeedRunLine({ line, index, onResult }) { ); } -function ProgressBar({ current, total }) { - const pct = Math.round((current / total) * 100); +// The name stays generic rather than "line 3 of 12": the spaced mode measures a +// synthetic chunk×line position, so only the percentage is meaningful across all +// four modes — and `aria-valuenow` already carries it. +function PracticeProgress({ current, total }) { return ( -
-
-
+ ); } diff --git a/client/src/components/meatspace/post/PostLlmDrillRunner.jsx b/client/src/components/meatspace/post/PostLlmDrillRunner.jsx index 52a259fbe8..55d98ea647 100644 --- a/client/src/components/meatspace/post/PostLlmDrillRunner.jsx +++ b/client/src/components/meatspace/post/PostLlmDrillRunner.jsx @@ -1,5 +1,6 @@ import { useState, useEffect, useRef, useCallback } from 'react'; import { CheckCircle, XCircle } from 'lucide-react'; +import ProgressBar from '../../ui/ProgressBar'; import { scorePostLlmDrill } from '../../../services/api'; import { DRILL_LABELS, WORDPLAY_LLM_DRILL_TYPES } from './constants'; import { AILoadingIndicator, MissedExamplesDisplay, CompoundChainUI, BridgeWordUI, DoubleMeaningUI, IdiomTwistUI, scoreWordplayResponse } from './WordplayDrillUI'; @@ -188,9 +189,9 @@ export default function PostLlmDrillRunner({ drill, timeLimitSec, drillIndex, dr } const timePct = timeLimitMs > 0 ? (timeLeft / timeLimitMs) * 100 : 0; - let timerColor = 'bg-port-accent'; - if (timePct <= 10) timerColor = 'bg-port-error'; - else if (timePct <= 25) timerColor = 'bg-port-warning'; + let timerTone = 'accent'; + if (timePct <= 10) timerTone = 'error'; + else if (timePct <= 25) timerTone = 'warning'; // Training mode: feedback overlay if (isTraining && trainingFeedback) { @@ -231,14 +232,7 @@ export default function PostLlmDrillRunner({ drill, timeLimitSec, drillIndex, dr > Next -
-
- Prompt {questionIndex + 1} of {totalPrompts} -
-
-
-
-
+
); } @@ -257,9 +251,14 @@ export default function PostLlmDrillRunner({ drill, timeLimitSec, drillIndex, dr {/* Timer bar (hidden in training mode) */} {!isTraining && ( <> -
-
-
+
{Math.ceil(timeLeft / 1000)}s remaining
)} @@ -427,7 +426,7 @@ export default function PostLlmDrillRunner({ drill, timeLimitSec, drillIndex, dr
- + )} @@ -532,17 +531,23 @@ export function buildLlmResponseObj({ drillType, questionIndex, items, inputValu // DRILL-SPECIFIC UI COMPONENTS // ───────────────────────────────────────────────────────────────────────────── -function ProgressBar({ index, total }) { +// "Prompt N of M" counter over the shared meter. Both knobs exist for the +// training-feedback screen, which draws the same counter without the numeric +// readout and in the accent-2 tone that marks training throughout this runner. +function PromptProgress({ index, total, tone = 'accent', showPercent = true }) { const pct = total > 0 ? ((index + 1) / total) * 100 : 0; return (
Prompt {index + 1} of {total} - {Math.round(pct)}% -
-
-
+ {showPercent && {Math.round(pct)}%}
+
); } @@ -586,7 +591,7 @@ function WordAssociationUI({ prompt, inputValue, setInputValue, onSubmit, inputR placeholder="Type your associations..." buttonLabel="Next" /> - + ); } @@ -605,7 +610,7 @@ function StoryRecallUI({ exercise, phase, onStartRecall, items, inputValue, setI > I'm Ready — Show Questions - + ); } @@ -647,7 +652,7 @@ function StoryRecallUI({ exercise, phase, onStartRecall, items, inputValue, setI Submit All Answers )} - + ); } @@ -701,7 +706,7 @@ function VerbalFluencyUI({ category, items, inputValue, setInputValue, onAddItem > Done — Submit {items.length} items - + ); } @@ -729,7 +734,7 @@ function WitComebackUI({ scenario, inputValue, setInputValue, onSubmit, inputRef placeholder="Your witty response..." buttonLabel="Next" /> - + ); } @@ -753,7 +758,7 @@ function PunWordplayUI({ challenge, inputValue, setInputValue, onSubmit, inputRe placeholder="Your pun or wordplay..." buttonLabel="Next" /> - + ); } @@ -776,7 +781,7 @@ function ImaginationUI({ label, prompt, badge, badgeColor, placeholder, inputVal placeholder={placeholder} buttonLabel="Next" /> - + ); } @@ -827,7 +832,7 @@ function AlternativeUsesUI({ object, items, inputValue, setInputValue, onAddItem > Done — Submit {items.length} uses - + ); } diff --git a/client/src/components/pipeline/AutopilotMilestones.jsx b/client/src/components/pipeline/AutopilotMilestones.jsx index 8e0e1d4440..5bdffb1761 100644 --- a/client/src/components/pipeline/AutopilotMilestones.jsx +++ b/client/src/components/pipeline/AutopilotMilestones.jsx @@ -8,6 +8,7 @@ import { isStoppedTerminal, MILESTONE_STATUS, } from '../../lib/autopilotMilestones'; +import ProgressBar from '../ui/ProgressBar'; // Per-status chrome in ONE table — icon and tones together, so a new status // can't get a row color and no icon (or the reverse). `blocked` reuses the @@ -53,19 +54,13 @@ export default function AutopilotMilestones({ plan, planTotals, progress, termin {/* Overall meter. A plan snapshot can under-count a step the run repeats, so this is honest about milestones settled — not a time estimate. */} {!dryRun ? ( -
-
-
+ ) : null} {/* Capped so a long plan (a comic series can project 16 milestones) can't diff --git a/client/src/components/pipeline/manuscript/ManuscriptReadAloud.jsx b/client/src/components/pipeline/manuscript/ManuscriptReadAloud.jsx index 28fcf6ffee..c7e9f82271 100644 --- a/client/src/components/pipeline/manuscript/ManuscriptReadAloud.jsx +++ b/client/src/components/pipeline/manuscript/ManuscriptReadAloud.jsx @@ -22,6 +22,7 @@ import { Play, Pause, Square, Loader2, Volume2, AlertTriangle } from 'lucide-rea import Modal from '../../ui/Modal'; import VoicePicker from '../../voice/VoicePicker'; import toast from '../../ui/Toast'; +import ProgressBar from '../../ui/ProgressBar'; import { formatDurationMs } from '../../../utils/formatters'; import { narratePipelineProse } from '../../../services/api'; import { STAGE_LABEL } from './constants'; @@ -293,9 +294,7 @@ export default function ManuscriptReadAloud({ open, onClose, section }) { {segments ? (
-
-
-
+
{segments.length} sentence{segments.length === 1 ? '' : 's'} diff --git a/client/src/components/pipeline/stages/EpisodeVideoStage.jsx b/client/src/components/pipeline/stages/EpisodeVideoStage.jsx index fba87d0c32..94931971e9 100644 --- a/client/src/components/pipeline/stages/EpisodeVideoStage.jsx +++ b/client/src/components/pipeline/stages/EpisodeVideoStage.jsx @@ -3,6 +3,7 @@ import { Link } from 'react-router'; import { Film, ExternalLink, Loader2, Sparkles, AlertCircle, CheckCircle2 } from 'lucide-react'; import toast from '../../ui/Toast'; import Banner from '../../ui/Banner'; +import ProgressBar from '../../ui/ProgressBar'; import { generatePipelineVisualImage, listVideoModels } from '../../../services/api'; import { getCreativeDirectorProject } from '../../../services/apiCreativeDirector'; import { getSceneStatusBadge, PROJECT_STATUS_LABEL } from '../../creative-director/sceneStatus'; @@ -360,12 +361,11 @@ export default function EpisodeVideoStage({ issue, onStageUpdate }) { {total > 0 && (
-
-
-
+
    {sortedScenes.map((s) => { const badge = getSceneStatusBadge(s.status); diff --git a/client/src/components/ui/ProgressBar.jsx b/client/src/components/ui/ProgressBar.jsx new file mode 100644 index 0000000000..02ef584662 --- /dev/null +++ b/client/src/components/ui/ProgressBar.jsx @@ -0,0 +1,114 @@ +/** + * ProgressBar — the shared horizontal meter: a rounded track with a tone-colored + * fill sized to `percent`. + * + * Six surfaces had hand-rolled the same `h-1.5 rounded-full` track + + * `style={{ width: `${pct}%` }}` fill (LoRA downloads, manuscript read-aloud, + * episode-video scenes, the POST drill runner, memory practice, and the + * autopilot milestone meter). They had already drifted on accessibility — only + * two carried `role="progressbar"` with `aria-valuenow`, so the rest were + * invisible to a screen reader. That is the point of centralizing: the ARIA trio + * plus an accessible name is emitted here, once, for every host. + * + * Knobs map to real call-site shapes — nothing speculative: + * percent — 0..100, clamped. `null`/`undefined` means INDETERMINATE (the + * LoRA installer gets no Content-Length from some mirrors) and + * draws a pulsing stub with no `aria-valuenow`. A non-finite + * number (a `0/0` ratio) is a *broken* measurement, not an absent + * one, so it renders an empty determinate bar rather than + * silently claiming "indeterminate". + * tone — semantic fill color. The drill timer swaps accent → warning → + * error as it runs out; chunk mastery swaps muted → warning → + * success; a stopped autopilot run reads warning. + * label — the accessible name. Required in spirit; defaults to 'Progress' + * so the trio is never nameless. + * size — `sm` (h-1.5, default) or `md` (h-2, the drill timer). + * track — `bg` (on a card, default) or `border` (on the page ground, where + * `bg-port-bg` would vanish). + * duration — fill transition in ms. Static class map, because Tailwind can't + * see an interpolated `duration-${n}` and would drop it from the + * build. + * `className` passes through for layout only (`flex-1`, `mt-1.5`). + */ + +const TONES = { + accent: 'bg-port-accent', + accent2: 'bg-port-accent-2', + success: 'bg-port-success', + warning: 'bg-port-warning', + error: 'bg-port-error', + muted: 'bg-gray-600', +}; + +const SIZES = { + sm: 'h-1.5', + md: 'h-2', +}; + +const TRACKS = { + bg: 'bg-port-bg', + border: 'bg-port-border', +}; + +// Interpolated Tailwind class names are invisible to the build, so the only +// durations available are the ones spelled out here. +const DURATIONS = { + 100: 'duration-100', + 150: 'duration-150', + 200: 'duration-200', + 300: 'duration-300', + 500: 'duration-500', +}; + +// `null`/`undefined` = "we can't measure this" (indeterminate). Anything else is +// a measurement, and a broken one reads as 0 rather than collapsing into the +// indeterminate sentinel. +export function clampPercent(percent) { + if (percent === null || percent === undefined) return null; + const n = Number(percent); + if (!Number.isFinite(n)) return 0; + return Math.min(100, Math.max(0, n)); +} + +export default function ProgressBar({ + percent, + tone = 'accent', + label = 'Progress', + size = 'sm', + track = 'bg', + duration = 200, + className = '', +}) { + const value = clampPercent(percent); + const indeterminate = value === null; + const fillTone = TONES[tone] || TONES.accent; + const trackCls = [ + 'w-full overflow-hidden rounded-full', + SIZES[size] || SIZES.sm, + TRACKS[track] || TRACKS.bg, + className, + ] + .join(' ') + .replace(/\s+/g, ' ') + .trim(); + const fillCls = [ + 'h-full rounded-full', + fillTone, + indeterminate + ? 'w-1/3 animate-pulse' + : `transition-[width] ${DURATIONS[duration] || DURATIONS[200]}`, + ].join(' '); + + return ( +
    +
    +
    + ); +} diff --git a/client/src/components/ui/ProgressBar.test.jsx b/client/src/components/ui/ProgressBar.test.jsx new file mode 100644 index 0000000000..baae793097 --- /dev/null +++ b/client/src/components/ui/ProgressBar.test.jsx @@ -0,0 +1,130 @@ +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; + +import ProgressBar, { clampPercent } from './ProgressBar.jsx'; + +// The fill is the only child of the track, and it carries no role of its own. +const fillOf = (container) => container.querySelector('[role="progressbar"] > div'); + +describe('ProgressBar', () => { + it('always emits the ARIA trio plus an accessible name', () => { + render(); + const bar = screen.getByRole('progressbar', { name: 'Download progress' }); + expect(bar).toHaveAttribute('aria-valuenow', '42'); + expect(bar).toHaveAttribute('aria-valuemin', '0'); + expect(bar).toHaveAttribute('aria-valuemax', '100'); + }); + + it('names the bar even when the host forgets a label', () => { + render(); + expect(screen.getByRole('progressbar', { name: 'Progress' })).toBeInTheDocument(); + }); + + it('sizes the fill to the percentage', () => { + const { container } = render(); + expect(fillOf(container)).toHaveStyle({ width: '37.5%' }); + }); + + it.each([ + ['over 100', 140, '100%', '100'], + ['below zero', -20, '0%', '0'], + ])('clamps a percentage %s', (_label, percent, width, valuenow) => { + const { container } = render(); + expect(fillOf(container)).toHaveStyle({ width }); + expect(screen.getByRole('progressbar')).toHaveAttribute('aria-valuenow', valuenow); + }); + + it('rounds aria-valuenow but keeps the fractional width', () => { + const { container } = render(); + expect(screen.getByRole('progressbar')).toHaveAttribute('aria-valuenow', '33'); + expect(fillOf(container)).toHaveStyle({ width: '33.333%' }); + }); + + // A `0 / 0` ratio is a BROKEN measurement, not an absent one — it must not + // collapse into the indeterminate sentinel and start pulsing. + it.each([['NaN', NaN], ['Infinity', Infinity]])( + 'reads a non-finite %s measurement as an empty determinate bar', + (_label, percent) => { + const { container } = render(); + expect(screen.getByRole('progressbar')).toHaveAttribute('aria-valuenow', '0'); + expect(fillOf(container).className).not.toContain('animate-pulse'); + }, + ); + + it.each([['null', null], ['undefined', undefined]])( + 'draws an indeterminate pulse with no aria-valuenow for a %s percentage', + (_label, percent) => { + const { container } = render(); + const bar = screen.getByRole('progressbar', { name: 'Downloading' }); + expect(bar).not.toHaveAttribute('aria-valuenow'); + // The bounds stay, so assistive tech still reads it as a 0..100 meter. + expect(bar).toHaveAttribute('aria-valuemin', '0'); + expect(bar).toHaveAttribute('aria-valuemax', '100'); + const fill = fillOf(container); + expect(fill.className).toContain('animate-pulse'); + expect(fill.getAttribute('style')).toBeFalsy(); + }, + ); + + it.each([ + ['accent', 'bg-port-accent'], + ['accent2', 'bg-port-accent-2'], + ['success', 'bg-port-success'], + ['warning', 'bg-port-warning'], + ['error', 'bg-port-error'], + ['muted', 'bg-gray-600'], + ])('paints the %s tone', (tone, cls) => { + const { container } = render(); + expect(fillOf(container).className).toContain(cls); + }); + + it('falls back to the accent tone for an unknown tone', () => { + const { container } = render(); + expect(fillOf(container).className).toContain('bg-port-accent'); + }); + + it('switches track height and ground on size / track', () => { + const { container } = render(); + const bar = screen.getByRole('progressbar'); + expect(bar.className).toContain('h-2'); + expect(bar.className).toContain('bg-port-border'); + expect(container.querySelector('.h-1\\.5')).toBeNull(); + }); + + it('defaults to the small size on the card ground', () => { + render(); + const bar = screen.getByRole('progressbar'); + expect(bar.className).toContain('h-1.5'); + expect(bar.className).toContain('bg-port-bg'); + }); + + it('passes layout classes through to the track', () => { + render(); + expect(screen.getByRole('progressbar').className).toContain('flex-1 mt-1.5'); + }); + + // Tailwind can't see an interpolated `duration-${n}`, so only mapped values + // survive the build — an unmapped one has to fall back, not emit a dead class. + it('emits a static duration class and falls back for an unmapped one', () => { + const { container: mapped } = render(); + expect(fillOf(mapped).className).toContain('duration-500'); + const { container: unmapped } = render(); + expect(fillOf(unmapped).className).toContain('duration-200'); + expect(fillOf(unmapped).className).not.toContain('duration-137'); + }); +}); + +describe('clampPercent', () => { + it.each([ + ['null stays indeterminate', null, null], + ['undefined stays indeterminate', undefined, null], + ['NaN reads as zero', NaN, 0], + ['a non-numeric string reads as zero', 'abc', 0], + ['a numeric string is parsed', '60', 60], + ['over 100 clamps down', 101, 100], + ['under 0 clamps up', -1, 0], + ['an in-range value passes through', 12.5, 12.5], + ])('%s', (_label, input, expected) => { + expect(clampPercent(input)).toBe(expected); + }); +}); diff --git a/client/src/components/ui/README.md b/client/src/components/ui/README.md new file mode 100644 index 0000000000..64b406fbc2 --- /dev/null +++ b/client/src/components/ui/README.md @@ -0,0 +1,48 @@ +# `client/src/components/ui/` — shared UI primitives + +Presentational building blocks with no domain knowledge. **Grep this catalog before +hand-rolling chrome** — nearly every primitive here exists because the same markup had +already been copy-pasted across three-to-ten surfaces and drifted (usually on +accessibility). Feature-specific components live under their own feature directory +(`components/pipeline/`, `components/meatspace/`, …), not here. + +| Component | What it's for | +| --- | --- | +| `AutoSizeTextarea` | Controlled `