[WIP]: defining arena structure#10
Conversation
WalkthroughThis pull request introduces a multi-activity arena system with support for drawing, coding, and typing competitions. It adds participant tracking, spectator logic, cursor synchronization, and activity-type-specific rendering components. A new Excalidraw dependency is added to support drawing functionality. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
src/routes/arena/-draw/canvas.tsx (1)
4-22: Remove or prefix unused props for clarity.The
DrawCanvascomponent destructurespaneId,elements,onElementsChange, andonCursorMovebut doesn't use them. Since this is a WIP placeholder for future Excalidraw integration:
- Either remove these unused props from the interface temporarily, or
- Prefix them with an underscore (
_paneId,_elements, etc.) to indicate they're intentionally unused and reserved for future implementation.🔎 Option 1: Prefix unused props
export function DrawCanvas({ - ownerId, + _paneId, + ownerId, ownerUsername, isEditable, - cursors, + _elements, + _cursors, + _onElementsChange, + _onCursorMove, onFocus }: DrawCanvasProps) {As per coding guidelines: Avoid unused variables in TypeScript.
src/routes/arena/-components/pane-grid.tsx (3)
29-41: Consider 3-player layout on wide screens.The third pane uses
w-1/2which could appear overly wide on large displays. Consider usingmax-w-xlor similar to cap the width while keeping it centered:<div className="flex justify-center"> - <div className="h-full w-1/2">{children(2)}</div> + <div className="h-full w-1/2 max-w-xl">{children(2)}</div> </div>
54-69: Document fallback grid calculation.The grid calculation logic is clever but not immediately obvious. Adding a comment explaining the math would improve maintainability:
// fallback for larger counts - basically equivalent to 1, 2, 4 grid +// Creates a square-ish grid: columns = ceil(sqrt(n)), rows = ceil(n/columns) +// e.g., 5 players → 3×2 grid, 6 players → 3×2 grid, 7 players → 3×3 grid return (
15-70: Consider mobile responsiveness.All layouts use fixed grid structures without responsive breakpoints. On mobile devices, side-by-side panes may be too cramped. Consider stacking panes vertically on smaller screens:
if (paneCount === 2) { return ( <div className="grid h-full w-full grid-cols-1 md:grid-cols-2 gap-2"> ... </div> ) }As per coding guidelines: Follow Tailwind CSS best practices.
src/routes/arena/-components/arena-header.tsx (1)
56-67: Simplify className construction with cn() helper.The template literal with nested ternary is difficult to read. Use the
cn()utility (from clsx/tailwind-merge) that's likely already in the project for cleaner className composition:🔎 Proposed refactor
+import { cn } from "@/lib/utils" <div - className={`flex items-center gap-2 font-bold font-mono text-xl ${ - isCriticalTime - ? "animate-pulse text-destructive" - : isLowTime - ? "text-yellow-500" - : "" - }`} + className={cn( + "flex items-center gap-2 font-bold font-mono text-xl", + isCriticalTime && "animate-pulse text-destructive", + isLowTime && !isCriticalTime && "text-yellow-500" + )} >As per coding guidelines: Prefer clear code over complex expressions.
src/routes/arena/-code/arena.tsx (2)
16-16: Unused prop: onCursorMove.The
onCursorMoveprop is received but never used in the component. Either wire it to the CodePane components or remove it from the props interface.#!/bin/bash # Description: Check if CodePane accepts onCursorMove prop # Search for CodePane interface/type definition ast-grep --pattern $'interface CodePaneProps { $$$ }' # Search for onCursorMove usage in pane.tsx rg -n 'onCursorMove' src/routes/arena/-code/pane.tsxAs per coding guidelines: Avoid unused variables in TypeScript.
66-78: Consider extracting CodePane props logic.The CodePane rendering logic is duplicated between the grid (lines 66-78) and the overlay (lines 91-107). Consider extracting a helper function to compute the shared props:
🔎 Proposed refactor
const getCodePaneProps = (participant: Participant) => ({ paneId: participant.id, ownerId: participant.id, ownerUsername: participant.username, isEditable: participant.id === userId && !isSpectator, code: codeByParticipant[participant.id] ?? "", testResults: testResultsByParticipant[participant.id] ?? [], cursor: participant.id === userId ? undefined : cursors.get(participant.id), onCodeChange: participant.id === userId ? onCodeChange : undefined }) // Then use it: <CodePane {...getCodePaneProps(participant)} onFocus={() => setFocusedPaneId(participant.id)} />Based on learnings: Colocate code that changes together in React components.
src/routes/arena/-code/pane.tsx (1)
11-30: Unused props in CodePaneProps interface.Several props defined in
CodePanePropsare not used in the component:paneId,ownerId,cursor,onCodeChange, andonCursorMove. Since this is a WIP PR, consider either:
- Adding a TODO comment explaining these are placeholders for future integration
- Removing unused props until they're needed (YAGNI principle)
src/routes/arena/-typing/arena.tsx (2)
64-69: Error counting may double-count on rapid typing or paste.The error check compares only the last character of the new value against the prompt. This works for single-character typing but could behave unexpectedly if a user pastes multiple characters or if events fire in unusual order.
Consider validating the entire typed string against the prompt or tracking which indices have already been scored as errors.
98-118: Consider memoizingrenderTextfor performance.The
renderTextfunction recreates on every render. While the current prompt length is likely small, if the prompt becomes large, this could cause unnecessary re-renders. Since it only depends onpromptandinput, it could be wrapped inuseCallbackoruseMemo.src/routes/arena/-components/arena-active.tsx (2)
50-50: Type casting empty string asId<"users">is a type safety concern.Casting an empty string to
Id<"users">bypasses type safety. Consider using a more explicit handling:🔎 Suggested alternative
- hostId: user?._id ?? ("" as Id<"users">) + hostId: user?._id as Id<"users"> // Will only be used when user existsOr handle the undefined case explicitly in the config type.
20-21: Unused propcurrentPlayerCount.The
currentPlayerCountprop is defined inArenaActivePropsbut never used in the component. The header usesparticipants.lengthfrom the socket instead.src/routes/arena/-draw/arena.tsx (2)
12-12: Consider using a more specific type thanunknown[]for elements.Using
unknown[]for Excalidraw elements loses type safety. If the Excalidraw types are available, consider usingExcalidrawElement[]or creating a type alias for clarity.What is the type for Excalidraw elements array in @excalidraw/excalidraw?
57-59: Cursor filtering logic could benefit from a clarifying comment.The cursor logic filters differently based on ownership:
paneCursorsexcludes the pane owner's cursor (shows others' cursors)- Owner panes receive empty array, non-owner panes receive
paneCursorsThis means owners don't see any cursors on their own pane, while viewers of others' panes see the cursors of other viewers. Consider adding a brief comment explaining this intentional behavior.
Also applies to: 69-69
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (13)
package.json(1 hunks)src/routes/arena/$arenaId.tsx(1 hunks)src/routes/arena/-code/arena.tsx(1 hunks)src/routes/arena/-code/pane.tsx(1 hunks)src/routes/arena/-components/arena-active.tsx(5 hunks)src/routes/arena/-components/arena-header.tsx(1 hunks)src/routes/arena/-components/focus-overlay.tsx(1 hunks)src/routes/arena/-components/pane-grid.tsx(1 hunks)src/routes/arena/-draw/arena.tsx(1 hunks)src/routes/arena/-draw/canvas.tsx(1 hunks)src/routes/arena/-hooks/use-arena-socket.ts(4 hunks)src/routes/arena/-typing/arena.tsx(1 hunks)src/routes/arena/-typing/wpm-display.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/alchemy_cloudflare.mdc)
Durable Object classes must extend
DurableObjectfrom 'cloudflare:workers' and implement async methods for state management
**/*.{ts,tsx}: Make use of function guards whenever possible in TypeScript
Keep logic in one function unless the code is composable or reusable in TypeScript
Resolve all TypeScript errors - type checking is strict
Do not use unnecessary destructuring of variables in TypeScript
Do not useelsestatements unless necessary in TypeScript
Do not usetry/catchif it can be avoided in TypeScript
Avoid usinganytype in TypeScript
Avoidletstatements in TypeScript - preferconst
Prefer single word variable names where possible in TypeScript
Only create an abstraction if it is actually needed in TypeScript
Prefer clear function/variable names over inline comments in TypeScript
Files:
src/routes/arena/$arenaId.tsxsrc/routes/arena/-components/arena-header.tsxsrc/routes/arena/-hooks/use-arena-socket.tssrc/routes/arena/-draw/canvas.tsxsrc/routes/arena/-components/focus-overlay.tsxsrc/routes/arena/-components/pane-grid.tsxsrc/routes/arena/-typing/wpm-display.tsxsrc/routes/arena/-code/arena.tsxsrc/routes/arena/-components/arena-active.tsxsrc/routes/arena/-draw/arena.tsxsrc/routes/arena/-code/pane.tsxsrc/routes/arena/-typing/arena.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{tsx,jsx}: Avoid massive JSX blocks and compose smaller components in React
Colocate code that changes together in React components
AvoiduseEffectunless absolutely needed in React
Files:
src/routes/arena/$arenaId.tsxsrc/routes/arena/-components/arena-header.tsxsrc/routes/arena/-draw/canvas.tsxsrc/routes/arena/-components/focus-overlay.tsxsrc/routes/arena/-components/pane-grid.tsxsrc/routes/arena/-typing/wpm-display.tsxsrc/routes/arena/-code/arena.tsxsrc/routes/arena/-components/arena-active.tsxsrc/routes/arena/-draw/arena.tsxsrc/routes/arena/-code/pane.tsxsrc/routes/arena/-typing/arena.tsx
**/*.{css,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow Tailwind CSS best practices in v4 with global CSS file format and shadcn/ui
Files:
src/routes/arena/$arenaId.tsxsrc/routes/arena/-components/arena-header.tsxsrc/routes/arena/-draw/canvas.tsxsrc/routes/arena/-components/focus-overlay.tsxsrc/routes/arena/-components/pane-grid.tsxsrc/routes/arena/-typing/wpm-display.tsxsrc/routes/arena/-code/arena.tsxsrc/routes/arena/-components/arena-active.tsxsrc/routes/arena/-draw/arena.tsxsrc/routes/arena/-code/pane.tsxsrc/routes/arena/-typing/arena.tsx
**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid unnecessary Tailwind utilities - keep styling as minimal as possible
Files:
src/routes/arena/$arenaId.tsxsrc/routes/arena/-components/arena-header.tsxsrc/routes/arena/-draw/canvas.tsxsrc/routes/arena/-components/focus-overlay.tsxsrc/routes/arena/-components/pane-grid.tsxsrc/routes/arena/-typing/wpm-display.tsxsrc/routes/arena/-code/arena.tsxsrc/routes/arena/-components/arena-active.tsxsrc/routes/arena/-draw/arena.tsxsrc/routes/arena/-code/pane.tsxsrc/routes/arena/-typing/arena.tsx
package.json
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
Always add
@types/nodetopackage.jsonwhen using any Node.js built-in modules
Files:
package.json
{package.json,bun.lockb,bunfig.toml}
📄 CodeRabbit inference engine (AGENTS.md)
Use bun and bunx for package management and script execution
Files:
package.json
🧠 Learnings (1)
📚 Learning: 2025-12-16T11:46:11.833Z
Learnt from: CR
Repo: muzzlol/spectra PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T11:46:11.833Z
Learning: Applies to **/*.{tsx,jsx} : Colocate code that changes together in React components
Applied to files:
src/routes/arena/-code/arena.tsxsrc/routes/arena/-code/pane.tsx
🧬 Code graph analysis (7)
src/routes/arena/-components/arena-header.tsx (2)
convex/schema/arena.ts (3)
ArenaType(19-19)ArenaMode(20-20)MODE_CONFIG(23-34)src/components/ui/spinner.tsx (1)
Spinner(8-45)
src/routes/arena/-hooks/use-arena-socket.ts (1)
shared/arena-protocol.ts (1)
Participant(32-36)
src/routes/arena/-draw/canvas.tsx (1)
src/routes/arena/-hooks/use-arena-socket.ts (1)
CursorData(11-16)
src/routes/arena/-typing/wpm-display.tsx (1)
shared/arena-protocol.ts (1)
Participant(32-36)
src/routes/arena/-components/arena-active.tsx (3)
src/routes/arena/-typing/wpm-display.tsx (1)
TypingProgress(4-10)src/routes/arena/-draw/arena.tsx (1)
DrawArena(19-108)src/routes/arena/-typing/arena.tsx (1)
TypingArena(14-186)
src/routes/arena/-code/pane.tsx (1)
src/routes/arena/-hooks/use-arena-socket.ts (1)
CursorData(11-16)
src/routes/arena/-typing/arena.tsx (2)
shared/arena-protocol.ts (1)
Participant(32-36)src/routes/arena/-typing/wpm-display.tsx (2)
TypingProgress(4-10)WpmDisplay(19-113)
🔇 Additional comments (18)
src/routes/arena/-hooks/use-arena-socket.ts (4)
10-16: LGTM: Clear cursor data structure.The CursorData type is well-defined with appropriate fields for tracking participant cursor positions. The comment correctly notes this is type-agnostic and interpreted by pane components.
99-112: LGTM: Correct cursor cleanup on participant departure.Properly removes the cursor entry when a participant leaves, preventing stale cursor data from accumulating.
121-134: Cursor message type is correctly defined in protocol.The
ServerMsgtype in~/shared/arena-protocol.tsincludes the cursor message variant with all required fields:participantId,x, andy. The handler implementation matches the protocol definition.
121-126: No change needed; the server cursor message does not include a timestamp field.The protocol definition shows the server message for cursor updates contains only
participantId,x, andycoordinates. The client-sideDate.now()timestamp is generated because the server does not provide one. If server-authoritative timestamps are required for this use case, the protocol would need to be extended, but that's a separate architectural decision.src/routes/arena/-components/focus-overlay.tsx (1)
29-39: LGTM: Proper cleanup of event listeners and body styles.The useEffect correctly manages keyboard event listeners and body overflow, with cleanup in the return function.
src/routes/arena/-components/arena-header.tsx (1)
35-42: LGTM: Clear connection state handling.The connection icon logic correctly maps ReadyState values to appropriate visual indicators (Wifi, Spinner, WifiOff).
src/routes/arena/-code/arena.tsx (2)
71-75: Verify intentional cursor behavior for owner.Line 74 passes
undefinedfor the cursor when the participant is the owner. This appears intentional (owners see their own cursor, not need to sync), but confirm this is the desired behavior rather than passing the owner's cursor data.
33-37: LGTM: Clear participant sorting logic.The sorting ensures the current user's pane appears first in the grid, improving UX by making the user's own editor immediately visible.
src/routes/arena/$arenaId.tsx (1)
84-84: arena.participants field type is correctly defined and typed.Arena schema includes
participants: v.array(v.id("users"))which correctly matches the expectedArenaActiveProps.participantIdstype ofId<"users">[].package.json (1)
21-21: Version 0.18.0 is valid and free from known security advisories.The specified version 0.18.0 is the latest stable release on npm. All documented XSS vulnerabilities in the package have been patched in version 0.15.3 and later, and additional stored XSS vulnerabilities related to the web embeddable component are fixed in 0.17.6 and 0.16.4. No active security advisories exist for version 0.18.0.
src/routes/arena/-code/pane.tsx (1)
34-116: Well-structured component with clear composition.The component is cleanly organized into distinct sections (header, editor area, test results panel) with appropriate conditional rendering. The test results summary logic and status indicators are implemented correctly.
src/routes/arena/-typing/arena.tsx (1)
91-95: useEffect for initial focus is appropriate here.This is a valid use of
useEffectfor triggering a DOM side effect (focusing the input) on mount. The dependency array correctly includesisSpectator.src/routes/arena/-typing/wpm-display.tsx (1)
26-34: Sorting logic is clear and correct.The sorting prioritizes finished participants first, then sorts by character progress descending. This provides a sensible leaderboard ordering.
src/routes/arena/-components/arena-active.tsx (3)
77-79: Spectator detection logic may have unintended behavior whenparticipantIdsis empty.When
participantIdsis empty ([]),participantIds.length > 0isfalse, so the entire expression evaluates tofalse(user is not a spectator). This means users are treated as participants when no participant list exists yet.If this is intentional (e.g., during arena setup), consider adding a comment. Otherwise, the logic might need adjustment.
39-41: Unused state setters indicate incomplete integration.The state declarations only destructure the getter, not the setter (
const [codeByParticipant]instead ofconst [codeByParticipant, setCodeByParticipant]). This is appropriate for WIP stubs, but ensure the TODO items in lines 176-177 and 195-196 are tracked for completion.
134-202: Good component composition with type-specific arena rendering.The conditional rendering based on arena type is clean and each sub-arena receives appropriate props. The connection state handling provides good UX feedback.
src/routes/arena/-draw/arena.tsx (2)
79-105: FocusOverlay integration is well-implemented.The overlay correctly computes ownership and passes appropriate handlers based on whether the focused participant is the current user. The conditional rendering of
DrawCanvasinside the overlay is clean.
32-36: Sorting prioritizes current user correctly.The sort places the current user first while maintaining relative order of other participants. This is a stable sort pattern and works as intended.
| const formatTime = (seconds: number) => { | ||
| const mins = Math.floor(seconds / 60) | ||
| const secs = Math.floor(seconds % 60) | ||
| return `${mins}:${secs.toString().padStart(2, "0")}` | ||
| } |
There was a problem hiding this comment.
Handle negative time values.
The formatTime function doesn't guard against negative timeRemaining values. If the timer goes negative (due to timing issues or delays), this could display incorrectly (e.g., "-1:-01").
🔎 Proposed fix
const formatTime = (seconds: number) => {
+ const clampedSeconds = Math.max(0, seconds)
- const mins = Math.floor(seconds / 60)
- const secs = Math.floor(seconds % 60)
+ const mins = Math.floor(clampedSeconds / 60)
+ const secs = Math.floor(clampedSeconds % 60)
return `${mins}:${secs.toString().padStart(2, "0")}`
}📝 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.
| const formatTime = (seconds: number) => { | |
| const mins = Math.floor(seconds / 60) | |
| const secs = Math.floor(seconds % 60) | |
| return `${mins}:${secs.toString().padStart(2, "0")}` | |
| } | |
| const formatTime = (seconds: number) => { | |
| const clampedSeconds = Math.max(0, seconds) | |
| const mins = Math.floor(clampedSeconds / 60) | |
| const secs = Math.floor(clampedSeconds % 60) | |
| return `${mins}:${secs.toString().padStart(2, "0")}` | |
| } |
🤖 Prompt for AI Agents
In src/routes/arena/-components/arena-header.tsx around lines 26 to 30,
formatTime currently formats negative seconds directly which yields outputs like
"-1:-01"; update the function to clamp the input to a non-negative integer
(e.g., use Math.max(0, Math.floor(seconds))) before computing mins and secs,
then format mins and padStart the seconds as before so negative inputs render
"0:00" (or the clamped value) instead of showing a negative time.
| return createPortal( | ||
| <div | ||
| className="fixed inset-0 z-50 flex items-center justify-center bg-black/80" | ||
| onClick={onClose} | ||
| > | ||
| <div | ||
| className="relative flex h-[90vh] w-[90vw] flex-col overflow-hidden rounded-lg border border-border bg-background shadow-2xl" | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| {/* Header */} | ||
| <div className="flex items-center justify-between border-border border-b px-4 py-2"> | ||
| <div className="flex items-center gap-3"> | ||
| {ownerUsername && ( | ||
| <span className="font-medium text-sm">{ownerUsername}</span> | ||
| )} | ||
| {!isOwner && ( | ||
| <span className="rounded bg-muted px-2 py-0.5 text-muted-foreground text-xs"> | ||
| View only | ||
| </span> | ||
| )} | ||
| </div> | ||
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| className="rounded p-1 text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" | ||
| > | ||
| <X className="h-5 w-5" /> | ||
| </button> | ||
| </div> | ||
|
|
||
| {/* Content */} | ||
| <div className="flex-1 overflow-hidden">{children}</div> | ||
| </div> | ||
| </div>, | ||
| document.body | ||
| ) |
There was a problem hiding this comment.
Add focus trap and ARIA attributes for accessibility.
The overlay is missing critical accessibility features:
- Focus trap: Keyboard users can tab out of the overlay into background content, breaking the modal interaction pattern.
- ARIA attributes: Missing
role="dialog",aria-modal="true", andaria-labelledbyto properly announce the modal to screen readers.
🔎 Recommended accessibility improvements
Consider using a library like react-focus-lock or implementing a focus trap manually. At minimum, add ARIA attributes:
return createPortal(
<div
className="fixed inset-0 z-50 flex items-center justify-center bg-black/80"
onClick={onClose}
+ role="dialog"
+ aria-modal="true"
>
<div
className="relative flex h-[90vh] w-[90vw] flex-col overflow-hidden rounded-lg border border-border bg-background shadow-2xl"
onClick={(e) => e.stopPropagation()}
+ role="document"
>For focus management, consider:
- Focus the overlay when it opens
- Trap focus within the overlay
- Restore focus to the trigger element when closed
As per coding guidelines: accessibility blockers that prevent task completion must be addressed.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/routes/arena/-components/focus-overlay.tsx around lines 43 to 78, the
modal lacks accessibility features: add role="dialog", aria-modal="true" and
aria-labelledby (pointing at the modal header title element) to the container
element; implement a focus trap (use react-focus-lock wrapping the modal content
or add keyboard sentinels + JS to keep focus inside) and ensure the modal
receives initial focus when opened and that focus is restored to the trigger
when closed; ensure the header title has an id to satisfy aria-labelledby and
stopPropagation remains on inner clicks.
| <input | ||
| ref={inputRef} | ||
| type="text" | ||
| value={input} | ||
| onChange={handleInput} | ||
| disabled={isFinished} | ||
| className="absolute opacity-0" | ||
| autoComplete="off" | ||
| autoCorrect="off" | ||
| autoCapitalize="off" | ||
| spellCheck={false} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Hidden input uses absolute positioning without a positioned ancestor.
The input has className="absolute opacity-0" but its parent container doesn't have relative positioning. This could cause the input to position relative to an unexpected ancestor, potentially causing layout issues or accessibility problems with screen readers.
🔎 Proposed fix
- <div className="flex-1 rounded-lg border border-border bg-muted/20 p-6">
+ <div className="relative flex-1 rounded-lg border border-border bg-muted/20 p-6">Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/routes/arena/-typing/arena.tsx around lines 133 to 145, the hidden input
is absolutely positioned but its parent container is not positioned, which can
cause it to be placed relative to an unexpected ancestor; fix by giving the
immediate parent wrapper a positioned context (add className="relative" to that
container) so the absolute input is anchored correctly — alternatively, if you
prefer not to use absolute positioning, remove "absolute" and use a
visually-hidden approach (e.g., "sr-only") so the input remains accessible and
does not rely on layout positioning.
| const progressPercent = progress | ||
| ? Math.round((progress.charIndex / totalChars) * 100) | ||
| : 0 |
There was a problem hiding this comment.
Potential division by zero if totalChars is 0.
If totalChars is 0 (empty prompt), the division progress.charIndex / totalChars will produce Infinity or NaN, which could cause unexpected UI behavior.
🔎 Proposed fix
const progressPercent = progress
- ? Math.round((progress.charIndex / totalChars) * 100)
+ ? totalChars > 0
+ ? Math.round((progress.charIndex / totalChars) * 100)
+ : 0
: 0📝 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.
| const progressPercent = progress | |
| ? Math.round((progress.charIndex / totalChars) * 100) | |
| : 0 | |
| const progressPercent = progress | |
| ? totalChars > 0 | |
| ? Math.round((progress.charIndex / totalChars) * 100) | |
| : 0 | |
| : 0 |
🤖 Prompt for AI Agents
In src/routes/arena/-typing/wpm-display.tsx around lines 45 to 47, the
progressPercent calculation divides by totalChars which may be 0; guard against
division by zero by checking totalChars > 0 before performing the division,
compute progressPercent as 0 when totalChars is 0, and clamp the final
percentage to the 0–100 range (also handle potential NaN) so the UI never
receives Infinity/NaN.
1 participant
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.