diff --git a/e2e/baseball-box-score.spec.ts b/e2e/baseball-box-score.spec.ts index 017cb1aeb..2c608c586 100644 --- a/e2e/baseball-box-score.spec.ts +++ b/e2e/baseball-box-score.spec.ts @@ -46,6 +46,19 @@ const SEEDED = const SCHEDULED_OPPONENT = 'Riverside University'; const COMPLETED_OPPONENT = 'Eastview College'; +/** + * Matches a game detail redirect, e.g. `/stats/games/`. + * + * `baseball_games.id` is a Postgres `uuid` (`DEFAULT gen_random_uuid()`), so + * matching the UUID shape — instead of the previous loose + * `[a-zA-Z0-9-]+$` — structurally excludes `/stats/games/create` (issue + * #952): the create-form route itself is 6 letters, never a UUID, so a + * submit that silently hangs on the create page (no redirect at all) can no + * longer false-pass this assertion. + */ +const GAME_DETAIL_URL_RE = + /\/stats\/games\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + /** * Service-role Supabase client for teardown-only writes (deleting rows this * spec itself created) — provided by scripts/e2e-supabase-admin.ts so this @@ -185,7 +198,7 @@ test.describe('Coach - Create New Game', () => { await page.locator('#new-game-venue').fill('E2E Created Field'); await page.getByRole('button', { name: /Create Game/i }).click(); - await expect(page).toHaveURL(/\/stats\/games\/[a-zA-Z0-9-]+$/, { timeout: 8000 }); + await expect(page).toHaveURL(GAME_DETAIL_URL_RE, { timeout: 8000 }); await waitForPageLoad(page); // Newly created, uncompleted game lands directly on the manual entry form. @@ -269,7 +282,7 @@ test.describe('Coach - Manual Box Score Entry', () => { await page.getByRole('button', { name: /Save Box Score/i }).click(); - await expect(page).toHaveURL(/\/stats\/games\/[a-zA-Z0-9-]+$/, { timeout: 8000 }); + await expect(page).toHaveURL(GAME_DETAIL_URL_RE, { timeout: 8000 }); await waitForPageLoad(page); // BoxScoreView now renders the just-completed game (read-only display — @@ -349,7 +362,7 @@ test.describe('Coach - Games List and Box Score View', () => { const gameCard = page.locator('[data-testid="game-card"]', { hasText: COMPLETED_OPPONENT }); await gameCard.getByRole('link').click(); await waitForPageLoad(page); - await expect(page).toHaveURL(/\/stats\/games\/[a-zA-Z0-9-]+$/); + await expect(page).toHaveURL(GAME_DETAIL_URL_RE); }); test('should display the box score view with the FINAL score and result badge', async ({ page }) => { diff --git a/src/app/admin/golf/__tests__/honest-rounds-delta.test.ts b/src/app/admin/golf/__tests__/honest-rounds-delta.test.ts new file mode 100644 index 000000000..ec7dbb6eb --- /dev/null +++ b/src/app/admin/golf/__tests__/honest-rounds-delta.test.ts @@ -0,0 +1,36 @@ +import { describe, it, expect } from 'vitest'; +import { honestRoundsDelta } from '../honest-rounds-delta'; + +/** + * ============================================================================ + * honestRoundsDelta (bug #949 #6 — "Rounds this week" stray arrow, no sparkline) + * ---------------------------------------------------------------------------- + * The KpiTile's `delta` used to be unconditional (`roundsThisWeek - + * roundsLastWeek`), decoupled from the `trendData` sparkline's own 2-point + * floor. A team with <2 weeks of `roundsByWeek` history rendered the + * TrendChip arrow with no sparkline beneath it. This locks the fix: the + * delta is only ever honest (a real number) when the trend series that will + * accompany it actually has enough points to draw. + * ========================================================================== */ +describe('honestRoundsDelta', () => { + it('is undefined with fewer than 2 weeks of history (no arrow without its sparkline)', () => { + expect(honestRoundsDelta([], 5, 0)).toBeUndefined(); + expect(honestRoundsDelta([{ week: '2026-07-13', count: 5 }], 5, 0)).toBeUndefined(); + }); + + it('is the real week-over-week delta once 2+ weeks of history exist', () => { + const roundsByWeek = [ + { week: '2026-07-06', count: 3 }, + { week: '2026-07-13', count: 5 }, + ]; + expect(honestRoundsDelta(roundsByWeek, 5, 3)).toBe(2); + }); + + it('a real zero-round last week still yields a real (not fabricated) delta once history exists', () => { + const roundsByWeek = [ + { week: '2026-07-06', count: 0 }, + { week: '2026-07-13', count: 4 }, + ]; + expect(honestRoundsDelta(roundsByWeek, 4, 0)).toBe(4); + }); +}); diff --git a/src/app/admin/golf/honest-rounds-delta.ts b/src/app/admin/golf/honest-rounds-delta.ts new file mode 100644 index 000000000..6b2dadb7f --- /dev/null +++ b/src/app/admin/golf/honest-rounds-delta.ts @@ -0,0 +1,22 @@ +/** + * Bug #949 #6 — the "Rounds this week" KpiTile used to pass an unconditional + * `delta={roundsThisWeek - roundsLastWeek}` alongside a `trendData` series + * that can genuinely be shorter than 2 points (a new team's first week live, + * or any week `roundsByWeek`'s 12-week window hasn't filled yet). StatTile's + * own sparkline only renders once its trend series has 2+ finite points, but + * `delta` had no matching gate — so the TrendChip arrow rendered ALONE, with + * no sparkline beneath it, whenever the week-history was thin. Gating the + * delta on the SAME 2-point floor keeps the arrow and its sparkline paired: + * neither renders without the other. + * + * Lives outside page.tsx: the admin-gate coverage tripwire requires every + * export of a page/layout/actions file to reach the gate, and this is a pure + * presentation helper. + */ +export function honestRoundsDelta( + roundsByWeek: ReadonlyArray, + roundsThisWeek: number, + roundsLastWeek: number, +): number | undefined { + return roundsByWeek.length >= 2 ? roundsThisWeek - roundsLastWeek : undefined; +} diff --git a/src/app/admin/golf/page.tsx b/src/app/admin/golf/page.tsx index 22bc9c1ad..5beca6099 100644 --- a/src/app/admin/golf/page.tsx +++ b/src/app/admin/golf/page.tsx @@ -16,6 +16,7 @@ import { PlayerWatchlist } from '../_components/PlayerWatchlist'; import { LocalTime } from '../_components/LocalTime'; import { AutoRefresh } from '../_components/AutoRefresh'; import { FeatureHealthRollup } from '../_components/FeatureHealthRollup'; +import { honestRoundsDelta } from './honest-rounds-delta'; export const dynamic = 'force-dynamic'; @@ -142,7 +143,7 @@ async function GolfBody() { w.count)} /> diff --git a/src/app/baseball/(dashboard)/dashboard/stats/games/create/NewGameClient.tsx b/src/app/baseball/(dashboard)/dashboard/stats/games/create/NewGameClient.tsx index 98707d35f..c4f7cec97 100644 --- a/src/app/baseball/(dashboard)/dashboard/stats/games/create/NewGameClient.tsx +++ b/src/app/baseball/(dashboard)/dashboard/stats/games/create/NewGameClient.tsx @@ -8,6 +8,7 @@ import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import { PaperCard } from '@/components/baseball/living-annual'; import { InlineNotice } from '@/components/fairway'; +import { toast } from '@/components/ui/sonner'; interface NewGameClientProps { teamId: string; @@ -38,21 +39,39 @@ export function NewGameClient({ teamId, teamName }: NewGameClientProps) { setSaving(true); setError(null); - const result = await createGame(teamId, { - game_date: gameDate, - game_type: gameType, - opponent_name: opponentName || undefined, - location: location || undefined, - home_away: homeAway, - event_time: eventTime || undefined, - create_calendar_event: createCalendarEvent, - }); - - if (result.success && result.data) { - router.push(`/baseball/dashboard/stats/games/${result.data.id}`); - } else { + try { + const result = await createGame(teamId, { + game_date: gameDate, + game_type: gameType, + opponent_name: opponentName || undefined, + location: location || undefined, + home_away: homeAway, + event_time: eventTime || undefined, + create_calendar_event: createCalendarEvent, + }); + + if (result.success && result.data) { + router.push(`/baseball/dashboard/stats/games/${result.data.id}`); + return; + } + setError(result.error ?? 'Failed to create game'); setSaving(false); + } catch (err) { + // createGame is server-side wrapped (withBaseballAction has its own + // top-level try/catch) and should always RESOLVE to a + // CreateGameResult — but the client-side call is a network round trip + // to invoke the server action, which CAN reject outright (dropped + // connection, server restart mid-request, aborted navigation). Before + // this fix nothing caught that rejection: `saving` stayed true + // forever, the submit button stayed disabled at "Creating…", and the + // coach got no feedback at all (issue #952). Form state (all the typed + // fields above) is untouched either way, so the coach can just retry. + const message = + err instanceof Error ? err.message : 'Failed to create game. Please try again.'; + setError(message); + toast.error('Could not create game', { description: message }); + setSaving(false); } } diff --git a/src/app/baseball/(dashboard)/dashboard/stats/games/create/__tests__/NewGameClient.test.tsx b/src/app/baseball/(dashboard)/dashboard/stats/games/create/__tests__/NewGameClient.test.tsx new file mode 100644 index 000000000..ae1835a5a --- /dev/null +++ b/src/app/baseball/(dashboard)/dashboard/stats/games/create/__tests__/NewGameClient.test.tsx @@ -0,0 +1,121 @@ +// ============================================================================= +// NewGameClient.test.tsx — issue #952 +// +// Before this fix, `handleSubmit` awaited `createGame()` with no try/catch. +// `createGame` is server-side wrapped and should always RESOLVE to a +// `CreateGameResult`, but the client-side call is a network round trip to +// invoke the server action, which CAN reject outright (dropped connection, +// server restart mid-request). A rejection stranded `saving=true` forever: +// the submit button stayed disabled at "Creating…" with no error shown at +// all. This guards the fix: a rejected `createGame()` call resets `saving` +// (the submit button becomes clickable again), surfaces the error inline AND +// via the repo's shared toast, and leaves the form fields untouched. +// ============================================================================= + +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const createGameMock = vi.fn(); +vi.mock('@/app/baseball/actions/games', () => ({ + createGame: (...args: unknown[]) => createGameMock(...args), +})); + +const routerPushMock = vi.fn(); +vi.mock('next/navigation', () => ({ + useRouter: () => ({ push: routerPushMock, back: vi.fn() }), +})); + +const toastErrorMock = vi.fn(); +vi.mock('@/components/ui/sonner', () => ({ + toast: { + error: (...args: unknown[]) => toastErrorMock(...args), + success: vi.fn(), + warning: vi.fn(), + }, +})); + +import { NewGameClient } from '../NewGameClient'; + +function submitButton() { + return screen.getByRole('button', { name: /Create Game|Creating/i }); +} + +describe('NewGameClient — createGame() rejection is caught (#952)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('resets saving, surfaces the error inline + via toast, and keeps form state when createGame() rejects', async () => { + createGameMock.mockRejectedValue(new Error('fetch failed')); + + render(); + + // Fill a field so we can assert it survives the failed submit. + fireEvent.change(screen.getByLabelText(/Opponent Name/), { + target: { value: 'State University' }, + }); + + fireEvent.click(submitButton()); + + // Immediately disabled + relabeled while the (doomed) request is in flight. + expect(screen.getByRole('button', { name: 'Creating…' })).toBeDisabled(); + + // The rejection is caught: the button recovers instead of hanging forever. + await waitFor(() => + expect(screen.getByRole('button', { name: /Create Game/i })).toBeEnabled(), + ); + + expect(screen.getByText('fetch failed')).toBeInTheDocument(); + expect(toastErrorMock).toHaveBeenCalledWith('Could not create game', { + description: 'fetch failed', + }); + expect(routerPushMock).not.toHaveBeenCalled(); + + // Form state is preserved — nothing was cleared on failure. + expect(screen.getByLabelText(/Opponent Name/)).toHaveValue('State University'); + }); + + it('falls back to a generic message when createGame() rejects with a non-Error value', async () => { + createGameMock.mockRejectedValue('boom'); + + render(); + fireEvent.click(submitButton()); + + await waitFor(() => + expect(screen.getByText('Failed to create game. Please try again.')).toBeInTheDocument(), + ); + expect(screen.getByRole('button', { name: /Create Game/i })).toBeEnabled(); + expect(toastErrorMock).toHaveBeenCalledWith('Could not create game', { + description: 'Failed to create game. Please try again.', + }); + }); + + it('resets saving and shows the inline error (no toast) when createGame() resolves with success: false', async () => { + createGameMock.mockResolvedValue({ success: false, error: 'Team not found' }); + + render(); + fireEvent.click(submitButton()); + + await waitFor(() => expect(screen.getByText('Team not found')).toBeInTheDocument()); + expect(screen.getByRole('button', { name: /Create Game/i })).toBeEnabled(); + // The logical `success: false` path is unchanged by this fix — it never + // toasted before, and still shouldn't (only the newly-caught rejection + // path gets the extra toast signal). + expect(toastErrorMock).not.toHaveBeenCalled(); + }); + + it('navigates to the new game and does not reset saving on success', async () => { + createGameMock.mockResolvedValue({ success: true, data: { id: 'game-123' } }); + + render(); + fireEvent.click(submitButton()); + + await waitFor(() => + expect(routerPushMock).toHaveBeenCalledWith( + '/baseball/dashboard/stats/games/game-123', + ), + ); + // Deliberately left disabled through navigation — see NewGameClient.tsx. + expect(screen.getByRole('button', { name: 'Creating…' })).toBeDisabled(); + }); +}); diff --git a/src/app/golf/(auth)/demo/page.tsx b/src/app/golf/(auth)/demo/page.tsx index 8ab7323dc..4eed524aa 100644 --- a/src/app/golf/(auth)/demo/page.tsx +++ b/src/app/golf/(auth)/demo/page.tsx @@ -133,14 +133,19 @@ function DemoGateContent() { } } + // #950 — the full brand→headline→card→footer entrance used to take ~1.05s + // (0.5s/0.65s durations + up to a 0.55s stagger delay); a conversion page + // should finish composing well under 1s. Same stagger SHAPE (brand, then + // headline, then the card, then the footer), just tightened durations/ + // delays so the whole sequence settles by ~0.5s. const motionCard = prefersReducedMotion ? { duration: 0 } - : { duration: 0.65, ease: [0.16, 1, 0.3, 1] as [number, number, number, number] }; + : { duration: 0.35, ease: [0.16, 1, 0.3, 1] as [number, number, number, number] }; const motionStagger = (delay: number) => prefersReducedMotion ? { duration: 0 } - : { duration: 0.5, delay, ease: [0.16, 1, 0.3, 1] as [number, number, number, number] }; + : { duration: 0.25, delay, ease: [0.16, 1, 0.3, 1] as [number, number, number, number] }; return ( @@ -203,7 +208,7 @@ function DemoGateContent() {

diff --git a/src/app/golf/(dashboard)/FairwayDashboardShell.tsx b/src/app/golf/(dashboard)/FairwayDashboardShell.tsx index 56441200a..a858a1bba 100644 --- a/src/app/golf/(dashboard)/FairwayDashboardShell.tsx +++ b/src/app/golf/(dashboard)/FairwayDashboardShell.tsx @@ -18,7 +18,7 @@ * overflow surfaces. * ========================================================================== */ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import dynamic from 'next/dynamic'; import Link from 'next/link'; import Image from 'next/image'; @@ -46,6 +46,7 @@ import { type GolfNavBadgeCounts, } from '@/lib/golf/nav-registry'; import { surfaceName, surfaceHref } from '@/lib/golf/surface-registry'; +import { isPageScrollHomeEndTarget, shouldResetScrollOnNavigate } from '@/lib/golf/scroll-behavior'; import { SidebarProvider, useSidebar } from '@/contexts/sidebar-context'; import { MobileNavProvider } from '@/contexts/mobile-nav-context'; @@ -329,6 +330,67 @@ function FairwayDashboardContent({ const { displayDensity, showAnimations } = useAppearancePreferences(); const role: Role = userData.role === 'coach' ? 'coach' : 'player'; + // #947: reset the document scroll position to the top on a route change. + // This shell mounts ONCE per session (it lives in (dashboard)/layout.tsx, + // which persists across every sibling navigation) and the dashboard is a + // plain document-scrolling page (see globals.css's `overflow-x: clip` + // comment — no inner `overflow-y-auto` wrapper), so without this a + // navigation to a new route inherited whatever scrollY the PREVIOUS page + // was left at (Dashboard → Brief landing mid-page instead of at the top). + // Browser back/forward is deliberately excluded — a `popstate` listener + // flags the next pathname change as "the browser already restored scroll + // for this one", so native back-button semantics are untouched — and a + // destination hash (`#section`) is excluded so anchor links still work. + // See `src/lib/golf/scroll-behavior.ts` for the (unit-tested) decision. + const isPopStateRef = useRef(false); + useEffect(() => { + const onPopState = () => { + isPopStateRef.current = true; + }; + window.addEventListener('popstate', onPopState); + return () => window.removeEventListener('popstate', onPopState); + }, []); + + const previousPathnameRef = useRef(null); + useEffect(() => { + const wasPopState = isPopStateRef.current; + isPopStateRef.current = false; // consume — good for exactly one pathname change + const reset = shouldResetScrollOnNavigate({ + previousPathname: previousPathnameRef.current, + nextPathname: pathname, + isPopState: wasPopState, + hash: typeof window !== 'undefined' ? window.location.hash : '', + }); + previousPathnameRef.current = pathname; + if (reset) window.scrollTo({ top: 0, left: 0, behavior: 'instant' }); + }, [pathname]); + + // #947: the dashboard content area ignored Home/End keyboard scrolling. + // The shell is document-scrolling (no inner `overflow-y-auto` wrapper — + // see the scroll-reset effect above), so this is a `window`-level listener + // rather than a handler scoped to one element: whatever currently has + // focus (a nav item, a card action, or nothing more specific than + // `document.body`) should still let Home/End move the page, exactly as a + // plain document would without any shell chrome layered over it. Native + // form controls and composite ARIA widgets (tabs/listbox/menu/grid/tree) + // that legitimately own Home/End for their own first/last-item navigation + // are excluded — see `isPageScrollHomeEndTarget`. + useEffect(() => { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key !== 'Home' && event.key !== 'End') return; + if (event.metaKey || event.ctrlKey || event.altKey || event.shiftKey) return; + if (!isPageScrollHomeEndTarget(event.target instanceof Element ? event.target : null)) return; + event.preventDefault(); + window.scrollTo({ + top: event.key === 'Home' ? 0 : document.documentElement.scrollHeight, + left: 0, + behavior: 'instant', + }); + }; + window.addEventListener('keydown', onKeyDown); + return () => window.removeEventListener('keydown', onKeyDown); + }, []); + // TeamSwitcher (program heads only): renders in the glass top bar's action // cluster — desktop AND mobile — when the coach is a multi-team head coach. const coachTeams = useMemo(() => userData.coachTeams ?? [], [userData.coachTeams]); diff --git a/src/app/golf/(dashboard)/dashboard/alerts/loading.tsx b/src/app/golf/(dashboard)/dashboard/alerts/loading.tsx index e8f9e7fc8..4a70fb5e9 100644 --- a/src/app/golf/(dashboard)/dashboard/alerts/loading.tsx +++ b/src/app/golf/(dashboard)/dashboard/alerts/loading.tsx @@ -9,6 +9,13 @@ import { Skeleton } from '@/components/fairway/feedback/Skeleton'; * layout: 3 MetricCard tiles + the toolbar row + a hero InsightCard + compact * card rows — all in Fairway tokens (bg-canvas / rounded-card / Skeleton's * matte sweep) so the live feed lands without a content jump. + * + * #947 fix: eyebrow + h1 are real static text (matching + * `FairwayCoachHelmSignals.tsx`'s `CoachHelmShell` call for this route — + * default eyebrow "CoachHelm AI", `title={title ?? 'Signals'}` and this route + * never passes a `title` override), not `` blocks — see + * `dashboard/intelligence/loading.tsx`'s doc comment for why this fallback, + * not the final render, is what was showing a blank/ghost title. */ export default function Loading() { return ( @@ -23,7 +30,14 @@ export default function Loading() { {/* title row */}

- +
+

+ CoachHelm AI +

+

+ Signals +

+
diff --git a/src/app/golf/(dashboard)/dashboard/analytics/coachhelm/loading.tsx b/src/app/golf/(dashboard)/dashboard/analytics/coachhelm/loading.tsx index 45b1ca7ff..6bee91d27 100644 --- a/src/app/golf/(dashboard)/dashboard/analytics/coachhelm/loading.tsx +++ b/src/app/golf/(dashboard)/dashboard/analytics/coachhelm/loading.tsx @@ -12,6 +12,12 @@ import { fairwayScope } from '@/lib/redesign/flag'; * the legacy `glass-standard` header + `Shimmer`/`ShimmerCard` tab UI, which * matched neither the redesigned cockpit layout nor its tokens (CLS + a * wrong-chrome flash on mount). + * + * #947 fix: eyebrow + h1 are real static text (matching + * `FairwayEffectiveness.tsx`'s `CoachHelmShell` call — default eyebrow + * "CoachHelm AI", `title="Is CoachHelm helping?"`), not `` blocks. + * The description (`Last ${days} days…`) stays a Skeleton — it's the one + * piece that varies with the selected date range. */ export default function Loading() { return ( @@ -20,8 +26,12 @@ export default function Loading() { {/* Masthead — ViewHeader silhouette (eyebrow + title + description + actions) */}
- - +

+ CoachHelm AI +

+

+ Is CoachHelm helping? +

{/* range Segmented + Refresh action cluster */} diff --git a/src/app/golf/(dashboard)/dashboard/coachhelm/chat/page.tsx b/src/app/golf/(dashboard)/dashboard/coachhelm/chat/page.tsx index 7c91629db..83258cf93 100644 --- a/src/app/golf/(dashboard)/dashboard/coachhelm/chat/page.tsx +++ b/src/app/golf/(dashboard)/dashboard/coachhelm/chat/page.tsx @@ -7,14 +7,26 @@ */ import { redirect } from 'next/navigation'; +import type { Metadata } from 'next'; import { createClient } from '@/lib/supabase/server'; import { getGolfSessionProfile } from '@/lib/auth/session'; import { listConversations, listMessages } from '@/lib/coachhelm/v3/chat/persistence'; import { getAlertCounts } from '@/app/golf/actions/alerts'; import { fairwayScope } from '@/lib/redesign/flag'; import { AskWorkspace, InlineNotice, Button } from '@/components/fairway'; +import { surfaceName } from '@/lib/golf/surface-registry'; import Link from 'next/link'; +// #948 (4) — this route inherited the `dashboard/layout.tsx` default title +// ("Dashboard | GolfHelm") because it never set its own `metadata`, unlike +// its coachhelm-tab siblings (Brief/Signals/Effectiveness — see +// `intelligence/page.tsx` / `analytics/coachhelm/page.tsx`). Sourced from +// surface-registry.ts, matching #936/#917's fix for the Players tab. +export const metadata: Metadata = { + title: `${surfaceName('ask')} | CoachHelm`, + description: 'Ask CoachHelm anything about your team in natural language — full conversation history.', +}; + interface PageProps { searchParams: Promise<{ c?: string }>; } diff --git a/src/app/golf/(dashboard)/dashboard/intelligence/loading.tsx b/src/app/golf/(dashboard)/dashboard/intelligence/loading.tsx index 22cb85ed5..eb706e512 100644 --- a/src/app/golf/(dashboard)/dashboard/intelligence/loading.tsx +++ b/src/app/golf/(dashboard)/dashboard/intelligence/loading.tsx @@ -12,6 +12,17 @@ import { Skeleton } from '@/components/fairway/feedback/Skeleton'; * collapsed "Deep analysis" disclosure bar — same pattern as * `dashboard/insights/loading.tsx` and `FairwayBrief.tsx`'s own * `DeepAnalysisSkeleton` (rendered later, under the disclosure). + * + * #947 fix: the eyebrow + h1 are real static text (matching + * `FairwayBrief.tsx`'s `CoachHelmShell` call — `eyebrow` default "CoachHelm + * AI", `title={shell?.title ?? 'Team Brief'}`), not `` blocks. This + * route is `force-dynamic` and awaits several sequential DB reads before it + * can render at all, so this fallback is what actually paints first on every + * navigation here — a generic gray bar in the title's place, live for as + * long as the fetch chain takes, read as a "ghost/blank title" flash when the + * real text finally popped in. Neither string above depends on fetched data, + * so there's nothing to reserve a placeholder for; only the description + * (data-flavored copy) stays a Skeleton. */ export default function IntelligenceLoading() { return ( @@ -26,9 +37,13 @@ export default function IntelligenceLoading() { {/* masthead */}
-
- - +
+

+ CoachHelm AI +

+

+ Team Brief +

diff --git a/src/app/golf/(dashboard)/dashboard/stats/team/loading.tsx b/src/app/golf/(dashboard)/dashboard/stats/team/loading.tsx index e51c9cce5..1dced067f 100644 --- a/src/app/golf/(dashboard)/dashboard/stats/team/loading.tsx +++ b/src/app/golf/(dashboard)/dashboard/stats/team/loading.tsx @@ -10,6 +10,11 @@ import { fairwayScope } from '@/lib/redesign/flag'; * from the token-correct Fairway Skeleton primitive (bg-surface-sunken / * rounded-card / border-border-subtle) — so the skeleton→content handoff is a * quiet fade, not a layout jump. + * + * #947 fix: eyebrow + h1 are real static text (matching + * `FairwayTeamStats.tsx`'s ``), not `` blocks. The description (`${teamName} · + * ${count} players`) stays a Skeleton — it needs the fetched roster. */ export default function TeamStatsLoading() { return ( @@ -25,8 +30,12 @@ export default function TeamStatsLoading() { {/* ── MASTHEAD: ViewHeader — eyebrow · title · description · primary action ── */}
- - +

+ Team Stats +

+

+ Team Stats +

diff --git a/src/app/golf/actions/__tests__/dashboard-data.test.ts b/src/app/golf/actions/__tests__/dashboard-data.test.ts index f9d226f21..037bf7d8f 100644 --- a/src/app/golf/actions/__tests__/dashboard-data.test.ts +++ b/src/app/golf/actions/__tests__/dashboard-data.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { computeScoringTrendFromRounds } from '@/lib/golf/scoring-trend'; // --------------------------------------------------------------------------- // Mock Supabase client @@ -294,6 +295,73 @@ describe('dashboard-data server actions', () => { expect(roundsChain.range).toHaveBeenCalledWith(0, 999); expect(roundsChain.range).toHaveBeenCalledWith(1000, 1999); }); + + // ────────────────────────────────────────────────────────────────────── + // Team Pulse parity (#945) — before this fix, Team Pulse reimplemented + // its own split-half-of-5 classifier and disagreed with the Players + // roster / Team Stats trajectory tile (both powered by the canonical + // `computeScoringTrendFromRounds`) for the SAME underlying rounds. This + // fixture feeds two players' round histories through BOTH paths — the + // dashboard action and the canonical function directly — and asserts + // they land on the identical verdict. + // ────────────────────────────────────────────────────────────────────── + it('Team Pulse classifies per-player trend via the canonical computeScoringTrendFromRounds — parity with the roster/Team Stats classifier', async () => { + // Rising: 5 recent rounds well below the 3 previous rounds (improving). + const risingRounds = [ + { id: 'r1', player_id: 'p1', total_score: 68, score_to_par: -4, round_date: '2026-07-10', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 'r2', player_id: 'p1', total_score: 69, score_to_par: -3, round_date: '2026-07-08', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 'r3', player_id: 'p1', total_score: 70, score_to_par: -2, round_date: '2026-07-06', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 'r4', player_id: 'p1', total_score: 71, score_to_par: -1, round_date: '2026-07-04', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 'r5', player_id: 'p1', total_score: 70, score_to_par: -2, round_date: '2026-07-02', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 'r6', player_id: 'p1', total_score: 80, score_to_par: 8, round_date: '2026-06-20', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 'r7', player_id: 'p1', total_score: 82, score_to_par: 10, round_date: '2026-06-18', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 'r8', player_id: 'p1', total_score: 81, score_to_par: 9, round_date: '2026-06-16', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + ]; + // Falling: 5 recent rounds well above the 3 previous rounds (declining). + const fallingRounds = [ + { id: 's1', player_id: 'p2', total_score: 82, score_to_par: 10, round_date: '2026-07-10', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 's2', player_id: 'p2', total_score: 83, score_to_par: 11, round_date: '2026-07-08', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 's3', player_id: 'p2', total_score: 81, score_to_par: 9, round_date: '2026-07-06', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 's4', player_id: 'p2', total_score: 84, score_to_par: 12, round_date: '2026-07-04', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 's5', player_id: 'p2', total_score: 80, score_to_par: 8, round_date: '2026-07-02', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 's6', player_id: 'p2', total_score: 70, score_to_par: -2, round_date: '2026-06-20', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 's7', player_id: 'p2', total_score: 71, score_to_par: -1, round_date: '2026-06-18', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + { id: 's8', player_id: 'p2', total_score: 69, score_to_par: -3, round_date: '2026-06-16', holes_played: 18, total_putts: null, total_gir: null, total_gir_possible: null }, + ]; + const roundsChain = createChainableMock({ data: [...risingRounds, ...fallingRounds] }); + mockFrom.mockImplementation((table: string) => { + if (table === 'golf_rounds') return roundsChain; + if (table === 'golf_team_members') { + return createChainableMock({ + data: [ + { player: { id: 'p1', first_name: 'Rising', last_name: 'Star', avatar_url: null } }, + { player: { id: 'p2', first_name: 'Falling', last_name: 'Behind', avatar_url: null } }, + ], + }); + } + if (table === 'golf_teams') { + return createChainableMock({ + singleData: { id: 'team-1', name: 'Eagles', season: '2026', join_code: 'E1', created_at: '2026-01-01' }, + }); + } + return createChainableMock(); + }); + + // Parity oracle: the SAME canonical function the Players roster table + // and Team Stats page route through, fed the SAME per-player fixtures. + const risingVerdict = computeScoringTrendFromRounds(risingRounds); + const fallingVerdict = computeScoringTrendFromRounds(fallingRounds); + expect(risingVerdict.hasSignal && risingVerdict.trend).toBe('improving'); + expect(fallingVerdict.hasSignal && fallingVerdict.trend).toBe('declining'); + + const result = await getCoachDashboardData('coach-1', 'user-1', 'team-1'); + + expect(result.teamPulse.improving).toBe(1); + expect(result.teamPulse.declining).toBe(1); + expect(result.teamPulse.stable).toBe(0); + expect(result.teamPulse.topMover?.name).toBe('Rising Star'); + expect(result.teamPulse.topMover?.delta).toBeCloseTo(-risingVerdict.delta, 1); + }); }); // ======================================================================== diff --git a/src/app/golf/actions/dashboard-data.ts b/src/app/golf/actions/dashboard-data.ts index fdf6da09b..e8c32d44e 100644 --- a/src/app/golf/actions/dashboard-data.ts +++ b/src/app/golf/actions/dashboard-data.ts @@ -4,6 +4,7 @@ import { createClient } from '@/lib/supabase/server'; import { fetchAllRowsResult } from '@/lib/supabase/fetch-all-rows'; import { getTodayRangeForTz } from '@/lib/utils/timezone'; import { withAdminObserved } from '@/lib/admin/observed-action'; +import { computeScoringTrendFromRounds } from '@/lib/golf/scoring-trend'; // ============================================================================ // TYPES @@ -615,41 +616,38 @@ async function getCoachDashboardDataImpl( }, }; - // Team pulse — per-player trend using normalized 18-hole equivalents - // (aligned with stats page algorithm: 3+ rounds, split-half, 1.0 threshold) - let bestDelta = 0; + // Team pulse — per-player trend via the SAME canonical + // `computeScoringTrendFromRounds` (5-vs-5 window, ≥3-previous-sample + // floor, 0.3-stroke threshold, 18-hole normalization) the Players + // roster table (development/page.tsx) and Team Stats trajectory tile + // (stats/team/page.tsx, FairwayTeamStats.tsx) route through (#914). + // Previously this reimplemented its OWN split-half-of-5 classifier + // with no "previous window" floor, landing on a DIFFERENT + // improving/stable/declining headcount than the two canonical + // surfaces for the identical underlying rounds (#945). + let bestImprovementDelta = 0; let bestMoverName = ''; players.forEach(p => { const pRounds = roundsByPlayer.get(p.id) ?? []; - // Normalize to 18-hole equivalents (matches stats page) - const pNormalized = pRounds - .map(r => { - if (r.total_score === null) return null; - const hp = (r as { holes_played?: number | null }).holes_played ?? 18; - return Math.round(r.total_score * (18 / hp)); - }) - .filter((s): s is number => s !== null); - if (pNormalized.length < 3) return; // Need 3+ rounds for trend - const trend = computeTrend(pNormalized); - if (trend === 'improving') teamPulse.improving++; - else if (trend === 'declining') teamPulse.declining++; + const trendResult = computeScoringTrendFromRounds(pRounds); + if (!trendResult.hasSignal) return; // not enough rounds for a real verdict yet + + if (trendResult.trend === 'improving') teamPulse.improving++; + else if (trendResult.trend === 'declining') teamPulse.declining++; else teamPulse.stable++; - // Top mover - if (pNormalized.length >= 3) { - const recent5 = pNormalized.slice(0, 5); - const mid = Math.floor(recent5.length / 2); - const recentAvg = recent5.slice(0, mid).reduce((a, b) => a + b, 0) / mid; - const olderAvg = recent5.slice(mid).reduce((a, b) => a + b, 0) / (recent5.length - mid); - const delta = olderAvg - recentAvg; // positive = improvement - if (delta > bestDelta) { - bestDelta = delta; - bestMoverName = `${p.first_name || ''} ${p.last_name || ''}`.trim(); - } + // Top mover — the player with the biggest improvement. The + // canonical delta is recentAvg − previousAvg (lower is better, so + // NEGATIVE = improved); flip the sign to a positive "improvement + // magnitude", matching what FairwayCoachDashboard renders. + const improvementDelta = -trendResult.delta; + if (improvementDelta > bestImprovementDelta) { + bestImprovementDelta = improvementDelta; + bestMoverName = `${p.first_name || ''} ${p.last_name || ''}`.trim(); } }); - if (bestMoverName && bestDelta > 0) { - teamPulse.topMover = { name: bestMoverName, delta: Number(bestDelta.toFixed(1)) }; + if (bestMoverName && bestImprovementDelta > 0) { + teamPulse.topMover = { name: bestMoverName, delta: Number(bestImprovementDelta.toFixed(1)) }; } } } diff --git a/src/app/golf/actions/team-category-insights.ts b/src/app/golf/actions/team-category-insights.ts index 40413b2ad..560f575ca 100644 --- a/src/app/golf/actions/team-category-insights.ts +++ b/src/app/golf/actions/team-category-insights.ts @@ -942,9 +942,18 @@ async function getTeamCategoryInsightsImpl( supabase, ); if (engineResult.ok) { + // Bug #943 — coach-voice the engine row's content using the SAME + // roster map already built above for `playerStats` (never a second + // lookup). `player_id -> display name`, mirroring the shape + // `insightsToSignalRows` takes on the Signals surface. + const playerNamesForVoice: Record = {}; + for (const [pid, info] of playerInfoMap) { + playerNamesForVoice[pid] = info.name; + } const engineByCategory = assembleBriefEngineInsights( engineResult.data, CATEGORIES.map((c) => ({ id: c.id, label: c.label })), + playerNamesForVoice, ); for (const cat of categories) { const engineInsight = engineByCategory.get(cat.id); diff --git a/src/components/auth/baseball-auth-shell.tsx b/src/components/auth/baseball-auth-shell.tsx index 60812dc3d..3a49dc411 100644 --- a/src/components/auth/baseball-auth-shell.tsx +++ b/src/components/auth/baseball-auth-shell.tsx @@ -223,8 +223,13 @@ export function BaseballAuthShell({ {children} + {/* #950 — index 1 (not 3): the header/card/footer entrance on a + conversion page (the demo gate) should settle fast. Every + Reveal here still runs on the shared STAGGER_STEP/DUR.ink + cadence (untouched — it's reused by 200+ other surfaces), + just fewer steps behind the header. */} {footer ? ( - + {footer} ) : null} @@ -319,7 +324,7 @@ export function AuthFooterLinks({ href="/" className="mt-2 inline-flex min-h-[44px] items-center gap-1 rounded-lg px-3 py-3 -my-3 text-sm text-warm-500 transition-colors hover:text-warm-700 active:bg-warm-100/50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[color:var(--focus-ring)] focus-visible:ring-offset-2" > - ← Back to HelmLabs + ← Back to Helm Sports Labs ) : null} diff --git a/src/components/fairway/cards-insight/InsightPanel.test.tsx b/src/components/fairway/cards-insight/InsightPanel.test.tsx new file mode 100644 index 000000000..c2042cd6d --- /dev/null +++ b/src/components/fairway/cards-insight/InsightPanel.test.tsx @@ -0,0 +1,36 @@ +// @vitest-environment jsdom +/** + * ============================================================================ + * InsightPanel — bottom-sheet width regression guard (bug #949 #5) + * ---------------------------------------------------------------------------- + * Both real callers (FairwayCoachHelmSignals, FairwayPlayerCoachHelm) force + * `mode="sheet"` unconditionally — including on wide desktop viewports, not + * just the narrow ones the sheet branch was designed for. The sheet's + * `side="bottom"` class is full-width by design (`inset-x-0`), matching every + * other `side="bottom"` Sheet in the app. InsightPanel used to ALSO apply a + * `w-[min(32rem,…)]` cap on top of that — left, right, AND width all pinned + * at once is an over-constrained CSS box that resolves by discarding `left` + * and flush-fitting to the right edge, corner-docking a narrow sliver of a + * panel instead of the roomy full-width sheet every sibling renders. This + * locks the fix: the sheet's rendered content never carries a fixed/arbitrary + * width class. + * ========================================================================== */ +import { render } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { InsightPanel } from './InsightPanel'; + +describe('InsightPanel — mode="sheet" never re-introduces a conflicting width cap', () => { + it('the rendered sheet content carries no arbitrary/fixed width class', () => { + render( + + Some signal body copy. + , + ); + const content = document.querySelector('[data-slot="sheet"]'); + expect(content).not.toBeNull(); + // No `w-[...]` arbitrary-value width utility, and no fixed `w-` class — + // the bottom sheet must stay full-width (`inset-x-0`), like every other + // side="bottom" Sheet in the app. + expect(content!.className).not.toMatch(/\bw-\[/); + }); +}); diff --git a/src/components/fairway/cards-insight/InsightPanel.tsx b/src/components/fairway/cards-insight/InsightPanel.tsx index 8478ddfb2..55678ba09 100644 --- a/src/components/fairway/cards-insight/InsightPanel.tsx +++ b/src/components/fairway/cards-insight/InsightPanel.tsx @@ -411,17 +411,33 @@ export const InsightPanel = forwardRef( {/* priority rail — the same tint vocabulary as the card's tint bar */} diff --git a/src/components/fairway/cards-insight/MetricCard.test.tsx b/src/components/fairway/cards-insight/MetricCard.test.tsx new file mode 100644 index 000000000..c049f645e --- /dev/null +++ b/src/components/fairway/cards-insight/MetricCard.test.tsx @@ -0,0 +1,55 @@ +// @vitest-environment jsdom +/** + * ============================================================================ + * MetricCard delta chip — "Flat" for a zero-value delta, not "— 0%" (#950) + * ---------------------------------------------------------------------------- + * Bug: a zero-change delta (e.g. GIR% unchanged over the last 5 rounds) + * rendered a Minus (—) icon next to a literal "0%" — visually "— 0%", an odd + * double-negative-looking read for "nothing changed". Fix: the neutral + * (delta.value === 0) branch renders the word "Flat" instead of pairing a + * dash glyph with a zero. + * ========================================================================== */ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { MetricCard } from './MetricCard'; + +describe('MetricCard delta chip', () => { + it('reads "Flat" (not "— 0%") when the delta is exactly zero', () => { + render( + , + ); + expect(screen.getByText('Flat')).toBeInTheDocument(); + expect(screen.queryByText('0%')).not.toBeInTheDocument(); + }); + + it('still renders the real signed value + icon for a genuine positive delta', () => { + render( + , + ); + expect(screen.queryByText('Flat')).not.toBeInTheDocument(); + expect(screen.getByText('4%')).toBeInTheDocument(); + }); + + it('still renders the real signed value + icon for a genuine negative delta', () => { + render( + , + ); + expect(screen.queryByText('Flat')).not.toBeInTheDocument(); + expect(screen.getByText('-2')).toBeInTheDocument(); + }); +}); diff --git a/src/components/fairway/cards-insight/MetricCard.tsx b/src/components/fairway/cards-insight/MetricCard.tsx index 50c027ecd..4f46232cc 100644 --- a/src/components/fairway/cards-insight/MetricCard.tsx +++ b/src/components/fairway/cards-insight/MetricCard.tsx @@ -339,8 +339,8 @@ function DeltaChip({ }) { const direction = resolveDirection(delta); const tone = deltaTone(direction, goodDirection); - const Icon = - direction === 'up' ? ArrowUpRight : direction === 'down' ? ArrowDownRight : Minus; + const isFlat = direction === 'neutral'; + const Icon = direction === 'up' ? ArrowUpRight : direction === 'down' ? ArrowDownRight : Minus; return ( @@ -355,15 +355,25 @@ function DeltaChip({ )} style={{ fontFeatureSettings: '"tnum" 1, "lnum" 1' }} > - - + {isFlat ? ( + // No real movement (delta.value === 0) — a Minus glyph next to a + // literal "0%" read as a stray "— 0%" (#950). Say "Flat" instead of + // pairing a dash icon with a zero value that isn't really a number + // worth reading. + Flat + ) : ( + <> + + + + )} {delta.label ? ( diff --git a/src/components/fairway/charts/Ribbon.test.tsx b/src/components/fairway/charts/Ribbon.test.tsx index dc91561c7..742b9513b 100644 --- a/src/components/fairway/charts/Ribbon.test.tsx +++ b/src/components/fairway/charts/Ribbon.test.tsx @@ -139,3 +139,128 @@ describe('Ribbon — goodDirection (score/lower-is-better trend coloring)', () = expect(getDeltaDirection()).toBe('flat'); }); }); + +/** + * Regression test for #946 (Team Brief hero, fix 1/5): the panel's shared + * eyebrow + header + readout bezel row squeezes the eyebrow/header into an + * overlapping mess when it renders inside a narrow grid column (viewport- + * width Tailwind breakpoints don't know the panel's actual rendered width). + * `readoutPlacement="below"` stacks the readout under the eyebrow/header as + * its own block instead of sharing a row with them — a structural guarantee + * against the collision, independent of container width. + */ +describe('Ribbon — readoutPlacement (#946 bezel collision fix)', () => { + const YARDAGE: RibbonPoint[] = [ + { x: '0-25y', y: -0.03 }, + { x: '275-300y', y: -0.52 }, + ]; + + it('default ("corner"): the readout renders INSIDE the shared bezel row (unchanged for existing callers)', () => { + render(); + const bezel = document.querySelector('[data-slot="instrument-bezel"]'); + const readout = document.querySelector('[data-slot="readout"]'); + expect(bezel).not.toBeNull(); + expect(readout).not.toBeNull(); + expect(bezel!.contains(readout)).toBe(true); + }); + + it('"below": the readout renders OUTSIDE the bezel row — it can never share a flex row with the eyebrow/header', () => { + render( + , + ); + const bezel = document.querySelector('[data-slot="instrument-bezel"]'); + const readout = document.querySelector('[data-slot="readout"]'); + expect(bezel).not.toBeNull(); + expect(readout).not.toBeNull(); + expect(bezel!.contains(readout)).toBe(false); + }); +}); + +/** + * Regression test for #946 (Team Brief hero, fix 2/5): the big value and the + * delta beneath it rendered with no distinguishing label ("−0.52" over an + * unlabeled "▼−0.55"), reading as two ambiguous near-duplicate numbers. + * `readoutLabels` resolves from the REAL (finite-filtered) first/last points + * so the caller can name both truthfully. + */ +describe('Ribbon — readoutLabels (#946 unlabeled delta fix)', () => { + const YARDAGE: RibbonPoint[] = [ + { x: '0-25y', y: -0.03 }, + { x: '275-300y', y: -0.52 }, + ]; + + it('labels the value with the resolved LAST point and the delta with the resolved FIRST point', () => { + render( + ({ + value: `Strokes vs par · ${last.x}`, + delta: `vs ${first.x}`, + })} + />, + ); + const readout = document.querySelector('[data-slot="readout"]'); + expect(readout!.textContent).toContain('Strokes vs par · 275-300y'); + const delta = document.querySelector('[data-slot="readout-delta"]'); + expect(delta!.textContent).toContain('vs 0-25y'); + }); + + it('without readoutLabels, the delta has no caption (unchanged default — no regression)', () => { + render(); + const readout = document.querySelector('[data-slot="readout"]'); + expect(readout!.textContent).toContain('Strokes vs par'); + expect(readout!.textContent).not.toContain(' · 275-300y'); + }); +}); + +/** + * Bug #949 #1 — "Score by round" reportedly drew in ~15% of its canvas with + * an 11-round series (a large empty plot to the right). The x-scale is + * INDEX-based (evenly spaced across `points.length`, never dependent on the + * `x` label values), so this locks BOTH halves of the contract: the traced + * path's own coordinates span (near) the full plot width regardless of round + * count, AND the rendered `` itself is pinned to fill its container via + * a CSS class (not just the `width="100%"` attribute, which a class always + * wins over) so nothing upstream can silently constrain it to a sliver. + */ +describe('Ribbon — the trace + its both fill the full plot width (bug #949 #1)', () => { + function elevenRounds(): RibbonPoint[] { + return Array.from({ length: 11 }, (_, i) => ({ x: `R${i + 1}`, y: 70 + i })); + } + + it('the is pinned block+full-width via a class, not just the width attribute', () => { + const { container } = render( + , + ); + const svg = container.querySelector('svg'); + expect(svg).not.toBeNull(); + expect(svg!.getAttribute('class')).toContain('w-full'); + expect(svg!.getAttribute('class')).toContain('block'); + }); + + it('an 11-point series traces from the left pad to the right pad of the 600-unit viewBox (no squeeze)', () => { + const { container } = render( + , + ); + const line = container.querySelector('path[stroke]'); + expect(line).not.toBeNull(); + const d = line!.getAttribute('d') ?? ''; + // First and last x-coordinates in the path — parse the two numbers + // following the leading "M" and the final "L" command. + const commands = d.trim().split(/\s+(?=[ML])/); + const firstX = Number(commands[0]?.replace(/^M\s*/, '').split(' ')[0]); + const lastX = Number(commands[commands.length - 1]?.replace(/^L\s*/, '').split(' ')[0]); + // VIEW_W is 600 with an 8px pad each side — the trace must span the vast + // majority of that (never collapse into ~15% ≈ 90px from the left pad). + expect(firstX).toBeCloseTo(8, 0); + expect(lastX).toBeGreaterThan(500); + }); +}); diff --git a/src/components/fairway/charts/Ribbon.tsx b/src/components/fairway/charts/Ribbon.tsx index 35a7c6113..0554a62b3 100644 --- a/src/components/fairway/charts/Ribbon.tsx +++ b/src/components/fairway/charts/Ribbon.tsx @@ -74,6 +74,32 @@ export interface RibbonProps { * Score-by-round trend read a −7.0 improvement as a warning-orange decline). */ goodDirection?: GoodDirection; + /** + * Where the last-value readout renders. `'corner'` (default, unchanged) uses + * the shared InstrumentPanel bezel's top-right slot — fine when the panel + * has the full row to itself. `'below'` stacks the readout under the + * eyebrow/header instead of beside them, for panels that render inside a + * narrow grid column where a side-by-side eyebrow + header + readout row can + * get squeezed into an overlapping mess regardless of viewport width (#946 + * — the Team Brief hero's yardage-map card, embedded in a ~0.9fr grid + * column, had "LAST 90 DAYS · TEAM" collide with the readout). Stacking is a + * structural guarantee — the three text blocks never share a row, so they + * can't overlap at any width. + */ + readoutPlacement?: 'corner' | 'below'; + /** + * Names what the last plotted point and its delta actually represent, + * computed from the REAL (finite-filtered) first/last points Ribbon + * resolves internally. Ribbon's delta is always `last.y - first.y`; for a + * genuine time series that reads naturally as "current vs a prior point", + * but for a non-time x-axis (e.g. FairwayBrief's yardage-band curve) an + * unlabeled delta invites the reader to assume a time trend when it's + * really "farthest plotted band vs the closest one" (#946 — the hero's + * "−0.52" value and the unlabeled "▼−0.55" delta beneath it read as two + * ambiguous near-duplicate numbers). Omit for the previous unlabeled + * behavior (unchanged for existing callers). + */ + readoutLabels?: (first: RibbonPoint, last: RibbonPoint) => { value?: React.ReactNode; delta?: string }; className?: string; } @@ -92,6 +118,8 @@ export function Ribbon({ minPoints = 2, awaiting = false, goodDirection = 'up', + readoutPlacement = 'corner', + readoutLabels, className, }: RibbonProps) { const reduced = useReducedMotion() ?? false; @@ -201,48 +229,59 @@ export function Ribbon({ : 'awaiting signal'), ); + // Resolved labels — from the REAL (finite-filtered) first/last points, so a + // caller can name what the value + delta actually represent (#946). + const resolvedLabels = first && last ? readoutLabels?.(first, last) : undefined; + + const readoutNode = !isAwaiting ? ( +
+ {last ? ( + { + // `fmt` (the caller's valueFormatter, e.g. FairwayBrief's + // fmtSG) may ALREADY prefix its own +/− sign. Strip any + // leading sign glyph before prepending ours, or a signed + // formatter double-signs ("▼ −+0.56" instead of "▼ −0.56"). + const magnitude = fmt(Math.abs(v)).replace(/^[+\-−]/, ''); + return `${v >= 0 ? '+' : '−'}${magnitude}`; + }, + } + : undefined + } + /> + ) : null} + {hasTable ? ( + setShowTable((v) => !v)} /> + ) : null} +
+ ) : undefined; + return ( - {last ? ( - { - // `fmt` (the caller's valueFormatter, e.g. FairwayBrief's - // fmtSG) may ALREADY prefix its own +/− sign. Strip any - // leading sign glyph before prepending ours, or a signed - // formatter double-signs ("▼ −+0.56" instead of "▼ −0.56"). - const magnitude = fmt(Math.abs(v)).replace(/^[+\-−]/, ''); - return `${v >= 0 ? '+' : '−'}${magnitude}`; - }, - } - : undefined - } - /> - ) : null} - {hasTable ? ( - setShowTable((v) => !v)} /> - ) : null} -
- ) : undefined - } + readout={readoutPlacement === 'corner' ? readoutNode : undefined} className={className} > + {/* 'below' placement: the readout stacks under the eyebrow/header as its + own block, never sharing a flex row with them — a structural + guarantee against the collision described in #946. */} + {readoutPlacement === 'below' && readoutNode ? ( +
{readoutNode}
+ ) : null} {showTable && hasTable ? ( ) : isAwaiting ? ( @@ -272,6 +311,15 @@ export function Ribbon({ width="100%" height={height} preserveAspectRatio="none" + // P949 #1: the SVG's rendered width relied ONLY on the `width="100%"` + // presentation attribute — correct per the CSS/SVG2 spec (percentages + // resolve against the containing block, same as `w-full` on the wrapper + // above), but a CSS class wins over a plain attribute in every engine + // and is the more defensive way to pin it: `block w-full` guarantees the + // trace always fills its instrument's width to match the x-domain (11 + // rounds spanning the FULL plot, not squeezed into a corner) regardless + // of how the SVG is embedded by a future caller. + className="block w-full" aria-hidden > diff --git a/src/components/fairway/charts/StandingStrip.test.tsx b/src/components/fairway/charts/StandingStrip.test.tsx new file mode 100644 index 000000000..e5bb97c85 --- /dev/null +++ b/src/components/fairway/charts/StandingStrip.test.tsx @@ -0,0 +1,84 @@ +// @vitest-environment jsdom +/** + * ============================================================================ + * StandingStrip — SG "Field Avg 0.00" redundant column (bug #949 #7) + * ---------------------------------------------------------------------------- + * SG metrics are computed AGAINST the field average — its reference value is + * DEFINITIONALLY 0 on every player, every team, every render. The old 3-up + * readout row always rendered a "Field Avg" column reading "0.00" for SG + * metrics, which was also a straight duplicate of the SAME "FIELD AVG" text + * already labeling the reference tick on the bar above (a triple label: tick + * label + readout label + a value that never varies). This locks the fix: + * the redundant readout column is dropped for SG metrics; the tick (a real + * visual anchor) and every non-SG metric's genuine PGA/LPGA readout stay. + * ========================================================================== */ +import { render, screen } from '@testing-library/react'; +import { describe, it, expect } from 'vitest'; +import { StandingStrip } from './StandingStrip'; +import type { StandingStripProps } from './StandingStrip'; + +const sgProps: StandingStripProps = { + metric_id: 'sg_total', + metric_label: 'SG: Total', + player_value: 0.5, + team_avg: 0.2, + team_n: 8, + team_pct: 62, + pga_value: 0, + direction: 'higher_better', + unit: 'strokes', + scale: { min: -2, max: 2 }, + size: 'card', +}; + +const nonSgProps: StandingStripProps = { + metric_id: 'gir_pct', + metric_label: 'GIR %', + player_value: 62, + team_avg: 58, + team_n: 8, + team_pct: 70, + pga_value: 68, + direction: 'higher_better', + unit: 'percent', + scale: { min: 0, max: 100 }, + size: 'card', +}; + +describe('StandingStrip — SG metrics drop the redundant Field Avg readout', () => { + it('renders no "Field Avg" readout value for an sg_ metric (only the tick carries the label)', () => { + render(); + // The reference tick below the track still reads "FIELD AVG" (a real + // visual anchor showing where 0 sits) — exactly once. + expect(screen.getAllByText('FIELD AVG')).toHaveLength(1); + // The old 3rd readout's own "Field Avg" label (mixed-case DOM text — + // CSS `uppercase` only changes the rendering, not the text node — versus + // the tick's genuinely-uppercased "FIELD AVG" above) is gone entirely. + expect(screen.queryByText('Field Avg')).toBeNull(); + // ...and the constant, never-varying "0.00" value it always showed no + // longer appears at all. + expect(screen.queryByText('0.00')).toBeNull(); + }); + + it('still renders the real PGA readout value for a non-SG metric (genuine, informative number)', () => { + render(); + expect(screen.getByText('68%')).toBeInTheDocument(); + // The tick's "PGA" label appears once, plus the readout's "PGA" label — + // two occurrences is the intended (non-duplicate) pair: one on the track, + // one titling the actual number. + expect(screen.getAllByText('PGA').length).toBeGreaterThanOrEqual(1); + }); + + it('sg metric card renders a 2-up readout row (You / Team), not 3-up', () => { + const { container } = render(); + const readoutRow = container.querySelector('[data-slot="standing-strip"] .grid.gap-2'); + expect(readoutRow?.className).toContain('grid-cols-2'); + expect(readoutRow?.className).not.toContain('grid-cols-3'); + }); + + it('non-sg metric card keeps the 3-up readout row', () => { + const { container } = render(); + const readoutRow = container.querySelector('[data-slot="standing-strip"] .grid.gap-2'); + expect(readoutRow?.className).toContain('grid-cols-3'); + }); +}); diff --git a/src/components/fairway/charts/StandingStrip.tsx b/src/components/fairway/charts/StandingStrip.tsx index 5d210bb02..3587069d5 100644 --- a/src/components/fairway/charts/StandingStrip.tsx +++ b/src/components/fairway/charts/StandingStrip.tsx @@ -73,6 +73,14 @@ export function StandingStrip(props: StandingStripProps) { // CF-3: SG metrics anchor to the field average (0), not a PGA Tour score. // Women's teams get "LPGA" instead of "PGA" for non-SG metrics. const refLabel = pgaReferenceLabel(props.metric_id, props.is_womens).short; + // Bug #949 #7: an SG metric's reference value is DEFINITIONALLY 0 (the + // field average IS the zero point SG is computed against) — a "Field Avg + // 0.00" readout column can never say anything else, on every player, every + // team, every time. It was also a straight duplicate of the SAME "FIELD + // AVG" text already labeling the reference tick on the bar above. Suppress + // the redundant readout for SG metrics only — non-SG metrics still anchor + // to a genuine, informative PGA/LPGA Tour number in that third column. + const isFieldAvgRef = /^sg_/.test(props.metric_id); // ONE "behind-benchmark" hue: 'bad' reads as the neutral/amber system tone // (fw-warning) everywhere a strip renders — not red — so it never collides @@ -121,16 +129,31 @@ export function StandingStrip(props: StandingStripProps) { refLabel={refLabel.toUpperCase()} /> - {/* High-contrast 3-up readouts (You is the green hero figure) */} -
+ {/* High-contrast readouts (You is the green hero figure). Bug #949 #7: + an SG metric's reference column is dropped here — it's a constant + "Field Avg 0.00" (SG is computed AGAINST that zero point, so it can + never read anything else) that only duplicated the same "FIELD AVG" + text the tick above already carries. The tick stays (a useful + visual anchor); the redundant, unchanging number does not. Non-SG + metrics keep the real, informative PGA/LPGA readout. */} +
{showTeam && props.team_avg !== null ? ( - + ) : ( - + )} {/* F006: suppress the reference value too when omitted — render "—". */} - {props.pga_omitted ? ( + {isFieldAvgRef ? null : props.pga_omitted ? ( ) : ( diff --git a/src/components/fairway/charts/TrendChart.test.ts b/src/components/fairway/charts/TrendChart.test.ts new file mode 100644 index 000000000..468061a57 --- /dev/null +++ b/src/components/fairway/charts/TrendChart.test.ts @@ -0,0 +1,51 @@ +/** + * ============================================================================ + * computeTrendYDomain — fit the y-axis to the data, not Recharts' `[0,'auto']` + * ---------------------------------------------------------------------------- + * Bug (#950): the dashboard "Performance Trend" chart (a team scoring average + * living in the 74-77 range) rendered on a 0-80 y-axis — Recharts' own + * default `domain=[0,'auto']` floors every chart at zero, so a series that + * never approaches zero collapses to a near-flat line pinned to the top, + * ~85% dead space below it. `computeTrendYDomain` fits the axis to the + * series' own min/max instead, with proportional padding. + * ========================================================================== */ +import { describe, expect, it } from 'vitest'; +import { computeTrendYDomain } from './TrendChart'; + +describe('computeTrendYDomain', () => { + it('returns undefined for an empty series (caller falls back to auto)', () => { + expect(computeTrendYDomain([])).toBeUndefined(); + }); + + it('pads a tight, far-from-zero band instead of flooring at 0 (the #950 case)', () => { + const [min, max] = computeTrendYDomain([74, 75.5, 77, 76]) ?? []; + expect(min).toBeGreaterThan(0); + // The whole point: the axis floor sits close to the data, nowhere near 0. + expect(min).toBeGreaterThan(60); + expect(max).toBeLessThan(90); + // Both ends still fully contain the series. + expect(min).toBeLessThanOrEqual(74); + expect(max).toBeGreaterThanOrEqual(77); + }); + + it('gives a single-point (or perfectly flat) series visible headroom, not a zero-height line', () => { + const [min, max] = computeTrendYDomain([72, 72, 72]) ?? []; + expect(max).toBeGreaterThan(min!); + expect(min).toBeLessThan(72); + expect(max).toBeGreaterThan(72); + }); + + it('extends the domain to cover a benchmark reference line, even if it falls outside the series', () => { + const [min, max] = computeTrendYDomain([74, 75, 76], 50) ?? []; + expect(min).toBeLessThanOrEqual(50); + expect(max).toBeGreaterThanOrEqual(76); + }); + + it('ignores non-finite values (NaN/Infinity) rather than letting them blow up the domain', () => { + const domain = computeTrendYDomain([74, Number.NaN, 76]); + expect(domain).toBeDefined(); + const [min, max] = domain!; + expect(Number.isFinite(min)).toBe(true); + expect(Number.isFinite(max)).toBe(true); + }); +}); diff --git a/src/components/fairway/charts/TrendChart.tsx b/src/components/fairway/charts/TrendChart.tsx index e29f64dfe..6da34dd34 100644 --- a/src/components/fairway/charts/TrendChart.tsx +++ b/src/components/fairway/charts/TrendChart.tsx @@ -80,6 +80,34 @@ interface Row { y: number; } +/** + * Fits a y-axis domain to the DATA, not Recharts' own default `[0, 'auto']` + * — that default floors every chart at 0, so a series that lives entirely in + * a narrow band well above zero (e.g. a golf scoring average of 74-77) + * renders as a near-flat line pinned to the top of a 0-80 axis, ~85% dead + * space. Padding is proportional to the series' own spread, with a floor so + * a near-flat (or single-point) series still gets visible headroom above + * and below the line rather than collapsing to zero height. + * + * `benchmarkValue` (a dashed reference line, e.g. team average / PGA + * baseline) is folded into the min/max so it's never clipped off-chart. + * Returns `undefined` for an empty series — callers fall back to Recharts' + * own `['auto', 'auto']`. + */ +export function computeTrendYDomain( + values: readonly number[], + benchmarkValue?: number, +): [number, number] | undefined { + const finite = values.filter((v) => Number.isFinite(v)); + if (benchmarkValue !== undefined && Number.isFinite(benchmarkValue)) finite.push(benchmarkValue); + if (finite.length === 0) return undefined; + const min = Math.min(...finite); + const max = Math.max(...finite); + const span = max - min; + const pad = Math.max(span * 0.15, Math.max(Math.abs(max), 1) * 0.05); + return [min - pad, max + pad]; +} + export function TrendChart({ title, overline, @@ -102,6 +130,11 @@ export function TrendChart({ const rows: Row[] = React.useMemo(() => data.map((d) => ({ x: d.x, y: d.y })), [data]); const markers = React.useMemo(() => data.filter((d) => d.marker), [data]); + const yDomain = React.useMemo( + () => computeTrendYDomain(rows.map((r) => r.y), benchmark?.value), + [rows, benchmark], + ); + const resolvedState: ChartFrameState = state ?? (data.length === 0 ? 'empty' : 'ready'); const tableData: ChartTableData = { @@ -158,6 +191,7 @@ export function TrendChart({ width={40} tickFormatter={fmt} tickCount={5} + domain={yDomain ?? ['auto', 'auto']} /> {benchmark ? ( diff --git a/src/components/fairway/charts/TrendChip.tsx b/src/components/fairway/charts/TrendChip.tsx index 6c461706d..4e395c670 100644 --- a/src/components/fairway/charts/TrendChip.tsx +++ b/src/components/fairway/charts/TrendChip.tsx @@ -27,6 +27,12 @@ import * as React from 'react'; import { cn } from '@/lib/utils'; import { TABULAR_NUMS } from './theme'; +import { + TREND_ARROW as CANONICAL_TREND_ARROW, + TREND_TEXT_TONE as CANONICAL_TREND_TEXT_TONE, + TREND_LABEL as CANONICAL_TREND_LABEL, + type TrendVerdict as CanonicalTrendVerdict, +} from '@/lib/coachhelm/trend'; /* -------------------------------------------------------------------------- */ /* The ONE shared trend classifier (exported — others import this) */ @@ -212,3 +218,80 @@ export const TrendChip = React.forwardRef(funct ); }); + +/* -------------------------------------------------------------------------- */ +/* TrendGlyph — chrome-free rendering counterpart of the canonical */ +/* `@/lib/coachhelm/trend` verdict (#945) */ +/* -------------------------------------------------------------------------- */ + +export interface TrendGlyphProps { + /** + * The verdict from `classifyTrendDelta`/`computeSeriesTrend` + * (`@/lib/coachhelm/trend`). The arrow is ALWAYS the performance + * direction — up-ish for improving, down-ish for declining — never the raw + * metric's own sign. + */ + direction: CanonicalTrendVerdict; + /** + * Unsigned magnitude rendered after the verdict word (e.g. the "3.4" in + * "Declining 3.4"). Always WITHOUT a +/− sign — this glyph communicates + * direction via the arrow + verdict word; a raw SIGNED metric delta (e.g. + * a cockpit readout's "−7.0") is a different concern the caller renders + * itself. Ignored when `label` is passed, or when `direction` is + * `'stable'` (a flat trend has no magnitude to report). + */ + magnitude?: number; + /** Decimal places for `magnitude`. Defaults to 1. */ + magnitudeDigits?: number; + /** Verdict word override. Defaults to Improving / Steady / Declining. */ + label?: React.ReactNode; + className?: string; +} + +/** + * Chrome-free "arrow + verdict word [+ magnitude]" primitive — the rendering + * counterpart of the canonical `@/lib/coachhelm/trend` classifier used by the + * Players roster, Team Stats (trajectory + player cards), and Team Pulse. + * Unlike `TrendChip`, no background/padding/height is imposed, so it drops + * into a surface's OWN existing typography (a roster row, a player-card + * trend line) — unifying the SEMANTICS (which arrow means what) across + * surfaces without touching each surface's SKIN (size/typography stay + * whatever `className` supplies). + */ +export function TrendGlyph({ + direction, + magnitude, + magnitudeDigits = 1, + label, + className, +}: TrendGlyphProps) { + const magnitudeText = + magnitude != null && Number.isFinite(magnitude) && direction !== 'stable' + ? Math.abs(magnitude).toFixed(magnitudeDigits) + : null; + const text = + label ?? + (magnitudeText != null ? ( + <> + {CANONICAL_TREND_LABEL[direction]}{' '} + {magnitudeText} + + ) : ( + CANONICAL_TREND_LABEL[direction] + )); + + return ( + + + {text} + + ); +} diff --git a/src/components/fairway/charts/index.ts b/src/components/fairway/charts/index.ts index 2239a894b..15f9a431a 100644 --- a/src/components/fairway/charts/index.ts +++ b/src/components/fairway/charts/index.ts @@ -46,12 +46,14 @@ export { classifyTrendFromValues, TREND_COLOR, TREND_ARROW, + TrendGlyph, } from './TrendChip'; export type { TrendChipProps, TrendDirection, GoodDirection, ClassifyTrendOptions, + TrendGlyphProps, } from './TrendChip'; export { Sparkline } from './Sparkline'; diff --git a/src/components/fairway/controls/Toolbar.test.tsx b/src/components/fairway/controls/Toolbar.test.tsx new file mode 100644 index 000000000..e8f9377b6 --- /dev/null +++ b/src/components/fairway/controls/Toolbar.test.tsx @@ -0,0 +1,50 @@ +// @vitest-environment jsdom +/** + * ============================================================================ + * Toolbar — filters no longer compete with search for growth at desktop + * widths (bug #949 #8) + * ---------------------------------------------------------------------------- + * `search` and `filters` both carried `flex-1` (equal growth share), so a + * search field with room to spare (capped at `sm:max-w-sm`) could still pull + * flex-grow share away from the filters cluster, which has NO width floor of + * its own (`min-w-0` + `overflow-x-auto`, so it silently absorbs any deficit + * via its own scrollbar instead of ever forcing the row to wrap). At >=1280px + * this squeezed a 3-pill filter set (Severity/Status/Category) down far + * enough that the trailing pill clipped under the view-toggle segmented + * control, even though the row had plenty of total room. Pinning `search` to + * a fixed width from `lg` up (rather than letting it keep growing) hands all + * the desktop-tier leftover space to `filters` instead. + * ========================================================================== */ +import { render } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { Toolbar } from './Toolbar'; + +describe('Toolbar — search stops competing with filters for growth at lg+', () => { + it('the search wrapper is pinned (flex-none) from `lg` up, not still flex-1', () => { + const { container } = render( + } + filters={} + viewToggle={} + />, + ); + const searchWrapper = container.querySelector('input')?.parentElement; + expect(searchWrapper).not.toBeNull(); + expect(searchWrapper!.className).toContain('lg:flex-none'); + expect(searchWrapper!.className).toContain('lg:w-72'); + }); + + it('the filters wrapper stays the only flex-1 grower on the row (absorbs desktop leftover space)', () => { + const { container } = render( + } + filters={} + viewToggle={} + />, + ); + const filtersWrapper = container.querySelector('button')?.parentElement; + expect(filtersWrapper).not.toBeNull(); + expect(filtersWrapper!.className).toContain('flex-1'); + expect(filtersWrapper!.className).toContain('min-w-0'); + }); +}); diff --git a/src/components/fairway/controls/Toolbar.tsx b/src/components/fairway/controls/Toolbar.tsx index 08e7de2ba..fbbd5cc07 100644 --- a/src/components/fairway/controls/Toolbar.tsx +++ b/src/components/fairway/controls/Toolbar.tsx @@ -243,10 +243,29 @@ const ToolbarRoot = forwardRef(function Toolbar( transition={{ duration: reduceMotion ? 0 : 0.16 }} className="flex min-h-[44px] flex-wrap items-center gap-3 px-3 py-2" > - {/* search — grows to absorb slack so the row reads as one quiet field+controls */} - {search ?
{search}
: null} + {/* search — grows to absorb slack so the row reads as one quiet + field+controls. Bug #949 #8: below `lg` this still competes + equally (flex-1) with the filters cluster for space, which is + fine on mobile/tablet (the filters strip is a horizontal + scroller there anyway). From `lg` up, pin it to a fixed + comfortable width instead of growing — a search input never + NEEDS more than that, and letting it keep pulling flex-grow + share from `filters` was exactly what squeezed a 3-pill + filter set (Severity/Status/Category) down far enough that + the trailing "Status" pill clipped under the view-toggle + segmented control even at wide desktop widths (>=1280px), + where there was actually plenty of total room. */} + {search ? ( +
{search}
+ ) : null} - {/* filters — horizontally scrollable so a long set never breaks the row */} + {/* filters — horizontally scrollable so a long set never breaks + the row. From `lg` up it's now the ONLY flex-1 item on this + line (search stopped competing for the same growth share + above), so it claims all the room left over from search + + the trailing view-toggle/primary-action cluster — the 3-pill + set fits without ever needing its scroll fallback at desktop + widths. */} {filters ? (
{ + it('never lets a parenthetical suffix leak a punctuation initial (the #950 case)', () => { + expect(initialsFromName('Coach (Demo)')).toBe('CD'); + }); + + it('handles a plain two-word name', () => { + expect(initialsFromName('Nick Rini')).toBe('NR'); + }); + + it('takes the first two letters of a single-word name', () => { + expect(initialsFromName('Cher')).toBe('CH'); + }); + + it('keeps apostrophes and hyphens as legitimate name characters', () => { + expect(initialsFromName("Mary-Jane O'Brien")).toBe('MO'); + }); + + it('returns an empty string for a null/undefined/blank name', () => { + expect(initialsFromName(null)).toBe(''); + expect(initialsFromName(undefined)).toBe(''); + expect(initialsFromName(' ')).toBe(''); + }); +}); diff --git a/src/components/fairway/controls/avatar.tsx b/src/components/fairway/controls/avatar.tsx index 82e7d29e9..60da60a3b 100644 --- a/src/components/fairway/controls/avatar.tsx +++ b/src/components/fairway/controls/avatar.tsx @@ -68,9 +68,14 @@ const statusLabel: Record = { offline: 'Offline', }; -function initialsFromName(name?: string | null): string { +export function initialsFromName(name?: string | null): string { if (!name) return ''; - const parts = name.trim().split(/\s+/).filter(Boolean); + // Strip anything that isn't a letter/space/apostrophe/hyphen before + // tokenizing — a display name like "Coach (Demo)" otherwise splits to + // ["Coach", "(Demo)"], and the last token's first character is "(", + // producing "C(" instead of two real letters (#950). + const cleaned = name.replace(/[^\p{L}\s'-]+/gu, ' ').trim(); + const parts = cleaned.split(/\s+/).filter(Boolean); if (parts.length === 0) return ''; if (parts.length === 1) return parts[0]!.slice(0, 2).toUpperCase(); return (parts[0]![0]! + parts[parts.length - 1]![0]!).toUpperCase(); diff --git a/src/components/fairway/index.ts b/src/components/fairway/index.ts index 579fccc97..d808d439b 100644 --- a/src/components/fairway/index.ts +++ b/src/components/fairway/index.ts @@ -255,6 +255,11 @@ export { type TrendDirection, type GoodDirection, type ClassifyTrendOptions, + // TrendGlyph: chrome-free arrow+label rendering counterpart of the + // canonical `@/lib/coachhelm/trend` verdict (#945) — for surfaces that need + // the SAME arrow/color semantics as TrendChip without its pill chrome. + TrendGlyph, + type TrendGlyphProps, Sparkline, type SparklineProps, EkgSparkline, diff --git a/src/components/fairway/pages/coachhelm/AskConversationRail.tsx b/src/components/fairway/pages/coachhelm/AskConversationRail.tsx index 458e5b6f1..0ad8e3981 100644 --- a/src/components/fairway/pages/coachhelm/AskConversationRail.tsx +++ b/src/components/fairway/pages/coachhelm/AskConversationRail.tsx @@ -144,14 +144,19 @@ export function AskConversationRail({ }: AskConversationRailProps) { const reduced = useReducedMotion() ?? false; - // ── The rail bezel: a Fraunces "Threads" eyebrow + an honest thread-count - // micro-Readout (awaiting until at least one conversation exists). ──────── + // ── The rail bezel: a Fraunces "Threads" heading (THREADS_HEADER, the + // panel's `header` slot) + an honest thread-count micro-Readout in the + // `readout` slot (awaiting until at least one conversation exists). + // #948: the Readout used to carry its OWN `label="threads"` too, so the + // bezel showed "Threads" (the heading) directly above/beside "THREADS 2" + // (the readout's own uppercase label + count) — the same word rendered + // twice for one piece of information. The heading already says what's + // being counted; the readout needs only the bare number. ───────────── const count = conversations.length; const countReadout = ( 0 ? 'live' : 'awaiting'} diff --git a/src/components/fairway/pages/coachhelm/AskWorkspace.tsx b/src/components/fairway/pages/coachhelm/AskWorkspace.tsx index 98f283e59..bb006480c 100644 --- a/src/components/fairway/pages/coachhelm/AskWorkspace.tsx +++ b/src/components/fairway/pages/coachhelm/AskWorkspace.tsx @@ -194,11 +194,22 @@ export function AskWorkspace({ > {/* ── The two-pane inbox: a conversation rail + a thread pane, both flat matte InstrumentPanels on the canvas wash. Ask is a conversation, so - the chrome stays calm and the chat inside stays clean + legible. ── */} -
+ the chrome stays calm and the chat inside stays clean + legible. + #948: the rail used to be a proportional `col-span-3` of 12 — on a + laptop-width viewport (not an ultra-wide monitor) that resolved to + ~180px, truncating every thread title down to "Can you…". A grid + template column with a hard `minmax(260px, 300px)` floor keeps the + rail readable regardless of viewport width; the thread pane takes + the rest (`minmax(0, 1fr)` — the `0` avoids the classic grid-blowout + gotcha where an unbreakable child forces the track wider). ── */} +
{/* ── Left: conversation rail (sticky on desktop so the thread scrolls independently beside a pinned inbox). ──────────────────────────── */} -