feat(coachhelm): consolidate player CoachHelm into one tabbed home - #299
feat(coachhelm): consolidate player CoachHelm into one tabbed home#299njrini99-code wants to merge 1 commit into
Conversation
The player CoachHelm was scattered across three standalone routes that all draw from the same engine — /coachhelm (Overview), /my-development, and /my-standing — each with its own masthead and its own sidebar rail item. A player had three "AI" entry points instead of one. Fold them into a single CoachHelm home using the existing CoachHelmShell + CoachHelmSubNav tab infrastructure (each tab is a real SSR route, not a client tab component): Overview · Development · Standing - CoachHelmSubNav: add the `standing` tab to the player tab set; rename the player "My Development" tab label to "Development". - CoachHelmShell: add `standing` to the CoachHelmTab union + DEFAULT_TITLE. - /my-standing: wrap the Fairway fork in CoachHelmShell (active="standing", role="player") so it shares the masthead + sub-nav instead of rendering a standalone header. Legacy (flag-off) branch unchanged. - Sidebars (GolfSidebar + FairwayDashboardShell): drop the duplicate standalone "My Development" player rail item — it's now a CoachHelm tab. Deep links from dashboard cards/round-review still land on the right tab. Coach-side consolidation follows separately. Gates: typecheck clean · lint 0 errors (2804 warns, under 6000) · 39 coachhelm tests pass incl. new CoachHelmSubNav.test.tsx (player tab set, active-prop paint, coach set intact). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 21 minutes and 22 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d32c6a78f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { name: 'CoachHelm AI', href: '/golf/dashboard/coachhelm', icon: IconSparkles }, | ||
| { name: 'My Rounds', href: '/golf/dashboard/rounds', icon: IconGolf }, |
There was a problem hiding this comment.
When NEXT_PUBLIC_REDESIGN is unset or false—the documented default—layout.tsx mounts the legacy GolfDashboardShell, while dashboard/coachhelm/page.tsx renders PlayerCoachHelmDashboard without the new tab navigation. Removing this link unconditionally therefore drops My Development from the player's normal navigation rather than consolidating it under CoachHelm; keep the entry for flag-off users and remove it only in the Fairway shell.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <div className="mx-auto w-full max-w-[860px] px-4 py-2 md:px-6"> | ||
| <CoachHelmShell | ||
| active="standing" | ||
| role="player" | ||
| eyebrow="My Standing" | ||
| title="Where you stack up" | ||
| description={ | ||
| `${isEmpty ? 'No standing data yet' : `${totalRows} metric${totalRows === 1 ? '' : 's'} tracked`} · you vs your team and PGA Tour, refreshed nightly.` | ||
| } | ||
| > |
There was a problem hiding this comment.
<div className="mx-auto w-full max-w-[860px] px-4 py-2 md:px-6"> wrapper was carried over from the old standalone page but was not removed when the content was wrapped in CoachHelmShell. The shell already provides its own px-4 md:px-6 on both the masthead and body containers, so the Standing tab ends up with 32 px of horizontal padding on each side (mobile) versus the 16 px that Overview and Development have — making it visually narrower and inconsistently padded compared to the other two tabs. Also remove the corresponding closing </div> on the line before </div> (the bg-canvas wrapper close).
| <div className="mx-auto w-full max-w-[860px] px-4 py-2 md:px-6"> | |
| <CoachHelmShell | |
| active="standing" | |
| role="player" | |
| eyebrow="My Standing" | |
| title="Where you stack up" | |
| description={ | |
| `${isEmpty ? 'No standing data yet' : `${totalRows} metric${totalRows === 1 ? '' : 's'} tracked`} · you vs your team and PGA Tour, refreshed nightly.` | |
| } | |
| > | |
| <CoachHelmShell | |
| active="standing" | |
| role="player" | |
| eyebrow="My Standing" | |
| title="Where you stack up" | |
| description={ | |
| `${isEmpty ? 'No standing data yet' : `${totalRows} metric${totalRows === 1 ? '' : 's'} tracked`} · you vs your team and PGA Tour, refreshed nightly.` | |
| } | |
| > |
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!
|
Superseded by #304, which rolls this onto a single branch (feat/coachhelm-stats-roundup) with all four PRs merged clean + a merge-interaction fix. Branch |
1 participant
Why
The player CoachHelm was scattered across three standalone routes that all draw from the same engine, each with its own masthead and its own sidebar rail item:
/golf/dashboard/coachhelm— Overview/golf/dashboard/my-development— focus areas + goals/golf/dashboard/my-standing— where-you-stack-up matrixA player had three "AI" entry points instead of one. ("CoachHelm is all over the place… data thrown everywhere.")
What
Fold them into a single CoachHelm home using the existing
CoachHelmShell+CoachHelmSubNavtab infrastructure (each tab is a real SSR route — not a client-side tab component, so deep links and SSR-active painting keep working):CoachHelmSubNav.tsxstandingto the player tab set; rename player "My Development" tab → "Development"CoachHelmShell.tsxstandingto theCoachHelmTabunion +DEFAULT_TITLErecordmy-standing/page.tsxCoachHelmShell(active="standing" role="player") so it shares the masthead + sub-nav. Legacy (flag-off) branch byte-for-byte unchangedGolfSidebar.tsx,FairwayDashboardShell.tsxCoachHelm AI(→ Overview) is the single rail entryDeep links from dashboard cards / round-review CTAs still resolve to the correct tab (Development / Standing) via the sub-nav's longest-prefix matcher.
Coach-side consolidation will follow in a separate PR.
Verification
tsc --noEmitcleannpm run lint— 0 errors (2804 warnings, under the 6000 gate)CoachHelmSubNav.test.tsx:active="standing"paintsaria-current="page"from the SSR propReviewer note
Hold for review of the core player experience before merge — this changes the player's primary navigation surface. Not merged.
🤖 Generated with Claude Code
Greptile Summary
This PR consolidates the player's three separate CoachHelm routes (Overview, My Development, My Standing) into a single tabbed home by wrapping each in the existing
CoachHelmShell+CoachHelmSubNavinfrastructure, and removes the now-redundant "My Development" rail item from both sidebars.CoachHelmSubNav.tsx/CoachHelmShell.tsx:standingis added to theCoachHelmTabunion and toPLAYER_TABS, giving players a three-tab nav (Overview · Development · Standing) while leaving the coach tab set intact.my-standing/page.tsx: The Fairway fork branch is wrapped inCoachHelmShell active="standing" role="player", sharing the masthead and sub-nav; the legacy branch is byte-for-byte unchanged.GolfSidebar.tsx/FairwayDashboardShell.tsx: The standalone "My Development" player rail item is removed from both nav registries.Confidence Score: 4/5
Safe to merge once the redundant padding wrapper in my-standing/page.tsx is removed — the layout bug is isolated to the Standing tab and does not affect auth, data loading, or the other two tabs.
The tab wiring, auth redirect, and nav removal are all correct. The one concrete defect is the leftover max-w-[860px] px-4 py-2 md:px-6 div in my-standing/page.tsx: because CoachHelmShell already applies its own px-4 md:px-6 on both the masthead and body containers, the Standing tab renders with double horizontal padding compared to Overview and Development, making it visibly narrower and inconsistent on every screen size.
src/app/golf/(dashboard)/dashboard/my-standing/page.tsx — the redundant outer wrapper div needs to be removed.
Important Files Changed
standingto CoachHelmTab union and adds the Standing tab to PLAYER_TABS with correct href/matchPrefixes; exact-match guard for the player Overview tab is preserved; coach tab set is unchanged.standing: 'Standing'to DEFAULT_TITLE; one-liner change, correct. Prop comment forroleis now slightly stale (still says "Brief + Players").Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Sidebar["GolfSidebar / FairwayDashboardShell\n(player rail)"] Sidebar -->|"CoachHelm AI\n/golf/dashboard/coachhelm"| Shell subgraph Shell["CoachHelmShell (role=player)"] SubNav["CoachHelmSubNav\nOverview · Development · Standing"] SubNav -->|"Overview\nhref=/golf/dashboard/coachhelm"| Overview["coachhelm/page.tsx"] SubNav -->|"Development\nhref=/golf/dashboard/my-development"| Dev["my-development/page.tsx"] SubNav -->|"Standing\nhref=/golf/dashboard/my-standing"| Standing["my-standing/page.tsx\n(new: CoachHelmShell wrapping)"] end style Standing fill:#fef3c7,stroke:#d97706Comments Outside Diff (1)
src/components/fairway/pages/coachhelm/CoachHelmShell.tsx, line 61 (link)roleprop JSDoc is now stale: the player variant has three tabs (Overview / Development / Standing), not "Brief + Players".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!
Reviews (1): Last reviewed commit: "feat(coachhelm): consolidate player Coac..." | Re-trigger Greptile