fix: app-tab Rule-8 sweep — roster/stats/pipeline/scorecard/qualifier become real phone layouts#801
Conversation
… tables become real phone layouts The Bridge-sweep scouts found the same disease on the app side: min-w tables and unwrapped control rows forcing horizontal scroll at 390px on phone-primary surfaces. Each fix keeps md+ byte-identical: - Golf calendar hero: phone composition = grid nav row (prev/today/next) + full-width primary action in the thumb zone (was a single unwrapping 4-control row that overflowed the Surface at 390px). - Baseball roster wall (default surface): full-width PlayerRowPlate rows with AVG/OPS below md; the 5-column min-w-[680px] wall stays at md+. - Baseball stats center record book: 2-stat self-labeled card rows below md (AVG/OPS hitting, ERA/WHIP pitching), leaders computed per column set; ghost ready-rows get matching placeholders. - Baseball pipeline list view: card rows below lg mirroring the Academics split; desktop table unchanged except the list row's avatar no longer stacks above the name (Button single-child contract — real desktop render bug, wrapped in one flex child). - Golf round scorecard: real phone scorecard — per-hole strips (hole chip · par · tone-colored score + to-par pill · putts · FW/GIR marks) + nine-total rows; the min-w table stays md+. Qualifier round-by-round becomes rank/name/total card rows with per-round chips. - Baseball dev-plan goal filter tabs ride a contained edge-bled scroller below md (4 nowrap triggers exceeded 390px content width). Verification: every packet adversarially verified (all correct / correct-with-nits); tsc, eslint --max-warnings 0, vitest 975 tests green. Local production build hit ENOSPC (disk, not code) — CI build is the authoritative gate on this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
PR Summary by QodoRule-8 mobile layouts for calendar, roster/stats, pipeline, scorecard & qualifiers
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Summary by CodeRabbit
WalkthroughResponsive mobile layouts were added across baseball dashboards and Fairway pages. Desktop presentations remain available at larger breakpoints, while mobile views use horizontally scrollable tabs, cards, compact stat rows, phone-specific controls, qualifier cards, and per-hole scorecard strips. ChangesResponsive UI surfaces
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 8❌ Failed checks (1 warning, 7 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)ast-grep could not parse rule config: /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml 🔧 ESLint
src/app/baseball/(dashboard)/dashboard/dev-plan/DevPlanClient.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/app/baseball/(dashboard)/dashboard/pipeline/PipelineClient.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. src/app/baseball/(dashboard)/dashboard/roster/RosterFairway.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
Code Review by Qodo
Context used✅ Compliance rules (platform):
98 rules 1. Select-all set mismatch
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| <Button variant="ghost" size="sm" onClick={toggleSelectAll}> | ||
| {selectedPlayers.size === filteredWatchlist.length && filteredWatchlist.length > 0 ? 'Deselect all' : 'Select all'} | ||
| </Button> |
There was a problem hiding this comment.
2. Select-all set mismatch 🐞 Bug ≡ Correctness
PipelinePage’s new mobile select-all/deselect-all row decides “all selected” via selectedPlayers.size === filteredWatchlist.length, which can be true even when the selected IDs differ from the filtered IDs. This can show the wrong label and cause toggleSelectAll to clear or replace unrelated selections when filters change.
Agent Prompt
### Issue description
The new mobile select-all/deselect-all UI uses a cardinality comparison (`selectedPlayers.size === filteredWatchlist.length`) to determine whether all filtered players are selected. Because `selectedPlayers` persists independently of filtering, this can misclassify the state (same size, different elements) and the action can clear/overwrite selections that are not part of the current filtered list.
### Issue Context
- `selectedPlayers` is maintained as a Set and is not cleared when `filterTab`, `positionFilter`, or `gradYearFilter` changes.
- `filteredWatchlist` is recomputed from those filters.
- The mobile header label and the desktop header checkbox both use the same size-based check.
### Fix Focus Areas
- src/app/baseball/(dashboard)/dashboard/pipeline/PipelineClient.tsx[970-976]
- src/app/baseball/(dashboard)/dashboard/pipeline/PipelineClient.tsx[1333-1340]
### What to change
1. Compute an `allFilteredSelected` boolean via set inclusion, e.g.:
- `filteredWatchlist.length > 0 && filteredWatchlist.every(i => selectedPlayers.has(i.id))`
2. Update the mobile button label (and desktop checkbox checked state) to use `allFilteredSelected`.
3. Update `toggleSelectAll` to act on the filtered IDs set-wise:
- If `allFilteredSelected`, remove only the filtered IDs from `selectedPlayers` (don’t necessarily clear everything).
- Else, add missing filtered IDs to `selectedPlayers` (don’t necessarily replace the entire set).
This avoids wiping selections that are outside the current filtered subset.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| <span | ||
| aria-hidden="true" | ||
| className="grid h-8 w-8 shrink-0 place-items-center rounded-full bg-surface-sunken font-fw-mono text-body-sm font-semibold tabular-nums text-text-secondary" | ||
| > | ||
| {hole.hole_number} | ||
| </span> |
There was a problem hiding this comment.
3. Hole number hidden 🐞 Bug ≡ Correctness
ScorecardHoleRow renders the hole number with aria-hidden="true" and does not provide any other accessible hole identifier. Screen-reader users will hear the row’s stats but not which hole they correspond to.
Agent Prompt
### Issue description
In the new phone scorecard layout, the hole number is the primary identifier for each row, but it is explicitly hidden from assistive technologies (`aria-hidden="true"`). There is no alternative accessible label like “Hole 3”, so the row loses critical context for screen-reader navigation.
### Issue Context
`ScorecardHoleRow` is used to render the phone-only per-hole strips inside a `<ul>`. The only place the hole number is rendered is in the `aria-hidden` span.
### Fix Focus Areas
- src/components/fairway/pages/rounds/FairwayRoundDetail.tsx[733-770]
### What to change
Pick one of these approaches:
1. Remove `aria-hidden` from the hole number span so it is announced.
2. Keep the decorative styling but add an explicit accessible label, e.g.:
- Add `<span className="sr-only">Hole {hole.hole_number}</span>` at the start of the row, and/or
- Add `aria-label={`Hole ${hole.hole_number}`}` to the `<li>` (or `aria-labelledby` wiring).
Ensure the hole identifier is announced before the stat values.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/baseball/`(dashboard)/dashboard/dev-plan/DevPlanClient.tsx:
- Around line 562-570: Update the responsive tab scroller wrapper inside the
Goals Tabs section of DevPlanClient by adding sm:-mx-6 sm:px-6 alongside the
existing -mx-4 px-4 classes, preserving the md overrides. Apply the same
responsive padding adjustment to the related wrapper at the additionally
referenced location.
In `@src/app/baseball/`(dashboard)/dashboard/pipeline/PipelineClient.tsx:
- Around line 718-723: Replace the arbitrary min-h-[44px] classes on the Save,
Cancel, and controls referenced around the note actions in PipelineClient with
the existing shared Button/IconButton size variants or contract. Remove one-off
control height styling while preserving the intended mobile touch target and
layout.
- Around line 643-649: The PaperCard in the pipeline item is a clickable div
without keyboard semantics, conflicting with its nested controls. Remove
onClick={onFocus} from PaperCard and invoke onFocus from the relevant
interactive controls, or convert the container to an accessible interactive
element with Enter/Space handling and visible focus styling while preserving
nested-control behavior.
- Around line 1333-1339: Update toggleSelectAll in PipelineClient.tsx to scope
selection changes to filteredWatchlist: count selected IDs that are currently
visible, deselect only those visible IDs when all visible rows are selected,
otherwise add all visible IDs while preserving hidden selections. Update the
button label condition to use the visible selected count rather than
selectedPlayers.size.
- Around line 655-678: Update the document-level keyboard handler near the list
navigation logic to return early when the event target is inside a button, link,
element with role="button", or an editable control, in addition to the existing
input, textarea, and select checks. Use this guard before handling Enter/Space
so nested controls in the PipelineClient button and checkbox sections retain
their native keyboard activation.
In `@src/components/fairway/pages/calendar/FairwayCalendarHero.tsx`:
- Around line 178-183: The Today navigation buttons in FairwayCalendarHero are
incorrectly marked with aria-pressed, which is only for toggle controls. Remove
aria-pressed from both the desktop and mobile Today buttons, or replace it with
an appropriate accessible label such as “Today, current date.”
In `@src/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsx`:
- Around line 450-480: The phone totals in the qualifier row are unlabeled for
screen readers. In the mobile totals block near the player link, add visually
hidden “Total” and “To par” labels associated with the respective values, or
restructure the block as a labeled definition list while preserving the existing
visual layout and conditional score formatting.
- Around line 450-480: Replace the `rounded-fw-sm` classes in the player `Link`
and round-score `<span>` within the qualifier detail rendering with an approved
radius token: `rounded-2xl`, `rounded-xl`, or `rounded-lg`, consistently
following the path instructions.
In `@src/components/fairway/pages/rounds/FairwayRoundDetail.tsx`:
- Around line 628-640: Replace the non-approved rounded-fw-md and rounded-full
classes in the total display block and the corresponding section around the
related FairwayRoundDetail markup with approved rounded-2xl, rounded-xl, or
rounded-lg utilities, preserving the intended visual hierarchy and avoiding any
other radius tokens.
- Around line 756-807: The hole row currently hides its identity and exposes
score metadata only through visual abbreviations and indicators. In the
row-rendering JSX around the hole number, add an accessible “Hole N” label and
explicit accessible labels for the score, to-par value, fairway status, and GIR
status; update or augment the existing `HitMark` usage as needed so assistive
technology receives text rather than relying on visual dots, while preserving
the current visual layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 23a923b7-ee4a-461c-acdb-4a6c0292e895
📒 Files selected for processing (7)
src/app/baseball/(dashboard)/dashboard/dev-plan/DevPlanClient.tsxsrc/app/baseball/(dashboard)/dashboard/pipeline/PipelineClient.tsxsrc/app/baseball/(dashboard)/dashboard/roster/RosterFairway.tsxsrc/components/baseball/stats-center/StatsCenterClient.tsxsrc/components/fairway/pages/calendar/FairwayCalendarHero.tsxsrc/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsxsrc/components/fairway/pages/rounds/FairwayRoundDetail.tsx
| {/* Goals with Tabs. The four icon+label+badge triggers are | ||
| whitespace-nowrap and sum past the ~338px content width at | ||
| 390px, and TabsList has no scroll handling of its own — so | ||
| below md the list rides a contained edge-bled scroller | ||
| (matching the page's px-4 shell) instead of overflowing the | ||
| page. md+ is untouched (overflow-visible, no bleed). */} | ||
| <Tabs defaultValue="active" value={activeTab} onChange={setActiveTab}> | ||
| <TabsList className="mb-4"> | ||
| <div className="-mx-4 mb-4 overflow-x-auto px-4 [scrollbar-width:none] [&::-webkit-scrollbar]:hidden md:mx-0 md:overflow-visible md:px-0"> | ||
| <TabsList> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the tab scroller to the responsive page shell (DevPlanClient.tsx:569).
The page switches to sm:px-6, but this wrapper remains -mx-4 px-4 until md. At 640–767px, the tabs therefore do not align with the shell edge. Add sm:-mx-6 sm:px-6.
As per path instructions, mobile screens must use consistent page padding and spacing.
Also applies to: 584-584
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(dashboard)/dashboard/dev-plan/DevPlanClient.tsx around
lines 562 - 570, Update the responsive tab scroller wrapper inside the Goals
Tabs section of DevPlanClient by adding sm:-mx-6 sm:px-6 alongside the existing
-mx-4 px-4 classes, preserving the md overrides. Apply the same responsive
padding adjustment to the related wrapper at the additionally referenced
location.
Source: Path instructions
| <PaperCard | ||
| onClick={onFocus} | ||
| className={cn( | ||
| 'p-4', | ||
| pressableClass({ ink: 'pursuit', lift: true }), | ||
| focused && 'ring-2 ring-inset ring-pursuit', | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not make the card container a keyboardless clickable <div> (PipelineClient.tsx:643-649).
PaperCard defaults to a div, yet it receives onClick={onFocus} and has no keyboard semantics. Remove the wrapper click and invoke onFocus from the relevant controls, or provide an accessible interaction model that does not conflict with the nested controls.
As per path instructions, clickable non-button elements must support Enter/Space keyboard handling and visible focus behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(dashboard)/dashboard/pipeline/PipelineClient.tsx around
lines 643 - 649, The PaperCard in the pipeline item is a clickable div without
keyboard semantics, conflicting with its nested controls. Remove
onClick={onFocus} from PaperCard and invoke onFocus from the relevant
interactive controls, or convert the container to an accessible interactive
element with Enter/Space handling and visible focus styling while preserving
nested-control behavior.
Source: Path instructions
| <Button | ||
| variant="ghost" | ||
| onClick={onOpenPeek} | ||
| rightIcon={<IconChevronRight size={16} aria-hidden className="text-text-tertiary" />} | ||
| className="h-auto min-h-0 flex-1 justify-between gap-3 rounded-fw-sm px-2 py-1 text-left font-normal" | ||
| > | ||
| {/* Fairway's <Button> renders non-icon children inside a single bare | ||
| <span> (no className) as a direct child of its inline-flex | ||
| container — that span gets blockified, and an inline Avatar | ||
| followed by a block-level name/school div inside it triggers | ||
| anonymous-block-box generation (Avatar stacks above the name | ||
| instead of beside it). An explicit flex wrapper here establishes | ||
| its own flex formatting context so Avatar + text lay out as a | ||
| real identity row, matching the Avatar+name idiom used elsewhere | ||
| (MessagesClient.tsx, AcademicsClient.tsx) instead of relying on | ||
| the Button's auto-wrapping children slot. */} | ||
| <div className="flex min-w-0 flex-1 items-center gap-3"> | ||
| <Avatar src={item.player?.avatar_url} name={name} size="md" /> | ||
| <div className="min-w-0 flex-1"> | ||
| <span className="block truncate font-annual text-body-lg text-text-primary">{name}</span> | ||
| <span className="block truncate text-eyebrow text-text-tertiary">{item.player?.high_school_name || 'No school'}</span> | ||
| </div> | ||
| </div> | ||
| </Button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Let nested controls own Enter/Space (PipelineClient.tsx:655-678,738-750).
The document handler at PipelineClient.tsx:1028-1035 excludes only input, textarea, and select elements. It therefore intercepts Enter/Space on these new buttons and checkboxes, opening the peek panel or toggling row selection instead of activating the focused control. Ignore events inside button, a, [role="button"], and editable elements before handling list navigation.
As per path instructions, components handling user input must preserve normal keyboard activation for interactive controls.
Also applies to: 738-750
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(dashboard)/dashboard/pipeline/PipelineClient.tsx around
lines 655 - 678, Update the document-level keyboard handler near the list
navigation logic to return early when the event target is inside a button, link,
element with role="button", or an editable control, in addition to the existing
input, textarea, and select checks. Use this guard before handling Enter/Space
so nested controls in the PipelineClient button and checkbox sections retain
their native keyboard activation.
Source: Path instructions
| <Button size="sm" onClick={onSaveNote} className="min-h-[44px] flex-1"> | ||
| Save | ||
| </Button> | ||
| <Button variant="ghost" size="sm" onClick={onCancelNote} className="min-h-[44px] flex-1"> | ||
| Cancel | ||
| </Button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use shared touch-target sizes instead of arbitrary heights (PipelineClient.tsx:718-723,738-750).
Replace min-h-[44px] with the existing shared Button/IconButton size contract so mobile controls do not introduce one-off control heights.
As per path instructions, do not introduce one-off spacing, radius, icon sizes, or control heights.
Also applies to: 738-750
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(dashboard)/dashboard/pipeline/PipelineClient.tsx around
lines 718 - 723, Replace the arbitrary min-h-[44px] classes on the Save, Cancel,
and controls referenced around the note actions in PipelineClient with the
existing shared Button/IconButton size variants or contract. Remove one-off
control height styling while preserving the intended mobile touch target and
layout.
Source: Path instructions
| <div className="flex items-center justify-between gap-3 lg:hidden"> | ||
| <span className="font-annual text-eyebrow uppercase tracking-[0.14em] text-text-tertiary"> | ||
| {filteredWatchlist.length} player{filteredWatchlist.length !== 1 ? 's' : ''} | ||
| </span> | ||
| <Button variant="ghost" size="sm" onClick={toggleSelectAll}> | ||
| {selectedPlayers.size === filteredWatchlist.length && filteredWatchlist.length > 0 ? 'Deselect all' : 'Select all'} | ||
| </Button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope “select all” to visible filtered rows (PipelineClient.tsx:1333-1339).
toggleSelectAll() at PipelineClient.tsx:970-975 compares the global selection size with filteredWatchlist.length, then replaces the selection with only visible IDs. With selections retained across filters, the button can show the wrong action and silently drop hidden selections. Compare the visible selected count and add/remove only the currently filtered IDs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(dashboard)/dashboard/pipeline/PipelineClient.tsx around
lines 1333 - 1339, Update toggleSelectAll in PipelineClient.tsx to scope
selection changes to filteredWatchlist: count selected IDs that are currently
visible, deselect only those visible IDs when all visible rows are selected,
otherwise add all visible IDs while preserving hidden selections. Update the
button label condition to use the visible selected count rather than
selectedPlayers.size.
| <Button | ||
| variant={focusIsToday ? 'secondary' : 'ghost'} | ||
| size="sm" | ||
| className="justify-self-center" | ||
| onClick={() => onNavigate('today')} | ||
| aria-pressed={focusIsToday} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the Today button semantics in src/components/fairway/pages/calendar/FairwayCalendarHero.tsx:178-183.
aria-pressed describes a toggle button, but this control performs navigation. Remove it or use an accessible label such as “Today, current date”; apply the same correction to the desktop button at Line 140.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/fairway/pages/calendar/FairwayCalendarHero.tsx` around lines
178 - 183, The Today navigation buttons in FairwayCalendarHero are incorrectly
marked with aria-pressed, which is only for toggle controls. Remove aria-pressed
from both the desktop and mobile Today buttons, or replace it with an
appropriate accessible label such as “Today, current date.”
| <Link | ||
| href={`/golf/dashboard/stats?player=${playerId}`} | ||
| className="rounded-fw-sm underline-offset-2 outline-none hover:underline focus-visible:ring-2 focus-visible:ring-border-focus focus-visible:ring-offset-2 focus-visible:ring-offset-canvas" | ||
| className="min-w-0 flex-1 truncate rounded-fw-sm font-fw-sans text-body font-medium text-text-primary underline-offset-2 outline-none hover:underline focus-visible:ring-2 focus-visible:ring-border-focus focus-visible:ring-offset-2 focus-visible:ring-offset-canvas" | ||
| > | ||
| {data.playerName} | ||
| </Link> | ||
| </td> | ||
| {roundColumns.map((n) => { | ||
| const round = data.rounds.find((r) => r.roundNumber === n); | ||
| return ( | ||
| <td key={n} className="px-2 py-2.5 text-center"> | ||
| {round ? ( | ||
| <div className="flex flex-col items-center"> | ||
| <span className="font-fw-mono text-body-sm font-medium text-text-primary tabular-nums"> | ||
| {round.score ?? '—'} | ||
| </span> | ||
| <span className={cn('font-fw-mono text-caption tabular-nums', toParToneClass(round.toPar))}> | ||
| {formatToPar(round.toPar)} | ||
| </span> | ||
| </div> | ||
| ) : ( | ||
| <span className="text-caption text-text-tertiary">—</span> | ||
| )} | ||
| </td> | ||
| ); | ||
| })} | ||
| <td className="py-2.5 pl-3 text-right font-fw-mono font-medium text-text-primary tabular-nums"> | ||
| {hasRounds ? data.totalScore : '—'} | ||
| </td> | ||
| <td | ||
| </div> | ||
| <div className="shrink-0 text-right"> | ||
| <p className="font-fw-mono text-body font-semibold tabular-nums text-text-primary"> | ||
| {hasRounds ? data.totalScore : '—'} | ||
| </p> | ||
| <p | ||
| className={cn( | ||
| 'font-fw-mono text-caption tabular-nums', | ||
| hasRounds ? toParToneClass(data.totalToPar) : 'text-text-tertiary', | ||
| )} | ||
| > | ||
| {hasRounds ? formatToPar(data.totalToPar) : '—'} | ||
| </p> | ||
| </div> | ||
| </div> | ||
|
|
||
| {hasRounds ? ( | ||
| <div className="flex flex-wrap gap-1.5"> | ||
| {roundColumns.map((n) => { | ||
| const round = data.rounds.find((r) => r.roundNumber === n); | ||
| if (!round) return null; | ||
| return ( | ||
| <span | ||
| key={n} | ||
| className="inline-flex items-center gap-1 rounded-fw-sm bg-surface-sunken px-2 py-1 font-fw-mono text-caption tabular-nums" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add accessible labels to phone totals in src/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsx:450-480.
The desktop table has Total and To par headers, but the phone row exposes two unlabeled values to screen readers. Add visually hidden labels or render these values as a labeled definition list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsx` around
lines 450 - 480, The phone totals in the qualifier row are unlabeled for screen
readers. In the mobile totals block near the player link, add visually hidden
“Total” and “To par” labels associated with the respective values, or
restructure the block as a labeled definition list while preserving the existing
visual layout and conditional score formatting.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use approved radius tokens in src/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsx:450-480.
The new rounded-fw-sm utilities violate the path instruction permitting only rounded-2xl, rounded-xl, or rounded-lg. Replace them, or explicitly update the rule if rounded-fw-sm is the canonical Fairway token.
As per path instructions, “Reject inline hex values, ad-hoc spacing, or radius outside rounded-2xl/rounded-xl/rounded-lg.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/fairway/pages/qualifiers/FairwayQualifierDetail.tsx` around
lines 450 - 480, Replace the `rounded-fw-sm` classes in the player `Link` and
round-score `<span>` within the qualifier detail rendering with an approved
radius token: `rounded-2xl`, `rounded-xl`, or `rounded-lg`, consistently
following the path instructions.
Source: Path instructions
| <div className="mt-1 flex items-center justify-between rounded-fw-md bg-surface-tint px-3 py-2.5"> | ||
| <span className="font-fw-display text-caption font-medium uppercase tracking-[0.1em] text-text-tertiary"> | ||
| {label} total | ||
| </span> | ||
| <div className="flex items-baseline gap-3"> | ||
| <span className="font-fw-mono text-caption tabular-nums text-text-secondary"> | ||
| {puttTotal || '—'} putts | ||
| </span> | ||
| <span className="font-fw-mono text-h3 font-semibold tabular-nums text-text-primary"> | ||
| {scoreTotal || '—'} | ||
| </Td> | ||
| </tr> | ||
| {/* Putts */} | ||
| <tr> | ||
| <RowLabel>Putts</RowLabel> | ||
| {holes.map((h) => ( | ||
| <Td key={h.hole_number} className="text-text-secondary"> | ||
| {finite(h.putts) ?? '—'} | ||
| </Td> | ||
| ))} | ||
| <Td className="bg-surface-tint text-text-secondary">{puttTotal || '—'}</Td> | ||
| </tr> | ||
| {/* Fairway hit — par-3s have no fairway (fairway_hit NULL → blank dot) */} | ||
| <tr> | ||
| <RowLabel>FW</RowLabel> | ||
| {holes.map((h) => ( | ||
| <Td key={h.hole_number}> | ||
| <HitMark value={h.fairway_hit} /> | ||
| </Td> | ||
| ))} | ||
| <Td className="bg-surface-tint" /> | ||
| </tr> | ||
| {/* GIR */} | ||
| <tr> | ||
| <RowLabel>GIR</RowLabel> | ||
| {holes.map((h) => ( | ||
| <Td key={h.hole_number}> | ||
| <HitMark value={h.gir} /> | ||
| </span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use approved radius tokens in src/components/fairway/pages/rounds/FairwayRoundDetail.tsx:628-640,757-780.
The new rounded-fw-md and rounded-full utilities violate the path instruction permitting only rounded-2xl, rounded-xl, or rounded-lg. Replace them, or update the rule if these are intended canonical Fairway tokens.
As per path instructions, “Reject inline hex values, ad-hoc spacing, or radius outside rounded-2xl/rounded-xl/rounded-lg.”
Also applies to: 757-780
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/fairway/pages/rounds/FairwayRoundDetail.tsx` around lines 628
- 640, Replace the non-approved rounded-fw-md and rounded-full classes in the
total display block and the corresponding section around the related
FairwayRoundDetail markup with approved rounded-2xl, rounded-xl, or rounded-lg
utilities, preserving the intended visual hierarchy and avoiding any other
radius tokens.
Source: Path instructions
| return ( | ||
| <li className="flex items-center gap-2.5 py-2.5 first:pt-0 last:pb-0"> | ||
| <span | ||
| aria-hidden="true" | ||
| className="grid h-8 w-8 shrink-0 place-items-center rounded-full bg-surface-sunken font-fw-mono text-body-sm font-semibold tabular-nums text-text-secondary" | ||
| > | ||
| {hole.hole_number} | ||
| </span> | ||
| <div className="w-8 shrink-0 leading-tight"> | ||
| <p className="font-fw-display text-eyebrow font-medium uppercase tracking-[0.06em] text-text-tertiary"> | ||
| Par | ||
| </p> | ||
| <p className="font-fw-mono text-body-sm font-medium tabular-nums text-text-secondary"> | ||
| {p ?? '—'} | ||
| </p> | ||
| </div> | ||
| <div className="flex min-w-0 flex-1 items-center gap-2"> | ||
| <span className={cn('font-fw-mono text-h3 font-semibold tabular-nums', scoreTone)}> | ||
| {s ?? '—'} | ||
| </span> | ||
| <span | ||
| className={cn( | ||
| 'shrink-0 rounded-full px-1.5 py-0.5 font-fw-mono text-caption font-medium tabular-nums', | ||
| pillTone, | ||
| )} | ||
| > | ||
| {formatToPar(toPar)} | ||
| </span> | ||
| </div> | ||
| {/* Label-over-value, mirroring the Par block — one micro-pattern | ||
| across the row's small data blocks. */} | ||
| <div className="w-10 shrink-0 text-right leading-tight"> | ||
| <p className="font-fw-display text-eyebrow font-medium uppercase tracking-[0.06em] text-text-tertiary"> | ||
| Putts | ||
| </p> | ||
| <p className="font-fw-mono text-body-sm tabular-nums text-text-secondary"> | ||
| {finite(hole.putts) ?? '—'} | ||
| </p> | ||
| </div> | ||
| <div className="flex shrink-0 flex-col items-end gap-1 pl-1"> | ||
| <span className="inline-flex items-center gap-1.5"> | ||
| <span className="font-fw-display text-eyebrow font-medium uppercase tracking-[0.06em] text-text-tertiary"> | ||
| FW | ||
| </span> | ||
| <HitMark value={hole.fairway_hit} /> | ||
| </span> | ||
| <span className="inline-flex items-center gap-1.5"> | ||
| <span className="font-fw-display text-eyebrow font-medium uppercase tracking-[0.06em] text-text-tertiary"> | ||
| GIR | ||
| </span> | ||
| <HitMark value={hole.gir} /> | ||
| </span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep hole identity and score metadata accessible in src/components/fairway/pages/rounds/FairwayRoundDetail.tsx:756-807.
aria-hidden="true" removes the hole number, so assistive technology cannot associate the row’s values with a hole. Add an accessible “Hole N” label and explicit labels for score, to-par, fairway, and GIR instead of relying on visual abbreviations and dots.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/fairway/pages/rounds/FairwayRoundDetail.tsx` around lines 756
- 807, The hole row currently hides its identity and exposes score metadata only
through visual abbreviations and indicators. In the row-rendering JSX around the
hole number, add an accessible “Hole N” label and explicit accessible labels for
the score, to-par value, fairway status, and GIR status; update or augment the
existing `HitMark` usage as needed so assistive technology receives text rather
than relying on visual dots, while preserving the current visual layout.
Greptile SummaryThis PR implements Rule-8 mobile layouts across 6 phone-primary surfaces — dev-plan goal tabs, recruiting pipeline, roster wall, stats center, golf calendar hero, qualifier breakdown, and round scorecard — using the established
Confidence Score: 4/5Safe to merge — all changes are layout-only with no data, auth, or business-logic paths touched. Every changed file is pure UI: dual-render splits (phone card / desktop table), a contained tab scroller, and a new per-hole strip scorecard. The qualifier breakdown correctly pre-computes rank positions in a Map before the dual render, avoiding a shared mutable counter. Pipeline stage options flow from the canonical PIPELINE_STAGES array, so the card Select cannot write an invalid enum value. The one gap worth a follow-up: the round scorecard nine-total strip omits nine-to-par on phone even though parTotal is already computed. FairwayRoundDetail.tsx — the phone nine-total strip has parTotal in scope but doesn't expose nine-to-par. PipelineClient.tsx — confirm the bulk-action toolbar is outside the desktop-only block so multi-select works on phone. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Breakpoint["Responsive Split (Rule 8)"]
BP{Viewport}
BP -->|"< md / < lg"| PHONE[Phone card layout]
BP -->|"≥ md / ≥ lg"| DESKTOP[Desktop table — unchanged]
end
subgraph Baseball["Baseball surfaces"]
PHONE --> DPC["DevPlanClient\n— contained tab scroller"]
PHONE --> PLC["PipelineClient\n— PipelineListCard\n (Checkbox + Stage Select + Note)"]
PHONE --> RFC["RosterFairway\n— PlayerRowPlate\n AVG + OPS only"]
PHONE --> SCC["StatsCenterClient\n— StatSpread mobile view\n AVG+OPS / ERA+WHIP"]
end
subgraph Golf["Golf / Fairway surfaces"]
PHONE --> CAL["FairwayCalendarHero\n— grid-cols-3 nav + full-width CTA"]
PHONE --> QUAL["FairwayQualifierDetail\n— rank/name/total cards\n + per-round chips"]
PHONE --> RND["FairwayRoundDetail\n— ScorecardHoleRow strips\n (hole · par · pill · putts · FW/GIR)"]
end
PLC --> STAGES["PIPELINE_STAGES\n(5 canonical enum values)"]
QUAL --> POSMAP["positions Map\n(pre-computed rank\nbefore dual render)"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
subgraph Breakpoint["Responsive Split (Rule 8)"]
BP{Viewport}
BP -->|"< md / < lg"| PHONE[Phone card layout]
BP -->|"≥ md / ≥ lg"| DESKTOP[Desktop table — unchanged]
end
subgraph Baseball["Baseball surfaces"]
PHONE --> DPC["DevPlanClient\n— contained tab scroller"]
PHONE --> PLC["PipelineClient\n— PipelineListCard\n (Checkbox + Stage Select + Note)"]
PHONE --> RFC["RosterFairway\n— PlayerRowPlate\n AVG + OPS only"]
PHONE --> SCC["StatsCenterClient\n— StatSpread mobile view\n AVG+OPS / ERA+WHIP"]
end
subgraph Golf["Golf / Fairway surfaces"]
PHONE --> CAL["FairwayCalendarHero\n— grid-cols-3 nav + full-width CTA"]
PHONE --> QUAL["FairwayQualifierDetail\n— rank/name/total cards\n + per-round chips"]
PHONE --> RND["FairwayRoundDetail\n— ScorecardHoleRow strips\n (hole · par · pill · putts · FW/GIR)"]
end
PLC --> STAGES["PIPELINE_STAGES\n(5 canonical enum values)"]
QUAL --> POSMAP["positions Map\n(pre-computed rank\nbefore dual render)"]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <div className="mt-1 flex items-center justify-between rounded-fw-md bg-surface-tint px-3 py-2.5"> | ||
| <span className="font-fw-display text-caption font-medium uppercase tracking-[0.1em] text-text-tertiary"> | ||
| {label} total | ||
| </span> | ||
| <div className="flex items-baseline gap-3"> | ||
| <span className="font-fw-mono text-caption tabular-nums text-text-secondary"> | ||
| {puttTotal || '—'} putts | ||
| </span> | ||
| <span className="font-fw-mono text-h3 font-semibold tabular-nums text-text-primary"> | ||
| {scoreTotal || '—'} | ||
| </Td> | ||
| </tr> | ||
| {/* Putts */} | ||
| <tr> | ||
| <RowLabel>Putts</RowLabel> | ||
| {holes.map((h) => ( | ||
| <Td key={h.hole_number} className="text-text-secondary"> | ||
| {finite(h.putts) ?? '—'} | ||
| </Td> | ||
| ))} | ||
| <Td className="bg-surface-tint text-text-secondary">{puttTotal || '—'}</Td> | ||
| </tr> | ||
| {/* Fairway hit — par-3s have no fairway (fairway_hit NULL → blank dot) */} | ||
| <tr> | ||
| <RowLabel>FW</RowLabel> | ||
| {holes.map((h) => ( | ||
| <Td key={h.hole_number}> | ||
| <HitMark value={h.fairway_hit} /> | ||
| </Td> | ||
| ))} | ||
| <Td className="bg-surface-tint" /> | ||
| </tr> | ||
| {/* GIR */} | ||
| <tr> | ||
| <RowLabel>GIR</RowLabel> | ||
| {holes.map((h) => ( | ||
| <Td key={h.hole_number}> | ||
| <HitMark value={h.gir} /> | ||
| </span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
parTotal is already computed (line 604) and shown in the header (Par {parTotal}), but the nine-total chip only shows scoreTotal and puttTotal. On desktop a coach reads nine-to-par by comparing the Par row total with the Score row total side by side; on phone there's no equivalent — the only way to know the front/back nine was +2 is to mentally sum the per-hole chips. Adding a to-par pill next to scoreTotal (scoreTotal - parTotal when both are non-zero, using formatToPar + toParToneClass) would give coaches the same at-a-glance context and is a single-expression addition using helpers already in scope.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/fairway/pages/rounds/FairwayRoundDetail.tsx
Line: 628-640
Comment:
**Nine-total strip missing to-par on phone**
`parTotal` is already computed (line 604) and shown in the header (`Par {parTotal}`), but the nine-total chip only shows `scoreTotal` and `puttTotal`. On desktop a coach reads nine-to-par by comparing the Par row total with the Score row total side by side; on phone there's no equivalent — the only way to know the front/back nine was +2 is to mentally sum the per-hole chips. Adding a to-par pill next to `scoreTotal` (`scoreTotal - parTotal` when both are non-zero, using `formatToPar` + `toParToneClass`) would give coaches the same at-a-glance context and is a single-expression addition using helpers already in scope.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Problem
The Bridge mobile sweep's scouts (#800) found the same defect classes on the app side: min-w tables and unwrapped control rows forcing horizontal scroll at 390px on phone-primary surfaces (golf calendar hero, baseball roster wall / stats record book / pipeline list, golf round scorecard, qualifier round-by-round, dev-plan filter tabs).
Fix / Outcome
Every surface gets a hand-composed phone treatment below
md(orlgfor pipeline, mirroring Academics); desktop stays byte-identical:Verification
5 packets each fixer→adversarial-verify (all correct/correct-with-nits) + a re-scout of baseball player surfaces. tsc ✓ eslint ✓ vitest 975 ✓. Local prod build died on ENOSPC (disk, not code) — CI build is the gate here.
Git Activity Timeline note
App-side Rule-8 mobile sweep: 6 phone-primary surfaces get real phone layouts; 1 desktop render bug fixed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX