fix: Bridge 12-tab mobile sweep — every /admin tab hand-composed at 390px#800
Merged
Conversation
…90px Owner report (prod, iPhone): Users & Teams and Work log panned sideways with content bleeding through the bottom nav. Root causes were class-wide, so every Bridge tab was audited and fixed against docs/MOBILE_DOCTRINE.md (one fixer per tab, each adversarially verified): - Rule 8 card rows below md replace min-w tables: users roster (760px), teams/[id] roster, jobs cron board + integrity grid, deploys list, ben-leah issue table; TeamHealthTable now owns its own card/table split (golf + baseball pages heal automatically, baseball's local duplicate removed). - Page-level width-forcers closed: min-w-0 down every flex/grid ancestor chain, base-tier grid-cols-1 on px-minmax grids (work log), break-words on free-form text (PR titles, deploy refs, error dumps). - Monolith KPI cards -> StatStrip rhythm on work/auth/errors/tracer/ ben-leah; StatStrip gains MD_LAST_SPAN_RESET_3 so count=3 + mdColumns>=3 releases the trailing-cell span at md (fixes the empty-cell gap in the 768-1023 range). - Composed phone treatments: work-log timeline gutter slimmed below md, jobs triage-first grouping (attention rows surface, routine rows behind a disclosure, all-clear collapses), health feature-grid collapses green/no-data chips behind a per-group disclosure, deploys show-more cap, ben-leah intake form gets 16px inputs (no iOS zoom), 48px touch targets, and a Rule-5 fixed CTA bar scoped to the form's viewport presence. - Hydration-safe times: PanelAllClear + auth timestamps via LocalTime (server-UTC bake-in bug class). - Inset gains the documented href contract (mirrors Surface) for tap-through rows; Button JSDoc documents the single-child contract. Desktop (md+) is byte-identical everywhere except the documented StatStrip gap change on auth. Gates: tsc, eslint --max-warnings 0, vitest (320 admin + 892 fairway), production build — all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
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. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Summary by CodeRabbit
WalkthroughThis PR updates admin interfaces with responsive mobile cards, collapsible lists, shared KPI strips, localized timestamps, improved truncation, mobile form actions, and corrected app-only error coverage metrics. ChangesAdmin responsive UI
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 9❌ Failed checks (2 warnings, 7 inconclusive)
✅ Passed checks (3 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/admin/_components/FeatureDotGrid.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/app/admin/_components/LocalTime.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. src/app/admin/_components/PanelStates.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.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Greptile SummaryA pure mobile-responsiveness sweep across all 12 admin Bridge tabs: no data layer, auth, or RLS changes. Every fix follows the documented Mobile Doctrine \u2014 doctrine-8 card/table splits below
Confidence Score: 4/5Safe to merge — pure UI refactor with no data layer, auth, or migration changes; all 12 admin tab card/table splits are correctly fenced behind responsive breakpoints. The sweep is correctly executed and well-documented. Two inconsistencies are worth cleaning up: ShowMoreList imports the raw shadcn button rather than the fairway wrapper, and three new phone-card timestamp renders in jobs/page.tsx and TeamHealthTable still call toLocaleString() in server components, which the PR otherwise makes a point of fixing via LocalTime. src/app/admin/deploys/_components/ShowMoreList.tsx (button import) and src/app/admin/jobs/page.tsx / src/app/admin/_components/TeamHealthTable.tsx (server-component timestamp rendering in new phone-card views). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Phone["Below md (390px)"]
A[Card rows] --> B[Identity + key stats\nbreak-words, min-w-0]
B --> C[Tap-through link\nInset as=Link]
D[KPI row] --> E[StatStrip\n2-col + 1-wide trailing]
F[Long list] --> G[ShowMoreList\ncollapse after N rows]
end
subgraph Desktop["md and up"]
I[min-w table\noverflow-x-auto scoped\nsticky first col]
J[StatStrip mdColumns grid]
K[Full list]
end
Phone -- "hidden md:block swap" --> Desktop
%%{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 Phone["Below md (390px)"]
A[Card rows] --> B[Identity + key stats\nbreak-words, min-w-0]
B --> C[Tap-through link\nInset as=Link]
D[KPI row] --> E[StatStrip\n2-col + 1-wide trailing]
F[Long list] --> G[ShowMoreList\ncollapse after N rows]
end
subgraph Desktop["md and up"]
I[min-w table\noverflow-x-auto scoped\nsticky first col]
J[StatStrip mdColumns grid]
K[Full list]
end
Phone -- "hidden md:block swap" --> Desktop
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/app/admin/deploys/_components/ShowMoreList.tsx:4
**Wrong button import — bypasses the fairway design system**
`ShowMoreList` imports `Button` from `@/components/ui/button` (the raw shadcn/ui primitive) instead of `@/components/fairway`. Every other button reference in this PR — `FeatureDotGrid`, `BenLeahForm`, `CronBoardCards` — uses the fairway `Button`, which carries the Helm design token layer. The shadcn primitive renders with different defaults and will produce a visually inconsistent "Show N more deployments" control on the deploys tab compared to every other ghost-button on the Bridge.
```suggestion
import { Button } from '@/components/fairway';
```
### Issue 2 of 2
src/app/admin/jobs/page.tsx:70-73
**Timezone-unsafe timestamps in new server-component phone cards**
`CronJobCard` uses `new Date(row.lastRunAt).toLocaleString()` and `IntegrityCheckCard` uses `new Date(check.lastRunAt).toLocaleString()` inside server-component renders. These execute at request time on the Vercel runtime (UTC), baking UTC timestamps into the HTML — the same bug class this PR explicitly fixes in `PanelAllClear` and the auth page via `LocalTime`. An overdue-job diagnosis from a phone at 8 PM local time showing "Last run: 4/1/2026, 3:00:00 AM" is confusing when the server clock is UTC.
`TeamHealthTable`'s new phone card (`new Date(t.lastActivity).toLocaleDateString()`) has the same issue, though date-only is less operationally misleading near midnight. The three `toLocaleString()` / `toLocaleDateString()` calls in the newly added phone views should go through `LocalTime` to match the pattern established in this PR.
Reviews (1): Last reviewed commit: "fix: Bridge 12-tab mobile sweep — every ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+70
to
+73
| <Inset padding="sm" className={cn(isAlarm && 'ring-1 ring-fw-danger/30')}> | ||
| <div className="flex items-start justify-between gap-3"> | ||
| <p className="min-w-0 break-words font-fw-mono text-xs font-medium text-warm-900">{row.jobType}</p> | ||
| <StatusPill tone={CRON_STATUS_TONE[row.status]} dot size="sm" className="shrink-0"> |
There was a problem hiding this comment.
CronJobCard uses new Date(row.lastRunAt).toLocaleString() and IntegrityCheckCard uses new Date(check.lastRunAt).toLocaleString() inside server-component renders. These execute at request time on the Vercel runtime (UTC), baking UTC timestamps into the HTML — the same bug class this PR explicitly fixes in PanelAllClear and the auth page via LocalTime. An overdue-job diagnosis from a phone at 8 PM local time showing "Last run: 4/1/2026, 3:00:00 AM" is confusing when the server clock is UTC.
TeamHealthTable's new phone card (new Date(t.lastActivity).toLocaleDateString()) has the same issue, though date-only is less operationally misleading near midnight. The three toLocaleString() / toLocaleDateString() calls in the newly added phone views should go through LocalTime to match the pattern established in this PR.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/admin/jobs/page.tsx
Line: 70-73
Comment:
**Timezone-unsafe timestamps in new server-component phone cards**
`CronJobCard` uses `new Date(row.lastRunAt).toLocaleString()` and `IntegrityCheckCard` uses `new Date(check.lastRunAt).toLocaleString()` inside server-component renders. These execute at request time on the Vercel runtime (UTC), baking UTC timestamps into the HTML — the same bug class this PR explicitly fixes in `PanelAllClear` and the auth page via `LocalTime`. An overdue-job diagnosis from a phone at 8 PM local time showing "Last run: 4/1/2026, 3:00:00 AM" is confusing when the server clock is UTC.
`TeamHealthTable`'s new phone card (`new Date(t.lastActivity).toLocaleDateString()`) has the same issue, though date-only is less operationally misleading near midnight. The three `toLocaleString()` / `toLocaleDateString()` calls in the newly added phone views should go through `LocalTime` to match the pattern established in this PR.
How can I resolve this? If you propose a fix, please make it concise.
PR Summary by QodoFix Bridge /admin mobile layouts (cards, StatStrip, overflow fixes)
AI Description
Diagram
High-Level Assessment
Files changed (27)
|
…safe timestamps, 44px disclosures, sticky issue-table header - LocalTime gains a 'date' variant; jobs cron/integrity cards + desktop cells and TeamHealthTable's card/cell render viewer-local via LocalTime instead of baking the server's UTC clock into HTML (the same class this PR fixes in PanelAllClear). - BenLeahIssueTable (client) formats timestamps post-hydration via a private FormattedWhen leaf (custom format preserved) — kills the guaranteed server/client hydration mismatch. - The new phone-card <summary> disclosures get >=44px touch targets. - BenLeahIssueTable desktop header is sticky inside a bounded scroll container (max-h + overflow-auto, matching ImportWizardClient's working idiom — sticky top-0 is a no-op without one). Gates: tsc, eslint, 320 admin tests green in the worktree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
![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/admin/_components/FeatureDotGrid.tsx`:
- Around line 82-85: Replace all noncanonical accent-* color classes in
FeatureDotGrid with the appropriate canonical design-token families, using
primary-* for positive/accent states and destructive, warm-*, or cream-* where
semantically applicable. Update the affected class strings and conditional
styling around the identified button/grid sections, ensuring no accent-* tokens
remain.
- Around line 199-210: Update the CTA styling and label logic in the
FeatureDotGrid render block so an all-neutral group is not treated as healthy:
apply the success class and “All …” healthy wording only when counts.green > 0
&& counts.neutral === 0, while retaining the existing needsEyes handling for
other groups.
In `@src/app/admin/_components/TeamHealthTable.tsx`:
- Line 66: Update the non-null timestamp branches in TeamHealthTable’s
last-activity rendering to pass a non-semantic hydration fallback such as “—” or
“loading…” to LocalTime, while retaining “never” only for null lastActivity
branches.
- Around line 55-60: Replace the noncanonical accent color classes in the
TeamHealthTable leader treatment: update isLeader’s `bg-accent-50` and the
dateline span’s `bg-accent-500` to the appropriate approved `primary-*` or
`cream-*` design-system tokens, preserving the existing styling and behavior.
In `@src/app/admin/deploys/_components/ShowMoreList.tsx`:
- Around line 39-47: Add an aria-expanded attribute to the disclosure Button in
ShowMoreList, binding it to the expanded state so assistive technologies receive
the current expanded or collapsed status.
In `@src/app/admin/deploys/page.tsx`:
- Around line 153-156: Extract the shared sentryHref, commitSha, and deployHref
derivation into a reusable helper near the desktop row map and DeploymentCard,
then use that helper in both render paths. Remove the duplicated inline
calculations while preserving the existing link conditions and values.
In `@src/app/admin/errors/`[fingerprint]/page.tsx:
- Around line 69-74: Add keyboard accessibility to the stack-trace <pre> in the
error page: set tabIndex={0}, add an accessible label such as aria-label="Stack
trace", and include a visible focus style in its className alongside
overflow-auto so keyboard users can focus and scroll long traces.
In `@src/app/admin/teams/`[id]/RosterTable.tsx:
- Around line 87-90: Update the jersey-number rendering in the roster row markup
near the `r.name` display to use an explicit null check instead of truthiness,
so numeric value `0` renders as `#0`; keep `'no #'` only for null values and
align the behavior with the desktop rendering path.
In `@src/app/admin/users/page.tsx`:
- Around line 114-115: Preserve jersey number 0 in both roster layouts by
replacing truthiness checks in the mobile and desktop render blocks around
player.jerseyNumber with an explicit null/undefined check, so only missing
values render “no #” while 0 renders as “#0”.
In `@src/components/fairway/surfaces/surface.tsx`:
- Around line 220-223: Prevent the documented href prop from reaching the
default div in the Inset component: destructure href from the props and exclude
it from the generic spread, then conditionally pass it only when the resolved as
component is a non-div link-like element. Update the rendering logic around
Inset’s props handling and element creation near the existing spread.
🪄 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: ce7ce633-8fa3-477a-ae4c-cb9eee6b0553
📒 Files selected for processing (28)
src/app/admin/_components/FeatureDotGrid.tsxsrc/app/admin/_components/LocalTime.tsxsrc/app/admin/_components/PanelStates.tsxsrc/app/admin/_components/TeamHealthTable.tsxsrc/app/admin/auth/page.tsxsrc/app/admin/baseball/page.tsxsrc/app/admin/ben-leah/BenLeahForm.tsxsrc/app/admin/ben-leah/BenLeahIssueBoard.tsxsrc/app/admin/ben-leah/BenLeahIssueTable.tsxsrc/app/admin/ben-leah/BenLeahIssueWorkflowSelect.tsxsrc/app/admin/ben-leah/page.tsxsrc/app/admin/deploys/_components/ShowMoreList.tsxsrc/app/admin/deploys/page.tsxsrc/app/admin/errors/[fingerprint]/page.tsxsrc/app/admin/errors/page.tsxsrc/app/admin/golf/tracer/TracerPlayerList.tsxsrc/app/admin/golf/tracer/page.tsxsrc/app/admin/health/page.tsxsrc/app/admin/jobs/page.tsxsrc/app/admin/teams/[id]/RosterTable.tsxsrc/app/admin/teams/[id]/page.tsxsrc/app/admin/users/[id]/page.tsxsrc/app/admin/users/page.tsxsrc/app/admin/work/WorkTimeline.tsxsrc/app/admin/work/page.tsxsrc/components/fairway/charts/StatStrip.tsxsrc/components/fairway/controls/button.tsxsrc/components/fairway/surfaces/surface.tsx
Comment on lines
+82
to
85
| 'h-auto min-h-0 w-full rounded-xl border px-3 py-2 text-left normal-case', | ||
| richGreen | ||
| ? 'border-accent-500/35 bg-accent-50/60 hover:bg-accent-50' | ||
| : 'border-border-subtle bg-surface', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use the canonical design-token families.
src/app/admin/_components/FeatureDotGrid.tsx:82-85,159-179,202-203 introduces accent-* classes, while the repository rules require primary-*, destructive, warm-*, and cream-* tokens. Replace these with the corresponding canonical tokens.
[details]
- 'border-accent-500/35 bg-accent-50/60 hover:bg-accent-50'
+ 'border-primary-600/35 bg-cream-50/60 hover:bg-cream-50'[/details]
As per coding guidelines, only canonical design-system color families are permitted.
As per path instructions, src/**/*.{tsx,css} must use canonical design-system tokens.
Also applies to: 159-179, 202-203
🤖 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/admin/_components/FeatureDotGrid.tsx` around lines 82 - 85, Replace
all noncanonical accent-* color classes in FeatureDotGrid with the appropriate
canonical design-token families, using primary-* for positive/accent states and
destructive, warm-*, or cream-* where semantically applicable. Update the
affected class strings and conditional styling around the identified button/grid
sections, ensuring no accent-* tokens remain.
Sources: Coding guidelines, Path instructions
Comment on lines
+199
to
+210
| className={cn( | ||
| 'mt-2 justify-start whitespace-normal rounded-xl px-3 py-2 text-left text-xs font-medium md:hidden', | ||
| needsEyes === 0 && !showHealthy | ||
| ? 'bg-fw-success-bg text-accent-700 hover:bg-fw-success-bg/80' | ||
| : 'text-warm-500 hover:text-warm-700', | ||
| )} | ||
| > | ||
| {showHealthy | ||
| ? 'Hide healthy & no-data features' | ||
| : needsEyes === 0 | ||
| ? `All ${formatCollapsedSummary(counts)} — show list →` | ||
| : `Show ${formatCollapsedSummary(counts)} →`} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not style an all-neutral group as healthy.
At src/app/admin/_components/FeatureDotGrid.tsx:199-210, a group with 0 green and only neutral features satisfies needsEyes === 0, producing a success-colored CTA and the text All 0 healthy · N no data. Use the success treatment only when counts.green > 0 && counts.neutral === 0.
[details]
- needsEyes === 0 && !showHealthy
+ needsEyes === 0 && counts.green > 0 && counts.neutral === 0 && !showHealthy
...
- needsEyes === 0
+ needsEyes === 0 && counts.green > 0 && counts.neutral === 0[/details]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| className={cn( | |
| 'mt-2 justify-start whitespace-normal rounded-xl px-3 py-2 text-left text-xs font-medium md:hidden', | |
| needsEyes === 0 && !showHealthy | |
| ? 'bg-fw-success-bg text-accent-700 hover:bg-fw-success-bg/80' | |
| : 'text-warm-500 hover:text-warm-700', | |
| )} | |
| > | |
| {showHealthy | |
| ? 'Hide healthy & no-data features' | |
| : needsEyes === 0 | |
| ? `All ${formatCollapsedSummary(counts)} — show list →` | |
| : `Show ${formatCollapsedSummary(counts)} →`} | |
| className={cn( | |
| 'mt-2 justify-start whitespace-normal rounded-xl px-3 py-2 text-left text-xs font-medium md:hidden', | |
| needsEyes === 0 && counts.green > 0 && counts.neutral === 0 && !showHealthy | |
| ? 'bg-fw-success-bg text-accent-700 hover:bg-fw-success-bg/80' | |
| : 'text-warm-500 hover:text-warm-700', | |
| )} | |
| > | |
| {showHealthy | |
| ? 'Hide healthy & no-data features' | |
| : needsEyes === 0 && counts.green > 0 && counts.neutral === 0 | |
| ? `All ${formatCollapsedSummary(counts)} — show list →` | |
| : `Show ${formatCollapsedSummary(counts)} →`} |
🤖 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/admin/_components/FeatureDotGrid.tsx` around lines 199 - 210, Update
the CTA styling and label logic in the FeatureDotGrid render block so an
all-neutral group is not treated as healthy: apply the success class and “All …”
healthy wording only when counts.green > 0 && counts.neutral === 0, while
retaining the existing needsEyes handling for other groups.
Comment on lines
+55
to
+60
| 'block rounded-fw-md px-2 py-3 transition-colors hover:bg-surface-sunken', | ||
| isLeader && 'bg-accent-50', | ||
| )} | ||
| > | ||
| {/* Dateline rule — replaces the retired border-l-2 leader stripe. */} | ||
| {isLeader && <span aria-hidden className="mb-1 block h-[2px] w-7 rounded-full bg-accent-500" />} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use canonical tokens for the mobile leader treatment.
src/app/admin/_components/TeamHealthTable.tsx:55-60 adds bg-accent-50 and bg-accent-500. Use the approved primary-* or cream-* equivalents instead.
As per coding guidelines, only canonical design-system color families are permitted.
As per path instructions, src/**/*.{tsx,css} must use canonical design-system tokens.
🤖 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/admin/_components/TeamHealthTable.tsx` around lines 55 - 60, Replace
the noncanonical accent color classes in the TeamHealthTable leader treatment:
update isLeader’s `bg-accent-50` and the dateline span’s `bg-accent-500` to the
appropriate approved `primary-*` or `cream-*` design-system tokens, preserving
the existing styling and behavior.
Sources: Coding guidelines, Path instructions
| <p className="truncate text-sm font-medium text-warm-900">{t.name}</p> | ||
| <p className="mt-1 font-fw-mono text-xs tabular-nums text-warm-500"> | ||
| {t.playerCount} players · last{' '} | ||
| {t.lastActivity ? <LocalTime iso={t.lastActivity} variant="date" fallback="never" /> : 'never'} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a non-semantic hydration fallback for real timestamps.
LocalTime at src/app/admin/_components/LocalTime.tsx:30-53 renders fallback during SSR and the first client render. With fallback="never", every non-null timestamp initially appears as “last never”; reserve “never” for the existing null branches and use — or loading… here.
[details]
- <LocalTime iso={t.lastActivity} variant="date" fallback="never" />
+ <LocalTime iso={t.lastActivity} variant="date" fallback="—" />[/details]
Also applies to: 112-112
🤖 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/admin/_components/TeamHealthTable.tsx` at line 66, Update the
non-null timestamp branches in TeamHealthTable’s last-activity rendering to pass
a non-semantic hydration fallback such as “—” or “loading…” to LocalTime, while
retaining “never” only for null lastActivity branches.
Comment on lines
+39
to
+47
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| size="sm" | ||
| onClick={() => setExpanded((v) => !v)} | ||
| className="mt-2 w-full bg-surface-sunken text-warm-700 hover:text-accent-700" | ||
| > | ||
| {expanded ? 'Show less' : `Show ${moreCount} more ${itemLabel}${moreCount === 1 ? '' : 's'}`} | ||
| </Button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add aria-expanded to the disclosure toggle.
The button toggles visibility of more but doesn't expose its state to assistive tech. Screen-reader users have no way to know whether the list is currently expanded or collapsed.
♿ Proposed fix
<Button
type="button"
variant="ghost"
size="sm"
+ aria-expanded={expanded}
onClick={() => setExpanded((v) => !v)}
className="mt-2 w-full bg-surface-sunken text-warm-700 hover:text-accent-700"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="sm" | |
| onClick={() => setExpanded((v) => !v)} | |
| className="mt-2 w-full bg-surface-sunken text-warm-700 hover:text-accent-700" | |
| > | |
| {expanded ? 'Show less' : `Show ${moreCount} more ${itemLabel}${moreCount === 1 ? '' : 's'}`} | |
| </Button> | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="sm" | |
| aria-expanded={expanded} | |
| onClick={() => setExpanded((v) => !v)} | |
| className="mt-2 w-full bg-surface-sunken text-warm-700 hover:text-accent-700" | |
| > | |
| {expanded ? 'Show less' : `Show ${moreCount} more ${itemLabel}${moreCount === 1 ? '' : 's'}`} | |
| </Button> |
🤖 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/admin/deploys/_components/ShowMoreList.tsx` around lines 39 - 47, Add
an aria-expanded attribute to the disclosure Button in ShowMoreList, binding it
to the expanded state so assistive technologies receive the current expanded or
collapsed status.
Comment on lines
+153
to
+156
| {rows.map((d) => { | ||
| const sentryHref = sentryReleaseHref(d.commitSha); | ||
| const commitSha = d.commitSha; | ||
| const deployHref = d.url ? `https://${d.url}` : null; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicated link-derivation logic between the desktop row map and DeploymentCard.
sentryHref/commitSha/deployHref are computed identically in the desktop <tbody> map (Lines 153-156) and again in DeploymentCard (Lines 226-229). Any future change to this derivation (e.g., new link condition) has to be kept in sync by hand in two places.
♻️ Proposed fix
+function deploymentLinks(d: VercelDeployment) {
+ return {
+ sentryHref: sentryReleaseHref(d.commitSha),
+ commitSha: d.commitSha,
+ deployHref: d.url ? `https://${d.url}` : null,
+ };
+}
+
async function DeploymentsTable() {
...
- {rows.map((d) => {
- const sentryHref = sentryReleaseHref(d.commitSha);
- const commitSha = d.commitSha;
- const deployHref = d.url ? `https://${d.url}` : null;
+ {rows.map((d) => {
+ const { sentryHref, commitSha, deployHref } = deploymentLinks(d);
return ( function DeploymentCard({ d }: { d: VercelDeployment }) {
- const sentryHref = sentryReleaseHref(d.commitSha);
- const commitSha = d.commitSha;
- const deployHref = d.url ? `https://${d.url}` : null;
+ const { sentryHref, commitSha, deployHref } = deploymentLinks(d);Also applies to: 226-229
🤖 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/admin/deploys/page.tsx` around lines 153 - 156, Extract the shared
sentryHref, commitSha, and deployHref derivation into a reusable helper near the
desktop row map and DeploymentCard, then use that helper in both render paths.
Remove the duplicated inline calculations while preserving the existing link
conditions and values.
Comment on lines
+69
to
+74
| // Contained CODE block, never a page-level pan: w-full + min-w-0 | ||
| // keep it from ever donating its long-line width to an ancestor, | ||
| // overflow-auto gives it its own horizontal+vertical scroller | ||
| // instead (classic min-w offender otherwise — Mobile Doctrine | ||
| // rule 8 territory even though this isn't literally a table). | ||
| <pre className="mt-2 max-h-48 w-full min-w-0 overflow-auto rounded bg-warm-100 p-2 text-caption">{e.stack_trace}</pre> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the contained stack trace keyboard-scrollable.
At src/app/admin/errors/[fingerprint]/page.tsx:74, overflow-auto creates the only scroll container, but <pre> is not focusable. Add tabIndex={0}, an accessible label, and a visible focus style so keyboard users can inspect long traces.
[details]
- <pre className="mt-2 max-h-48 w-full min-w-0 overflow-auto rounded bg-warm-100 p-2 text-caption">
+ <pre
+ tabIndex={0}
+ aria-label="Stack trace"
+ className="mt-2 max-h-48 w-full min-w-0 overflow-auto rounded bg-warm-100 p-2 text-caption focus-visible:outline focus-visible:outline-2 focus-visible:outline-primary-600"
+ >[/details]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Contained CODE block, never a page-level pan: w-full + min-w-0 | |
| // keep it from ever donating its long-line width to an ancestor, | |
| // overflow-auto gives it its own horizontal+vertical scroller | |
| // instead (classic min-w offender otherwise — Mobile Doctrine | |
| // rule 8 territory even though this isn't literally a table). | |
| <pre className="mt-2 max-h-48 w-full min-w-0 overflow-auto rounded bg-warm-100 p-2 text-caption">{e.stack_trace}</pre> | |
| <pre | |
| tabIndex={0} | |
| aria-label="Stack trace" | |
| className="mt-2 max-h-48 w-full min-w-0 overflow-auto rounded bg-warm-100 p-2 text-caption focus-visible:outline focus-visible:outline-2 focus-visible:outline-primary-600" | |
| > |
🤖 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/admin/errors/`[fingerprint]/page.tsx around lines 69 - 74, Add
keyboard accessibility to the stack-trace <pre> in the error page: set
tabIndex={0}, add an accessible label such as aria-label="Stack trace", and
include a visible focus style in its className alongside overflow-auto so
keyboard users can focus and scroll long traces.
Comment on lines
+87
to
+90
| <p className={cn('truncate font-medium', isDormant ? 'text-warm-500' : 'text-warm-900')}>{r.name}</p> | ||
| <p className="mt-1 font-fw-mono text-xs tabular-nums text-warm-500"> | ||
| {r.jerseyNumber ? `#${r.jerseyNumber}` : 'no #'} ·{' '} | ||
| {r.lastRoundScore !== null ? ( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve jersey number 0.
src/app/admin/teams/[id]/RosterTable.tsx:89 treats 0 as missing, while the desktop path at Line 144 correctly preserves it with nullish handling. jerseyNumber is typed as number | null, so use an explicit null check for consistent mobile and desktop output.
Proposed fix
- {r.jerseyNumber ? `#${r.jerseyNumber}` : 'no #'} ·{' '}
+ {r.jerseyNumber !== null ? `#${r.jerseyNumber}` : 'no #'} ·{' '}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <p className={cn('truncate font-medium', isDormant ? 'text-warm-500' : 'text-warm-900')}>{r.name}</p> | |
| <p className="mt-1 font-fw-mono text-xs tabular-nums text-warm-500"> | |
| {r.jerseyNumber ? `#${r.jerseyNumber}` : 'no #'} ·{' '} | |
| {r.lastRoundScore !== null ? ( | |
| <p className={cn('truncate font-medium', isDormant ? 'text-warm-500' : 'text-warm-900')}>{r.name}</p> | |
| <p className="mt-1 font-fw-mono text-xs tabular-nums text-warm-500"> | |
| {r.jerseyNumber !== null ? `#${r.jerseyNumber}` : 'no #'} ·{' '} | |
| {r.lastRoundScore !== null ? ( |
🤖 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/admin/teams/`[id]/RosterTable.tsx around lines 87 - 90, Update the
jersey-number rendering in the roster row markup near the `r.name` display to
use an explicit null check instead of truthiness, so numeric value `0` renders
as `#0`; keep `'no #'` only for null values and align the behavior with the
desktop rendering path.
Comment on lines
+114
to
+115
| player.jerseyNumber ? `#${player.jerseyNumber}` : 'no #', | ||
| player.position, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve jersey number 0 in both roster layouts.
src/app/admin/users/page.tsx:114 and src/app/admin/users/page.tsx:203 use truthiness checks, so a valid jerseyNumber of 0 renders as no #. Since the field is numeric and both mobile and desktop share the same bug, use an explicit null check.
Proposed fix
- player.jerseyNumber ? `#${player.jerseyNumber}` : 'no #',
+ player.jerseyNumber !== null ? `#${player.jerseyNumber}` : 'no #',
- {player.jerseyNumber ? `#${player.jerseyNumber}` : 'no #'}
+ {player.jerseyNumber !== null ? `#${player.jerseyNumber}` : 'no #'}Also applies to: 202-204
🤖 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/admin/users/page.tsx` around lines 114 - 115, Preserve jersey number
0 in both roster layouts by replacing truthiness checks in the mobile and
desktop render blocks around player.jerseyNumber with an explicit null/undefined
check, so only missing values render “no #” while 0 renders as “#0”.
Comment on lines
+220
to
+223
| /** Destination when rendered as an anchor / Next `Link` via `as`. Ignored by | ||
| * the default `div`. Enables `<Inset as={Link} href=…>` tap-through rows — | ||
| * the same contract Surface documents above. */ | ||
| href?: string; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not forward href to the default <div>.
The prop is documented as ignored for the default element, but Line 244 still spreads it through. <Inset href="…"> therefore renders an invalid href attribute on a <div>. Destructure href and only pass it when as resolves to a non-div link-like component.
Proposed fix
export const Inset = forwardRef<HTMLDivElement, InsetProps>(function Inset(
- { padding = 'sm', as, className, children, ...props },
+ { padding = 'sm', as, href, className, children, ...props },
ref,
) {
const Comp = (as ?? 'div') as ElementType;
return (
<Comp
ref={ref}
data-slot="inset"
className={cn(
'bg-surface-sunken rounded-fw-md text-text-secondary',
PADDING[padding],
className,
)}
{...props}
+ {...(Comp === 'div' ? {} : { href })}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** Destination when rendered as an anchor / Next `Link` via `as`. Ignored by | |
| * the default `div`. Enables `<Inset as={Link} href=…>` tap-through rows — | |
| * the same contract Surface documents above. */ | |
| href?: string; | |
| export const Inset = forwardRef<HTMLDivElement, InsetProps>(function Inset( | |
| { padding = 'sm', as, href, className, children, ...props }, | |
| ref, | |
| ) { | |
| const Comp = (as ?? 'div') as ElementType; | |
| return ( | |
| <Comp | |
| ref={ref} | |
| data-slot="inset" | |
| className={cn( | |
| 'bg-surface-sunken rounded-fw-md text-text-secondary', | |
| PADDING[padding], | |
| className, | |
| )} | |
| {...props} | |
| {...(Comp === 'div' ? {} : { href })} | |
| > |
🤖 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/surfaces/surface.tsx` around lines 220 - 223, Prevent
the documented href prop from reaching the default div in the Inset component:
destructure href from the props and exclude it from the generic spread, then
conditionally pass it only when the resolved as component is a non-div link-like
element. Update the rendering logic around Inset’s props handling and element
creation near the existing spread.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Owner screenshots (prod, iPhone): Users & Teams and Work log weren't mobile — the page panned sideways (min-w tables with broken
min-w-0ancestor chains), a giant lone "1 OPEN" KPI card, text clipped mid-word, and rows bleeding through the bottom nav (that last one is the sharedbg-surface/95nav bug already fixed in #799 — the Bridge inherits it on merge).Fix / Outcome
All 12 Bridge tabs audited and fixed against
docs/MOBILE_DOCTRINE.md— one fixer per tab, each adversarially verified (4 packets needed repair rounds; final verdicts all correct/correct-with-nits):mdreplace min-w tables: users roster, teams/[id] roster, jobs cron board + integrity grid, deploys list, ben-leah issue table.TeamHealthTablenow owns its own card/table split (golf + baseball heal automatically; baseball's local duplicate removed).min-w-0down every flex/grid ancestor chain, base-tiergrid-cols-1on px-minmax grids,break-wordson free-form text (PR titles, deploy refs, error dumps).MD_LAST_SPAN_RESET_3(count=3 + mdColumns≥3 releases the trailing span atmd— fixes the 768–1023px empty-cell gap).LocalTime(server-UTC bake-in class).Insetgains the documentedhrefcontract (mirrorsSurface);Buttondocuments its single-child contract.Desktop (md+) byte-identical everywhere except the documented StatStrip gap change on auth.
Verification
tsc ✓ · eslint --max-warnings 0 ✓ · vitest 320 admin + 892 fairway ✓ · production build exit 0 ✓
Git Activity Timeline note
Bridge mobile sweep: 12 tabs card-ified/composed for 390px per Mobile Doctrine; shared StatStrip/TeamHealthTable/PanelStates upgrades.
🤖 Generated with Claude Code
https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX