Skip to content

Paused wave#2223

Merged
simo6529 merged 7 commits intomainfrom
paused-wave
Apr 8, 2026
Merged

Paused wave#2223
simo6529 merged 7 commits intomainfrom
paused-wave

Conversation

@simo6529
Copy link
Copy Markdown
Collaborator

@simo6529 simo6529 commented Apr 7, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved paused-state messaging in the timeline header: now shows "Next decision after [date]" based on the actual next-decision time, and displays "No decision scheduled" when no upcoming decision exists.
  • Tests

    • Updated and added tests to verify paused behavior and deterministic date output for the timeline header.

simo6529 added 3 commits April 7, 2026 10:27
Signed-off-by: Simo <simo@6529.io>
Signed-off-by: Simo <simo@6529.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

Removed useWave from TimelineToggleHeader and its wave prop; component now relies on nextDecisionTime (strict number check) for paused/non-paused messaging. Tests updated: mocks restored after each test and new cases added for paused banner behavior and "No decision scheduled".

Changes

Cohort / File(s) Summary
Component Implementation
components/waves/leaderboard/time/TimelineToggleHeader.tsx
Removed useWave hook and wave prop. Changed hasNextDecision to typeof nextDecisionTime === "number". Simplified paused-state rendering to use nextDecisionTime (show Next decision after <date>) or No decision scheduled when absent; adjusted JSX className/layout strings.
Test Updates
__tests__/components/waves/leaderboard/time/TimelineToggleHeader.test.tsx
Removed useWave mock, kept calculateTimeLeft and fetchUrl mocks, added afterEach to restore mocks, reformatted an existing test, and added two tests: (1) paused with nextDecisionTime uses that value (mocked toLocaleDateString), (2) paused with nextDecisionTime={null} shows No decision scheduled.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Disable chat fix #1641: Edits the same TimelineToggleHeader component and adjusts header rendering/props, likely overlapping with these JSX/prop changes.

Suggested reviewers

  • ragnep
  • prxt6529

Poem

🐰 I hopped in tests and trimmed a hook,
Next decision now leads the book.
No more waves to look behind,
Dates are clear and neatly lined.
Hooray — the header's sleek and brisk!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Paused wave' is vague and generic, lacking specificity about the actual changes made to the codebase. Consider a more descriptive title that captures the main change, such as 'Update TimelineToggleHeader to use nextDecisionTime for paused state messaging' or 'Refactor paused wave messaging logic in TimelineToggleHeader'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch paused-wave

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
components/waves/leaderboard/time/TimelineToggleHeader.tsx (1)

15-27: Remove unused wave prop from interface.

The wave prop is declared in the interface (Line 15) but not destructured or used in the component body (Line 27). Since the useWave hook and related waveData logic were removed, this prop appears to be leftover.

Note: The parent WaveLeaderboardTime.tsx still passes wave={wave} (per context snippet 1), so both the interface and the call site should be cleaned up together.

♻️ Suggested fix

In TimelineToggleHeader.tsx:

 interface TimelineToggleHeaderProps {
   readonly isOpen: boolean;
   readonly setIsOpen: (isOpen: boolean) => void;
   readonly nextDecisionTime: number | null;
   readonly isPaused?: boolean | undefined;
   readonly currentPause?: ApiWaveDecisionPause | null | undefined;
-  readonly wave?: ApiWave | undefined;
 }

And remove the unused import:

-import type { ApiWave } from "@/generated/models/ApiWave";

In WaveLeaderboardTime.tsx, remove the prop from the call site:

 <TimelineToggleHeader
   isOpen={isDecisionDetailsOpen}
   setIsOpen={handleDecisionDetailsOpenChange}
   nextDecisionTime={nextDecisionTime}
   isPaused={Boolean(currentPause)}
   currentPause={currentPause}
-  wave={wave}
 />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/waves/leaderboard/time/TimelineToggleHeader.tsx` around lines 15 -
27, The TimelineToggleHeaderProps interface still declares the unused wave?:
ApiWave prop while TimelineToggleHeader does not destructure or use it; remove
the wave property from the TimelineToggleHeaderProps interface and delete the
now-unused ApiWave import, and update the parent call site WaveLeaderboardTime
where it passes wave={wave} to stop passing that prop; ensure the prop type and
call site are kept in sync so no unused prop or import remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@components/waves/leaderboard/time/TimelineToggleHeader.tsx`:
- Around line 15-27: The TimelineToggleHeaderProps interface still declares the
unused wave?: ApiWave prop while TimelineToggleHeader does not destructure or
use it; remove the wave property from the TimelineToggleHeaderProps interface
and delete the now-unused ApiWave import, and update the parent call site
WaveLeaderboardTime where it passes wave={wave} to stop passing that prop;
ensure the prop type and call site are kept in sync so no unused prop or import
remains.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6dcdab58-55cc-4911-89e9-65677c65e895

📥 Commits

Reviewing files that changed from the base of the PR and between 8c7f46a and 81342bf.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • __tests__/components/waves/leaderboard/time/TimelineToggleHeader.test.tsx
  • components/waves/leaderboard/time/TimelineToggleHeader.tsx

Signed-off-by: Simo <simo@6529.io>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/waves/leaderboard/time/TimelineToggleHeader.tsx (1)

8-14: ⚠️ Potential issue | 🟠 Major

Prop contract changed, but caller still passes removed wave prop.
TimelineToggleHeaderProps no longer accepts wave, but components/waves/leaderboard/WaveLeaderboardTime.tsx (Line 157 in the provided snippet) still passes wave={wave}. Please update that call site to match the new interface.

Caller-side cleanup
   <TimelineToggleHeader
     isOpen={isDecisionDetailsOpen}
     setIsOpen={handleDecisionDetailsOpenChange}
     nextDecisionTime={nextDecisionTime}
     isPaused={Boolean(currentPause)}
     currentPause={currentPause}
-    wave={wave}
   />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/waves/leaderboard/time/TimelineToggleHeader.tsx` around lines 8 -
14, The TimelineToggleHeaderProps type no longer includes a wave prop but the
caller WaveLeaderboardTime component is still passing wave={wave}; update the
usage of the TimelineToggleHeader component in WaveLeaderboardTime.tsx (the JSX
where TimelineToggleHeader is rendered) to remove the wave={wave} prop and any
related prop spread that supplies wave, and ensure any data the header needs is
passed via the remaining props isOpen, setIsOpen, nextDecisionTime, isPaused, or
currentPause instead.
🧹 Nitpick comments (1)
components/waves/leaderboard/time/TimelineToggleHeader.tsx (1)

87-90: Set explicit button type to avoid accidental form submit behavior.
Consider adding type="button" for safer embedding in form contexts.

Suggested fix
         <button
+          type="button"
           className="tw-flex tw-h-6 tw-w-6 tw-flex-shrink-0 tw-items-center tw-justify-center tw-rounded-md tw-border tw-border-solid tw-border-iron-600/40 tw-bg-iron-700/50 tw-transition-all tw-duration-300 tw-ease-out desktop-hover:hover:tw-border-iron-500/50 desktop-hover:hover:tw-bg-iron-600/60"
           aria-label={isOpen ? "Collapse" : "Expand"}
         >
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/waves/leaderboard/time/TimelineToggleHeader.tsx` around lines 87 -
90, The toggle button in TimelineToggleHeader (the <button> rendered where
aria-label uses isOpen) lacks an explicit type which can cause it to act as a
submit button inside forms; add type="button" to that button element in
TimelineToggleHeader.tsx to prevent accidental form submissions while preserving
existing aria-label and className props.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/waves/leaderboard/time/TimelineToggleHeader.tsx`:
- Around line 30-31: The paused-UI branch in TimelineToggleHeader is incorrectly
gated by currentPause causing paused state (determined by isPaused and
nextDecisionTime) to fall through; update the conditional that renders the
paused UI so it only checks isPaused (and optionally nextDecisionTime) rather
than requiring currentPause, i.e., change the if that references isPaused &&
currentPause to use isPaused (and use nextDecisionTime for display logic) so
paused rendering appears whenever isPaused is true regardless of currentPause.

---

Outside diff comments:
In `@components/waves/leaderboard/time/TimelineToggleHeader.tsx`:
- Around line 8-14: The TimelineToggleHeaderProps type no longer includes a wave
prop but the caller WaveLeaderboardTime component is still passing wave={wave};
update the usage of the TimelineToggleHeader component in
WaveLeaderboardTime.tsx (the JSX where TimelineToggleHeader is rendered) to
remove the wave={wave} prop and any related prop spread that supplies wave, and
ensure any data the header needs is passed via the remaining props isOpen,
setIsOpen, nextDecisionTime, isPaused, or currentPause instead.

---

Nitpick comments:
In `@components/waves/leaderboard/time/TimelineToggleHeader.tsx`:
- Around line 87-90: The toggle button in TimelineToggleHeader (the <button>
rendered where aria-label uses isOpen) lacks an explicit type which can cause it
to act as a submit button inside forms; add type="button" to that button element
in TimelineToggleHeader.tsx to prevent accidental form submissions while
preserving existing aria-label and className props.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: eb86354f-913d-4948-8092-a2d2778e9542

📥 Commits

Reviewing files that changed from the base of the PR and between 81342bf and 1e64fdb.

📒 Files selected for processing (1)
  • components/waves/leaderboard/time/TimelineToggleHeader.tsx

Comment thread components/waves/leaderboard/time/TimelineToggleHeader.tsx
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 8, 2026

@simo6529 simo6529 merged commit 1694306 into main Apr 8, 2026
8 checks passed
@simo6529 simo6529 deleted the paused-wave branch April 8, 2026 11:26
@coderabbitai coderabbitai Bot mentioned this pull request Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants