feat(initiatives): add multi-track podium architecture and accessible track switching - #200
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
barunaniket
left a comment
barunaniket
left a comment
There was a problem hiding this comment.
Reviewed the full diff and ran it locally. Understood that the AlgoHunt winners are a deliberate scaffold — you've built the multi-track format so @planksconstant can fill in the real content under #177 — so I've left the placeholder data alone. That was a genuinely useful thing to do: the format question was the hard part, and having a worked example of the frontmatter shape makes the content work much easier to hand over.
The schema extension is exactly right: track and badge are both optional, so every existing single-track event still validates untouched. The dead-link fix on winner members is correct too, and the hooks are properly declared above the early return as you noted.
Verified locally (not just CI): vitest 72/72 across 8 suites, tsc --noEmit clean, eslint 0 errors, and I regenerated the manifest with buildInitiativesManifest() — byte-identical to the committed one. I also ran next dev and confirmed both podiums render with the track switcher.
One blocker and a few small things.
Blocker — the track tabs can't be reached by keyboard
See the inline comment on WinnersShowcase.tsx:82. tabIndex={isCurrent ? 0 : -1} is the roving-tabindex pattern, which requires an onKeyDown handler to move focus between tabs with the arrow keys. There isn't one anywhere in the file — I grepped for onKeyDown / ArrowRight / useRef / focus() and got nothing.
The effect is that the inactive tab is removed from the tab order and no keystroke can reach it, so a keyboard-only visitor cannot view the Tough Track podium at all. I confirmed this in the rendered HTML on next dev: one tab is tabindex="0", the other tabindex="-1", with no handler to bridge them.
Worth flagging because the description claims WCAG 2.1 AA and "Full keyboard navigability (Arrow / Tab / Enter / Space)" — as written this is a 2.1.1 Keyboard failure at Level A, and strictly worse than plain buttons would have been. The one-line fix (drop the tabIndex prop) restores full access; the fuller fix adds the arrow-key handler. Either is fine by me.
Minor ARIA issues
Two small correctness points on the tab semantics, both inline: aria-controls on the inactive tab points at an element id that is never rendered (only the active panel exists), and single-track events render an orphan role="tabpanel" with no owning tab. Neither breaks anything visually, but they're the kind of thing a screen reader reports as a broken relationship.
.gitignore
The *.pdf rule is unrelated to this feature — see inline. No harm today, but a repo-wide binary ignore is easy to add and hard to notice later.
Coordination note (not a defect)
Since #177 will rewrite algohunt.md, be aware initiatives.test.ts now asserts the literal track names "Easy Track" / "Tough Track" and winners.length >= 6 against the committed content. Whoever edits that file will need to update the test in the same PR or CI goes red — worth mentioning to @planksconstant so it doesn't surprise anyone against the 8 September deadline.
Happy to approve once the keyboard issue is sorted.
| id={tabId} | ||
| aria-selected={isCurrent} | ||
| aria-controls={panelId} | ||
| tabIndex={isCurrent ? 0 : -1} |
There was a problem hiding this comment.
This is the blocker. Roving tabindex takes the inactive tab out of the tab order, but there's no onKeyDown on the tablist to move focus with the arrow keys — so there is no way to reach the Tough Track tab without a mouse.
Simplest fix, fully accessible immediately:
| tabIndex={isCurrent ? 0 : -1} |
Just deleting the line leaves both tabs in the natural tab order, where Enter/Space already work because they're real <button> elements.
If you'd rather keep the proper APG pattern, add a handler on the role="tablist" container instead — ArrowRight/ArrowLeft to cycle selectedTrack, Home/End for first/last — and move focus to the newly-selected tab via a ref array. Both are correct; the first is one line.
| role="tab" | ||
| id={tabId} | ||
| aria-selected={isCurrent} | ||
| aria-controls={panelId} |
There was a problem hiding this comment.
aria-controls is set on every tab, but only the active panel is ever rendered (id={activePanelId} on the single motion.div below). So the inactive tab points at an id that doesn't exist in the DOM, which assistive tech reports as a dangling reference.
Either render a panel per track (hidden ones with hidden), or drop aria-controls — aria-labelledby on the panel already establishes the relationship in the direction that matters here.
| <AnimatePresence mode="wait"> | ||
| <motion.div | ||
| key={activeTrack || "all"} | ||
| role="tabpanel" |
There was a problem hiding this comment.
role="tabpanel" is applied unconditionally, but the tablist only renders when hasMultipleTracks is true. So every single-track event — currently every event except AlgoHunt — gets a tabpanel with no owning tab and no aria-labelledby, which is invalid on its own.
Gating the role on hasMultipleTracks (alongside the aria-labelledby you already gate) would cover it:
| role="tabpanel" | |
| role={hasMultipleTracks ? "tabpanel" : undefined} |
|
|
||
| # misc | ||
| *.pem | ||
There was a problem hiding this comment.
Unrelated to the podium feature — worth splitting into its own PR so it's findable later.
No harm right now (nothing tracked matches), but a repo-wide *.pdf will silently refuse a legitimate document later — a slide deck or a report someone deliberately wants committed — and that failure is confusing when it happens. If the aim is to stop accidental binary commits, scoping it to the directory where they keep landing would be tighter.
…e keyboard navigation - Implement WAI-ARIA roving tabindex keyboard navigation (ArrowRight, ArrowLeft, ArrowDown, ArrowUp, Home, End) - Drop dangling aria-controls references on unmounted tabpanels - Gate role=tabpanel so single-track events do not produce orphan panels - Decouple multi-track unit test assertions from hardcoded content strings to protect Issue #177 - Revert unrelated *.pdf rule from .gitignore
9894281 to
eaf9f7b
Compare
|
Thanks for the thorough review and catch on the roving tabindex, @barunaniket! All points have been resolved in the latest push (
Verified locally: |
barunaniket
left a comment
barunaniket
left a comment
There was a problem hiding this comment.
All four points from the previous review are addressed in eaf9f7b, and verified locally rather than from the diff alone.
Keyboard access — you took the fuller APG route rather than the one-line escape, which is the better call. onKeyDown on the tablist handles ArrowRight/Down and ArrowLeft/Up with modulo wraparound plus Home/End, preventDefault() is scoped correctly, and focus follows selection via tabRefs. The default: return before preventDefault() means Tab still exits the tablist normally, so nothing else regressed. The WCAG claim in the description is now accurate.
ARIA — aria-controls removed entirely, and role="tabpanel" is gated on hasMultipleTracks. I confirmed the second one at runtime: /initiatives/leetcode-101 now renders zero tabpanels, so the orphan really is gone on single-track events, and /initiatives/algohunt renders exactly one tablist, two tabs and one tabpanel.
.gitignore — dropped from the PR. Thanks for splitting it out.
Local verification on this head: 72/72 tests across 8 suites, tsc --noEmit clean, eslint clean on the changed component, and buildInitiativesManifest() regenerated byte-identical to the committed manifest.
One note that isn't yours to fix, carried over for whoever picks up #177: initiatives.test.ts asserts the literal track names and winners.length >= 6 against the committed AlgoHunt content, so rewriting that file will need the test updated in the same PR. I've flagged it on the issue.
Nice work on the format — this gives #177 a real example to fill in rather than a spec to interpret.
2 participants
Summary of Changes
This PR enhances the Initiatives subsystem by introducing a multi-track podium architecture to the Hall of Fame (
WinnersShowcase), allowing complex chapter events with multiple tracks (e.g., Easy Track vs Tough Track, Web3 vs AI/ML) to showcase independent 1st, 2nd, and 3rd place podium finishes with dedicated track-switching controls, WAI-ARIA accessibility, and brand-calibrated glowing accents.🌟 Key Enhancements
Multi-Track Podium Architecture (
WinnersShowcase.tsx):motion/react) when toggling between tracks.WAI-ARIA Accessibility Compliance (WCAG 2.1 AA):
role="tablist"on the switcher container,role="tab"witharia-selectedand dynamictabIndexon buttons, androle="tabpanel"linked viaaria-controls/idattributes.Brand-Aligned Glow System (Light & Dark Theme Calibrated):
border-gold,border-silver,border-bronze).0.14–0.22opacity) to prevent neon washout in dark mode.0.28–0.36opacity) that feels responsive across both light and dark themes.Dead Link Elimination:
<span>elements rather than dummy<a href="#">tags, preventing broken tab stops and console warnings.Build-Time Schema Validation & TypeScript Types:
WinnerSchemainscripts/build-initiatives.mjswith optionaltrack: z.string().optional()andbadge: z.string().optional().InitiativeWinnertype interface insrc/lib/initiatives.ts.Content Showcase (
content/initiatives/events/algohunt.md):Easy TrackandTough Trackpodiums.Repository Hygiene:
*.pdfto.gitignoreto protect the repository from accidental binary document commits.🧪 Verification & Quality Assurance
npm testpassing (72/72tests pass across all 8 suites, including dedicated multi-track podium tests insrc/lib/initiatives.test.ts).npx eslint src/app/initiatives/[slug]/WinnersShowcase.tsx(0 errors, 0 warnings).useMemo,useState) are declared unconditionally at the top level prior to any early returns.node scripts/build-initiatives.mjssuccessfully parses and writesinitiatives.manifest.json.📂 Modified Files
.gitignore: Added*.pdfrulecontent/initiatives/events/algohunt.md: Multi-track frontmatter examplescripts/build-initiatives.mjs:WinnerSchemaextension for track & badgesrc/app/initiatives/[slug]/WinnersShowcase.tsx: Multi-track podium UI & WAI-ARIA tabssrc/app/initiatives/initiatives.manifest.json: Re-generated static manifestsrc/lib/initiatives.ts: UpdatedInitiativeWinnerinterfacesrc/lib/initiatives.test.ts: Added unit test coverage for multi-track parsing