feat: redesign public competitions page#360
Merged
Merged
Conversation
… filters Replaces the utilitarian list view with a card-based grid layout featuring profile images, status-aware filter tabs, text search with deferred updates, and advanced filters (location, organizer, type). Cards show competition logo or deterministic gradient fallback with initials. - New CompetitionCard component with profile image / gradient fallback - Status filter tabs (All, Open, Live, Upcoming, Past) synced to URL - Advanced filters panel (location, organizer, competition type) - Search input with useTransition + useDeferredValue for responsiveness - Staggered card entrance animation with prefers-reduced-motion support - Accessibility fixes: aria-labels, semantic search input, sr-only text - Card enter keyframe animation in styles.css Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds a client-side Changes
Sequence Diagram(s)(Skipped.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lat.md/series-event-templates.md">
<violation number="1" location="lat.md/series-event-templates.md:81">
P3: This sentence misstates the `series_event_mappings` schema. The table has an `id` primary key; the four listed fields are columns, not four keys.</violation>
<violation number="2" location="lat.md/series-event-templates.md:116">
P2: The route examples in this section still omit the `/compete/organizer` prefix, so readers will copy incorrect URLs from the docs.</violation>
</file>
<file name="apps/wodsmith-start/src/components/competition-card.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/competition-card.tsx:122">
P2: Use TanStack Router's `Link` for this internal competition route. The raw `<a href>` forces a full page reload instead of client-side navigation.</violation>
</file>
<file name="apps/wodsmith-start/src/routes/compete/index.tsx">
<violation number="1" location="apps/wodsmith-start/src/routes/compete/index.tsx:146">
P2: Keep the controlled search input synchronized with `Route.useSearch()` changes; otherwise the field can display a stale query after URL-driven navigation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| ## Routes | ||
|
|
||
| All series event template routes are nested under the organizer dashboard. |
Contributor
There was a problem hiding this comment.
P2: The route examples in this section still omit the /compete/organizer prefix, so readers will copy incorrect URLs from the docs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lat.md/series-event-templates.md, line 116:
<comment>The route examples in this section still omit the `/compete/organizer` prefix, so readers will copy incorrect URLs from the docs.</comment>
<file context>
@@ -109,6 +113,8 @@ All defined in `src/server-fns/series-event-template-fns.ts`:
## Routes
+All series event template routes are nested under the organizer dashboard.
+
- `/series/{groupId}/events` — Layout route with `<Outlet />`
</file context>
| const showPast = past === true | ||
| const activeStatus = search.filter ?? "all" | ||
| const [, startTransition] = useTransition() | ||
| const [localSearch, setLocalSearch] = useState(search.q ?? "") |
Contributor
There was a problem hiding this comment.
P2: Keep the controlled search input synchronized with Route.useSearch() changes; otherwise the field can display a stale query after URL-driven navigation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/compete/index.tsx, line 146:
<comment>Keep the controlled search input synchronized with `Route.useSearch()` changes; otherwise the field can display a stale query after URL-driven navigation.</comment>
<file context>
@@ -33,147 +59,502 @@ export const Route = createFileRoute("/compete/")({
- const showPast = past === true
+ const activeStatus = search.filter ?? "all"
+ const [, startTransition] = useTransition()
+ const [localSearch, setLocalSearch] = useState(search.q ?? "")
+ const deferredSearchQuery = useDeferredValue(search.q)
+ const [showAdvanced, setShowAdvanced] = useState(
</file context>
| ### Persistence | ||
|
|
||
| Mappings are stored in `series_event_mappings` (groupId, competitionId, competitionEventId, templateEventId). `saveSeriesEventMappingsFn` does a full replace — deletes all existing mappings for the group, then inserts the new set atomically in a transaction. | ||
| Mappings are stored in `series_event_mappings` with four keys: groupId, competitionId, competitionEventId, templateEventId. |
Contributor
There was a problem hiding this comment.
P3: This sentence misstates the series_event_mappings schema. The table has an id primary key; the four listed fields are columns, not four keys.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lat.md/series-event-templates.md, line 81:
<comment>This sentence misstates the `series_event_mappings` schema. The table has an `id` primary key; the four listed fields are columns, not four keys.</comment>
<file context>
@@ -76,7 +78,9 @@ Each template event can only be claimed once (no duplicate mappings).
### Persistence
-Mappings are stored in `series_event_mappings` (groupId, competitionId, competitionEventId, templateEventId). `saveSeriesEventMappingsFn` does a full replace — deletes all existing mappings for the group, then inserts the new set atomically in a transaction.
+Mappings are stored in `series_event_mappings` with four keys: groupId, competitionId, competitionEventId, templateEventId.
+
+`saveSeriesEventMappingsFn` does a full replace — deletes all existing mappings for the group, then inserts the new set atomically in a transaction.
</file context>
Suggested change
| Mappings are stored in `series_event_mappings` with four keys: groupId, competitionId, competitionEventId, templateEventId. | |
| Mappings are stored in `series_event_mappings` with an `id` primary key plus four columns: groupId, competitionId, competitionEventId, templateEventId. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/wodsmith-start/src/components/competition-card.tsx`:
- Around line 122-123: Replace the raw anchor used for navigation with TanStack
Router's Link: import Link from the router and swap the <a
href={`/compete/${competition.slug}`}> usage in the CompetitionCard component
with <Link to="/compete/$slug" params={{ slug: competition.slug }}> preserving
the same children and classes/props; ensure the import for Link is added at the
top of the file (competition-card.tsx) and remove or update the old href-based
anchor so client-side navigation and router preloading work.
In `@apps/wodsmith-start/src/routes/compete/index.tsx`:
- Around line 118-123: The "all" filter currently excludes past competitions (it
returns status !== "past"), causing the label/count to be misleading; update the
filter logic so when filter === "all" it returns true (include every status) or,
if you prefer the existing behavior, rename the tab/label to reflect "non-past";
modify the conditional block that checks filter and status (the expressions with
filter === "all", "past", "active", "registration-open", "upcoming") so the
"all" branch includes past (or change the UI label/counter logic accordingly),
and apply the same change to the duplicate block that mirrors this logic
elsewhere in the file (the other conditional set using filter and status).
- Around line 352-360: The search input currently uses placeholder text only and
type="text"; change it to a real accessible search control by setting
type="search" and adding an accessible name (either add aria-label="Search
competitions" on the Input or add a visible/visually-hidden <label> tied via
id/htmlFor). Update the Input JSX where value={localSearch} and onChange={(e) =>
handleSearchChange(e.target.value)} to include the new aria-label or id so
assistive tech can announce the field properly.
- Around line 145-169: The current dual state (localSearch + URL search.q) and
manual navigation in handleSearchChange cause stale input vs URL mismatches and
create history entries per keystroke; replace this pattern by using NUQS:
install nuqs, wrap the root route with <NuqsAdapter />
(nuqs/adapters/tanstack-router), then remove localSearch, setLocalSearch,
deferredSearchQuery, and updateSearch logic and instead use useQueryState to
bind the q param to the input; update handleSearchChange to write to the
useQueryState setter (which will handle replace/merge semantics) and update the
"Clear all filters" handler to clear the same useQueryState keys so URL and
input remain single source of truth (refer to localSearch, handleSearchChange,
updateSearch, and deferredSearchQuery to find the code to remove/replace).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2116c5da-9875-49c5-8ca0-7aeb33f44d2d
📒 Files selected for processing (5)
apps/wodsmith-start/src/components/competition-card.tsxapps/wodsmith-start/src/components/competition-search.tsxapps/wodsmith-start/src/routes/compete/index.tsxapps/wodsmith-start/src/styles.csslat.md/series-event-templates.md
Comment on lines
+118
to
+123
| if (filter === "all") return status !== "past" | ||
| if (filter === "past") return status === "past" | ||
| if (filter === "active") return status === "active" | ||
| if (filter === "registration-open") return status === "registration-open" | ||
| if (filter === "upcoming") | ||
| return status === "coming-soon" || status === "registration-closed" |
Contributor
There was a problem hiding this comment.
All doesn't actually mean all right now.
The all branch filters out past, and the counter does the same. That makes the label/count misleading and hides completed competitions from the default view. Either include past in all, or rename the tab to match the behavior.
🧭 Suggested fix if `All` should include every status
function matchesStatusFilter(
status: CompetitionStatus,
filter: StatusFilter,
): boolean {
- if (filter === "all") return status !== "past"
+ if (filter === "all") return true
if (filter === "past") return status === "past"
if (filter === "active") return status === "active"
if (filter === "registration-open") return status === "registration-open"
if (filter === "upcoming")
return status === "coming-soon" || status === "registration-closed"
return true
}
@@
}
for (const comp of competitionsWithStatus) {
- if (comp._status !== "past") c.all++
+ c.all++
if (comp._status === "registration-open") c["registration-open"]++
if (comp._status === "active") c.active++
if (Also applies to: 208-217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/routes/compete/index.tsx` around lines 118 - 123, The
"all" filter currently excludes past competitions (it returns status !==
"past"), causing the label/count to be misleading; update the filter logic so
when filter === "all" it returns true (include every status) or, if you prefer
the existing behavior, rename the tab/label to reflect "non-past"; modify the
conditional block that checks filter and status (the expressions with filter ===
"all", "past", "active", "registration-open", "upcoming") so the "all" branch
includes past (or change the UI label/counter logic accordingly), and apply the
same change to the duplicate block that mirrors this logic elsewhere in the file
(the other conditional set using filter and status).
Comment on lines
+145
to
+169
| const [, startTransition] = useTransition() | ||
| const [localSearch, setLocalSearch] = useState(search.q ?? "") | ||
| const deferredSearchQuery = useDeferredValue(search.q) | ||
| const [showAdvanced, setShowAdvanced] = useState( | ||
| Boolean(search.location || search.organizer || search.type), | ||
| ) | ||
|
|
||
| // Handlers for search state updates | ||
| const handleSearchChange = (value: string) => { | ||
| navigate({ | ||
| search: (prev) => ({ ...prev, q: value || undefined }), | ||
| const updateSearch = (updates: Partial<CompeteSearch>) => { | ||
| startTransition(() => { | ||
| navigate({ | ||
| search: (prev) => { | ||
| const next = { ...prev, ...updates } | ||
| // Clean undefined values | ||
| for (const key of Object.keys(next) as (keyof CompeteSearch)[]) { | ||
| if (next[key] === undefined) delete next[key] | ||
| } | ||
| return next | ||
| }, | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| const handleShowPastChange = (value: boolean) => { | ||
| navigate({ | ||
| search: (prev) => ({ ...prev, past: value || undefined }), | ||
| }) | ||
| const handleSearchChange = (value: string) => { | ||
| setLocalSearch(value) | ||
| updateSearch({ q: value || undefined }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files apps/wodsmith-start/src/routes/compete/index.tsxRepository: wodsmith/thewodapp
Length of output: 110
🏁 Script executed:
find . -type f -name "package.json" | head -5Repository: wodsmith/thewodapp
Length of output: 244
🏁 Script executed:
wc -l apps/wodsmith-start/src/routes/compete/index.tsxRepository: wodsmith/thewodapp
Length of output: 114
🏁 Script executed:
cat -n apps/wodsmith-start/src/routes/compete/index.tsx | sed -n '140,175p'Repository: wodsmith/thewodapp
Length of output: 1467
🏁 Script executed:
cat -n apps/wodsmith-start/src/routes/compete/index.tsx | sed -n '545,560p'Repository: wodsmith/thewodapp
Length of output: 606
🏁 Script executed:
find . -name "package.json" -path "*/apps/wodsmith-start/*"Repository: wodsmith/thewodapp
Length of output: 96
🏁 Script executed:
cat apps/wodsmith-start/package.json | grep -A 5 -B 5 "nuqs\|dependencies"Repository: wodsmith/thewodapp
Length of output: 660
🏁 Script executed:
rg "nuqs" --type ts --type tsxRepository: wodsmith/thewodapp
Length of output: 89
🏁 Script executed:
rg "CompeteSearch" --type ts --type tsx -A 3Repository: wodsmith/thewodapp
Length of output: 89
🏁 Script executed:
rg "nuqs" --glob "*.ts" --glob "*.tsx"Repository: wodsmith/thewodapp
Length of output: 44
🏁 Script executed:
rg "CompeteSearch" --glob "*.ts" --glob "*.tsx" -A 3Repository: wodsmith/thewodapp
Length of output: 1527
🏁 Script executed:
cat apps/wodsmith-start/package.json | jq '.dependencies | keys[]' | grep -i nuqsRepository: wodsmith/thewodapp
Length of output: 44
🏁 Script executed:
cat apps/wodsmith-start/package.json | jq '.dependencies'Repository: wodsmith/thewodapp
Length of output: 3218
🏁 Script executed:
rg "useSearch|useNavigate" --glob "*.ts" --glob "*.tsx" apps/wodsmith-start/src/routes/ -l | head -10Repository: wodsmith/thewodapp
Length of output: 863
🏁 Script executed:
find apps/wodsmith-start/src/routes -name "*.tsx" -o -name "*.ts" | head -10Repository: wodsmith/thewodapp
Length of output: 593
🏁 Script executed:
cat -n apps/wodsmith-start/src/routes/compete/index.tsx | head -30Repository: wodsmith/thewodapp
Length of output: 1309
🏁 Script executed:
cat -n apps/wodsmith-start/src/routes/compete/index.tsx | sed -n '25,60p'Repository: wodsmith/thewodapp
Length of output: 1389
🌐 Web query:
NUQS TanStack Router compatibility v1
💡 Result:
Yes, NUQS (nuqs) is compatible with TanStack Router v1. Official documentation confirms support for @tanstack/react-router@^1 via the nuqs/adapters/tanstack-router adapter. Key details: - Wrap your root route with from 'nuqs/adapters/tanstack-router'. - Support added in nuqs v2.5 (experimental initially), matured through fixes like serialization (PR #1128) and trailing slash navigation (Issue #1215, fixed in v2.8.1). - Limited type-safe integration via Standard Schema for trivial types (strings, numbers, booleans, JSON); complex serialization differences limit full TSR type-safety. - Does not yet support TanStack Start. - Installation: npm install nuqs, requires React ^18.3 or ^19. Example usage: import { NuqsAdapter } from 'nuqs/adapters/tanstack-router' import { Outlet, createRootRoute } from '@tanstack/react-router' export const Route = createRootRoute({ component: => ( ), }) Use hooks like useQueryState as usual in components.
Citations:
- 1: https://nuqs.dev/docs
- 2: https://nuqs.dev/docs/adapters
- 3: https://www.npmjs.com/package/nuqs?activeTab=readme
- 4: useQueryState causes a navigation to trailing slash with search param w/ TanStack Router 47ng/nuqs#1215
- 5: fix: Use TanStack Router default stringify for search params 47ng/nuqs#1128
- 6: https://nuqs.dev/blog/nuqs-2.5
Implement NUQS for URL search parameter state management.
The dual state setup (localSearch and search.q) allows URL-driven changes like back/forward navigation or deep-linked queries to clear the results while leaving stale text in the search field. Additionally, handleSearchChange navigates on every keystroke, which creates a history entry per character typed unless using replace mode. Using NUQS with the TanStack Router adapter will unify input state and URL params into one source of truth.
This project's coding guidelines require "Use NUQS for URL search parameter state management" for TypeScript/TSX files. After installing NUQS, wrap the root route with <NuqsAdapter /> from nuqs/adapters/tanstack-router, then replace the manual search state management with useQueryState hooks to synchronize the input field and URL automatically.
This applies to the search input at lines 145–169 and the "Clear all filters" button at line 554.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/routes/compete/index.tsx` around lines 145 - 169, The
current dual state (localSearch + URL search.q) and manual navigation in
handleSearchChange cause stale input vs URL mismatches and create history
entries per keystroke; replace this pattern by using NUQS: install nuqs, wrap
the root route with <NuqsAdapter /> (nuqs/adapters/tanstack-router), then remove
localSearch, setLocalSearch, deferredSearchQuery, and updateSearch logic and
instead use useQueryState to bind the q param to the input; update
handleSearchChange to write to the useQueryState setter (which will handle
replace/merge semantics) and update the "Clear all filters" handler to clear the
same useQueryState keys so URL and input remain single source of truth (refer to
localSearch, handleSearchChange, updateSearch, and deferredSearchQuery to find
the code to remove/replace).
Contributor
Author
There was a problem hiding this comment.
We dont use NUQS anymore
Contributor
There was a problem hiding this comment.
@theianjones, got it — my apologies for the noise. I'll remove the outdated NUQS guideline from the knowledge base now.
The current approach using TanStack Router's useSearch / useNavigate for URL search parameter state management looks fine then. Would you like me to add a learning to capture what you use instead (e.g. "Use TanStack Router's useSearch/useNavigate for URL search parameter state management"), so future reviews don't re-flag this pattern?
⛔ Learnings removed
Learnt from: CR
Repo: wodsmith/thewodapp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T22:10:54.989Z
Learning: Applies to **/*.{ts,tsx} : Use NUQS for URL search parameter state management
🧠 Learnings used
Learnt from: CR
Repo: wodsmith/thewodapp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T22:10:54.989Z
Learning: Applies to **/*.{ts,tsx} : Use NUQS for URL search parameter state management
…bility Replace raw <a href> with TanStack Router <Link> for client-side navigation and preloading. Add aria-label and type="search" to competition search input. Fix broken lat.md refs for registration sections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/routes/compete/index.tsx">
<violation number="1" location="apps/wodsmith-start/src/routes/compete/index.tsx:353">
P2: `type="search"` adds a native clear button in Blink/WebKit, so this input now shows two competing clear controls.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+353
to
+357
| type="search" | ||
| aria-label="Search competitions" | ||
| name="competition-search" | ||
| placeholder="Search…" | ||
| value={localSearch} |
Contributor
There was a problem hiding this comment.
P2: type="search" adds a native clear button in Blink/WebKit, so this input now shows two competing clear controls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/compete/index.tsx, line 353:
<comment>`type="search"` adds a native clear button in Blink/WebKit, so this input now shows two competing clear controls.</comment>
<file context>
@@ -350,7 +350,8 @@ function CompetePage() {
/>
<Input
- type="text"
+ type="search"
+ aria-label="Search competitions"
name="competition-search"
</file context>
Remove unused totalPlatformFeeCents variable, replace nav with div for tablist role, remove useless undefined initializations, simplify ternary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/wodsmith-start/src/routes/compete/index.tsx (2)
114-124:⚠️ Potential issue | 🟠 Major
Allstill excludes completed competitions.The filter and the counter both drop
past, so the default tab label/count are misleading. This also makes a dataset with only past competitions render as empty.Suggested fix
function matchesStatusFilter( status: CompetitionStatus, filter: StatusFilter, ): boolean { - if (filter === "all") return status !== "past" + if (filter === "all") return true if (filter === "past") return status === "past" if (filter === "active") return status === "active" if (filter === "registration-open") return status === "registration-open" if (filter === "upcoming") return status === "coming-soon" || status === "registration-closed" return true } @@ } for (const comp of competitionsWithStatus) { - if (comp._status !== "past") c.all++ + c.all++ if (comp._status === "registration-open") c["registration-open"]++ if (comp._status === "active") c.active++ if (Also applies to: 199-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/src/routes/compete/index.tsx` around lines 114 - 124, The "all" branch in matchesStatusFilter currently excludes past competitions (if (filter === "all") return status !== "past"), causing counts/tabs to be misleading and datasets of only past comps to render empty; update matchesStatusFilter so the "all" case returns true (include past) and verify/align the duplicate implementation (the other matchesStatusFilter block) to use the same behavior so both the filter and counter treat "all" consistently.
146-169:⚠️ Potential issue | 🟠 MajorKeep the search box synced with URL state, and don’t push history on every keystroke.
localSearchonly hydrates fromsearch.qonce. After browser back/forward or Line 555’snavigate({ search: {} }), the list can clear while the input keeps stale text. This path also adds a history entry per character typed.One way to tighten this up
const [, startTransition] = useTransition() const [localSearch, setLocalSearch] = useState(search.q ?? "") const deferredSearchQuery = useDeferredValue(search.q) + useEffect(() => { + setLocalSearch(search.q ?? "") + }, [search.q]) @@ const handleSearchChange = (value: string) => { setLocalSearch(value) - updateSearch({ q: value || undefined }) + startTransition(() => { + navigate({ + replace: true, + search: (prev) => { + const next = { ...prev, q: value || undefined } + if (next.q === undefined) delete next.q + return next + }, + }) + }) }// also update the React import at Line 1 import { useDeferredValue, useEffect, useMemo, useState, useTransition } from "react"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/src/routes/compete/index.tsx` around lines 146 - 169, The input value isn't kept in sync with URL changes and you push a history entry on every keystroke; fix by syncing localSearch from the URL and only pushing URL updates from the deferred value (or using replace) so typing doesn't create history entries: add a useEffect that sets setLocalSearch(search.q ?? "") when search.q changes (to handle back/forward and external navigate calls), and change the update path so the actual navigate call is triggered from an effect that watches deferredSearchQuery (the useDeferredValue of search.q) or calls navigate with { replace: true } when updating from the typing path; reference localSearch/setLocalSearch, deferredSearchQuery, handleSearchChange, updateSearch and navigate to locate where to apply these changes.
🧹 Nitpick comments (2)
apps/wodsmith-start/src/components/competition-card.tsx (1)
86-104: Prefer a named params object forformatDateRange.
startDateandendDateare the same type, so the current call site is easy to swap accidentally. A small params interface keeps the call explicit.Possible cleanup
+interface FormatDateRangeParams { + startDate: string + endDate: string +} + -function formatDateRange(startDate: string, endDate: string) { +function formatDateRange({ startDate, endDate }: FormatDateRangeParams) { const start = new Date(startDate) const end = new Date(endDate)const dateRange = formatDateRange({ startDate: competition.startDate, endDate: competition.endDate, })As per coding guidelines, "Use named object parameters for functions with more than one parameter" and "Use TypeScript everywhere and prefer interfaces over types".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/src/components/competition-card.tsx` around lines 86 - 104, The function formatDateRange should accept a single named-params object to avoid argument-order mistakes: change its signature to accept an object (e.g., { startDate, endDate }) with a small interface (prefer interface IFormatDateRangeParams { startDate: string; endDate: string }), update the internal references (startDate/endDate -> params.startDate/params.endDate or destructure in the parameter list) and adjust all callers (e.g., replace formatDateRange(a,b) with formatDateRange({ startDate: a, endDate: b })); keep the existing logic (including the isSameUTCDay call) intact.apps/wodsmith-start/src/routes/compete/index.tsx (1)
28-34: Use an interface forCompeteSearch.This is an object contract, so it can follow the repo’s TypeScript convention and stay consistent with the other prop/search shapes in this change.
Minimal change
-type CompeteSearch = { +interface CompeteSearch { q?: string filter?: StatusFilter location?: string organizer?: string type?: "in-person" | "online" -} +}As per coding guidelines, "Use TypeScript everywhere and prefer interfaces over types".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/src/routes/compete/index.tsx` around lines 28 - 34, Replace the type alias CompeteSearch with an equivalent interface to follow the repo convention: declare interface CompeteSearch { q?: string; filter?: StatusFilter; location?: string; organizer?: string; type?: "in-person" | "online"; } so the object contract remains identical; update any imports/usages referencing CompeteSearch if needed but keep the same optional fields and the union for the type property.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/wodsmith-start/src/components/competition-card.tsx`:
- Around line 126-135: The inline animation set via the style prop (style={{
animation: `card-enter 0.35s ease-out ${index * 50}ms backwards` }}) still runs
for users who prefer reduced motion; change the component to detect
prefers-reduced-motion (e.g., using window.matchMedia('(prefers-reduced-motion:
reduce)').matches or a useReducedMotion hook) and only set the style.animation
when reduced motion is false, otherwise omit the animation property (or set it
to 'none'); update the code that uses the inline style and the index-based
stagger so the animation is conditionally applied.
In `@apps/wodsmith-start/src/routes/compete/index.tsx`:
- Around line 299-341: The controls are simple list filters but currently use
tablist/tab semantics; change them to use standard toggle buttons by removing
role="tablist" on the <nav> and role="tab" and aria-selected on each <button>,
and instead add aria-pressed={isActive} to the <button> (keep the existing
aria-label on the nav and keyboard behavior untouched). Update the render that
references STATUS_TABS, activeStatus, counts, and updateSearch so each button
reflects isActive via aria-pressed and continues to call updateSearch when
clicked.
---
Duplicate comments:
In `@apps/wodsmith-start/src/routes/compete/index.tsx`:
- Around line 114-124: The "all" branch in matchesStatusFilter currently
excludes past competitions (if (filter === "all") return status !== "past"),
causing counts/tabs to be misleading and datasets of only past comps to render
empty; update matchesStatusFilter so the "all" case returns true (include past)
and verify/align the duplicate implementation (the other matchesStatusFilter
block) to use the same behavior so both the filter and counter treat "all"
consistently.
- Around line 146-169: The input value isn't kept in sync with URL changes and
you push a history entry on every keystroke; fix by syncing localSearch from the
URL and only pushing URL updates from the deferred value (or using replace) so
typing doesn't create history entries: add a useEffect that sets
setLocalSearch(search.q ?? "") when search.q changes (to handle back/forward and
external navigate calls), and change the update path so the actual navigate call
is triggered from an effect that watches deferredSearchQuery (the
useDeferredValue of search.q) or calls navigate with { replace: true } when
updating from the typing path; reference localSearch/setLocalSearch,
deferredSearchQuery, handleSearchChange, updateSearch and navigate to locate
where to apply these changes.
---
Nitpick comments:
In `@apps/wodsmith-start/src/components/competition-card.tsx`:
- Around line 86-104: The function formatDateRange should accept a single
named-params object to avoid argument-order mistakes: change its signature to
accept an object (e.g., { startDate, endDate }) with a small interface (prefer
interface IFormatDateRangeParams { startDate: string; endDate: string }), update
the internal references (startDate/endDate -> params.startDate/params.endDate or
destructure in the parameter list) and adjust all callers (e.g., replace
formatDateRange(a,b) with formatDateRange({ startDate: a, endDate: b })); keep
the existing logic (including the isSameUTCDay call) intact.
In `@apps/wodsmith-start/src/routes/compete/index.tsx`:
- Around line 28-34: Replace the type alias CompeteSearch with an equivalent
interface to follow the repo convention: declare interface CompeteSearch { q?:
string; filter?: StatusFilter; location?: string; organizer?: string; type?:
"in-person" | "online"; } so the object contract remains identical; update any
imports/usages referencing CompeteSearch if needed but keep the same optional
fields and the union for the type property.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aac4f714-180e-4b4c-91ca-a3341cf71809
📒 Files selected for processing (3)
apps/wodsmith-start/src/components/competition-card.tsxapps/wodsmith-start/src/routes/compete/index.tsxapps/wodsmith-start/src/server-fns/registration-fns.ts
✅ Files skipped from review due to trivial changes (1)
- apps/wodsmith-start/src/server-fns/registration-fns.ts
Comment on lines
+126
to
+135
| className={cn( | ||
| "group relative flex gap-4 rounded-lg border bg-card p-4 sm:p-5", | ||
| "transition-[border-color,box-shadow] duration-200", | ||
| "hover:border-primary/30 hover:shadow-md hover:shadow-primary/5", | ||
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2", | ||
| "motion-reduce:transition-none", | ||
| )} | ||
| style={{ | ||
| animation: `card-enter 0.35s ease-out ${index * 50}ms backwards`, | ||
| }} |
Contributor
There was a problem hiding this comment.
The enter animation still runs for reduced-motion users.
motion-reduce:transition-none only disables transitions. The inline animation on Lines 133-135 still fires, so the staggered entrance effect is not actually disabled for prefers-reduced-motion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/components/competition-card.tsx` around lines 126 -
135, The inline animation set via the style prop (style={{ animation:
`card-enter 0.35s ease-out ${index * 50}ms backwards` }}) still runs for users
who prefer reduced motion; change the component to detect prefers-reduced-motion
(e.g., using window.matchMedia('(prefers-reduced-motion: reduce)').matches or a
useReducedMotion hook) and only set the style.animation when reduced motion is
false, otherwise omit the animation property (or set it to 'none'); update the
code that uses the inline style and the index-based stagger so the animation is
conditionally applied.
Comment on lines
+299
to
+341
| <nav | ||
| className="flex items-center gap-0.5 -ml-2" | ||
| role="tablist" | ||
| aria-label="Filter competitions" | ||
| > | ||
| {STATUS_TABS.map((tab) => { | ||
| const isActive = activeStatus === tab.value | ||
| const count = counts[tab.value] | ||
| return ( | ||
| <button | ||
| key={tab.value} | ||
| type="button" | ||
| role="tab" | ||
| aria-selected={isActive} | ||
| onClick={() => | ||
| updateSearch({ | ||
| filter: tab.value === "all" ? undefined : tab.value, | ||
| }) | ||
| } | ||
| className={cn( | ||
| "relative px-3 py-1.5 text-sm rounded-md", | ||
| "transition-colors duration-100 motion-reduce:transition-none", | ||
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2", | ||
| isActive | ||
| ? "font-semibold text-foreground bg-secondary" | ||
| : "font-medium text-muted-foreground hover:text-foreground hover:bg-secondary/50", | ||
| )} | ||
| > | ||
| {tab.label} | ||
| {count > 0 && ( | ||
| <span | ||
| className={cn( | ||
| "ml-1.5 text-xs tabular-nums", | ||
| isActive | ||
| ? "text-foreground/50" | ||
| : "text-muted-foreground/60", | ||
| )} | ||
| > | ||
| {count} | ||
| </span> | ||
| )} | ||
| </button> | ||
| ) |
Contributor
There was a problem hiding this comment.
Use pressed-state buttons here instead of ARIA tabs.
These controls filter one list; they do not implement roving focus, arrow-key navigation, or tabpanel relationships. The current tablist/tab roles can mislead assistive tech.
Safer semantics for this pattern
- <nav
- className="flex items-center gap-0.5 -ml-2"
- role="tablist"
- aria-label="Filter competitions"
- >
+ <nav className="flex items-center gap-0.5 -ml-2" aria-label="Filter competitions">
@@
<button
key={tab.value}
type="button"
- role="tab"
- aria-selected={isActive}
+ aria-pressed={isActive}
onClick={() =>
updateSearch({
filter: tab.value === "all" ? undefined : tab.value,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/routes/compete/index.tsx` around lines 299 - 341, The
controls are simple list filters but currently use tablist/tab semantics; change
them to use standard toggle buttons by removing role="tablist" on the <nav> and
role="tab" and aria-selected on each <button>, and instead add
aria-pressed={isActive} to the <button> (keep the existing aria-label on the nav
and keyboard behavior untouched). Update the render that references STATUS_TABS,
activeStatus, counts, and updateSearch so each button reflects isActive via
aria-pressed and continues to call updateSearch when clicked.
Add motion-reduce:!animate-none to override inline animation style when user has reduced motion preference enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
useTransition+useDeferredValuefor lag-free typingprefers-reduced-motionsupportaria-labels, semantic input types,sr-onlytextTest plan
prefers-reduced-motiondisables animations🤖 Generated with Claude Code
Summary by cubic
Redesigned the public competitions page into a responsive card grid with status tabs, fast search, and advanced filters; state syncs to URL params for shareable views. Also fixes minor accessibility and lint issues without changing behavior.
New Features
filterin the URL, with countsuseTransition+useDeferredValuefor lag-free typingprefers-reduced-motionsupportAccessibility & UX
@tanstack/react-routerLinkfor client-side navigation and preloadingtype="search",name,autoComplete="off",spellCheck={false}, clear button, and ARIA labelWritten for commit 75672a1. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI/UX
Documentation