fix(web): timeline-bug-fix #2087
Conversation
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
WalkthroughRefactors Timeline.tsx to call useTimeline(dispute, currentPeriodIndex) and update the hook to accept only currentPeriodIndex; standardizes titles, switches countdown and period-state checks to currentPeriodIndex, and conditionally includes the "Commit" period via filtering when hiddenVotes is true. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Timeline Component
participant Hook as useTimeline
participant Data as Dispute
UI->>Hook: useTimeline(dispute, currentPeriodIndex)
Hook->>Data: read periods, hiddenVotes
Hook-->>UI: computed titles + visible periods (filter Commit if !hiddenVotes)
UI->>UI: render periods and countdowns based on currentPeriodIndex
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
web/src/pages/Cases/CaseDetails/Timeline.tsx (2)
85-91: Nit: fix typo in comment (“loosing” → “losing”)Minor spelling fix to keep comments clean.
Apply this diff:
- // only show if loosing option was funded and winner needs funding, else no action is needed from user + // only show if losing option was funded and winner needs funding, else no action is needed from user
106-110: Optional: lift titles to a module-level constanttitles is static and recreated on every render. Moving it to a module-level constant avoids re-allocations and clarifies intent.
You can add this at the top-level (outside the hook):
const TIMELINE_TITLES = ["Evidence", "Commit", "Voting", "Appeal", "Executed"] as const;Then inside the hook:
- const titles = ["Evidence", "Commit", "Voting", "Appeal", "Executed"]; + const titles = TIMELINE_TITLES;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
web/src/pages/Cases/CaseDetails/Timeline.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/src/pages/Cases/CaseDetails/Timeline.tsx (2)
web/src/utils/date.ts (1)
secondsToDayHourMinute(10-16)web/src/components/StyledSkeleton.tsx (1)
StyledSkeleton(8-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
web/src/pages/Cases/CaseDetails/Timeline.tsx (1)
71-71: LGTM: Hook usage aligns with simplified signatureSwitching to useTimeline(dispute, currentPeriodIndex) matches the new hook contract and removes the previous coupling to currentItemIndex. This simplifies the data flow and avoids ambiguity between item and period indices.
| const getSubitems = (index: number): string[] | React.ReactNode[] => { | ||
| if (typeof countdown !== "undefined" && dispute) { | ||
| if (index === titles.length - 1) { | ||
| return []; | ||
| } else if (index === currentItemIndex && countdown === 0) { | ||
| } else if (index === currentPeriodIndex && countdown === 0) { | ||
| return ["Time's up!"]; | ||
| } else if (index < currentItemIndex) { | ||
| } else if (index < currentPeriodIndex) { | ||
| return []; | ||
| } else if (index === currentItemIndex) { | ||
| } else if (index === currentPeriodIndex) { | ||
| return [secondsToDayHourMinute(countdown)]; | ||
| } else { | ||
| return [secondsToDayHourMinute(dispute?.court.timesPerPeriod[index])]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix type mismatch and potential NaN in future-period subitems
secondsToDayHourMinute expects a number, but dispute?.court.timesPerPeriod[index] is typed as string | undefined. This can produce type errors and NaN at runtime when values are undefined or non-numeric. Parse the value and provide a safe default.
Apply this diff:
- } else {
- return [secondsToDayHourMinute(dispute?.court.timesPerPeriod[index])];
- }
+ } else {
+ const raw = dispute?.court.timesPerPeriod?.[index];
+ const periodSeconds = Number(raw ?? 0);
+ return [secondsToDayHourMinute(periodSeconds)];
+ }📝 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 getSubitems = (index: number): string[] | React.ReactNode[] => { | |
| if (typeof countdown !== "undefined" && dispute) { | |
| if (index === titles.length - 1) { | |
| return []; | |
| } else if (index === currentItemIndex && countdown === 0) { | |
| } else if (index === currentPeriodIndex && countdown === 0) { | |
| return ["Time's up!"]; | |
| } else if (index < currentItemIndex) { | |
| } else if (index < currentPeriodIndex) { | |
| return []; | |
| } else if (index === currentItemIndex) { | |
| } else if (index === currentPeriodIndex) { | |
| return [secondsToDayHourMinute(countdown)]; | |
| } else { | |
| return [secondsToDayHourMinute(dispute?.court.timesPerPeriod[index])]; | |
| } | |
| } else if (index === currentPeriodIndex) { | |
| return [secondsToDayHourMinute(countdown)]; | |
| } else { | |
| - return [secondsToDayHourMinute(dispute?.court.timesPerPeriod[index])]; | |
| + const raw = dispute?.court.timesPerPeriod?.[index]; | |
| + const periodSeconds = Number(raw ?? 0); | |
| + return [secondsToDayHourMinute(periodSeconds)]; | |
| } |
🤖 Prompt for AI Agents
In web/src/pages/Cases/CaseDetails/Timeline.tsx around lines 117 to 129,
secondsToDayHourMinute is being passed dispute?.court.timesPerPeriod[index]
which is typed string | undefined causing type errors and possible NaN; convert
the value to a number safely and provide a fallback before calling
secondsToDayHourMinute (e.g. const secs =
Number(dispute?.court.timesPerPeriod[index]); if (!Number.isFinite(secs) ||
isNaN(secs)) use 0 or another sensible default) and pass secs to
secondsToDayHourMinute so the function always receives a valid number.
| } else if (index === currentPeriodIndex && countdown === 0) { | ||
| return ["Time's up!"]; | ||
| } else if (index < currentItemIndex) { | ||
| } else if (index < currentPeriodIndex) { | ||
| return []; | ||
| } else if (index === currentItemIndex) { | ||
| } else if (index === currentPeriodIndex) { | ||
| return [secondsToDayHourMinute(countdown)]; | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clamp “Time’s up!” condition to countdown <= 0 to avoid negative durations
If useCountdown ever returns negative values after the deadline, the UI could show negative durations instead of “Time’s up!”. Using <= 0 is safer and avoids transient negative subitems.
Apply this diff:
- } else if (index === currentPeriodIndex && countdown === 0) {
+ } else if (index === currentPeriodIndex && countdown <= 0) {
return ["Time's up!"];📝 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.
| } else if (index === currentPeriodIndex && countdown === 0) { | |
| return ["Time's up!"]; | |
| } else if (index < currentItemIndex) { | |
| } else if (index < currentPeriodIndex) { | |
| return []; | |
| } else if (index === currentItemIndex) { | |
| } else if (index === currentPeriodIndex) { | |
| return [secondsToDayHourMinute(countdown)]; | |
| } else { | |
| } else if (index === currentPeriodIndex && countdown <= 0) { | |
| return ["Time's up!"]; | |
| } else if (index < currentPeriodIndex) { | |
| return []; | |
| } else if (index === currentPeriodIndex) { | |
| return [secondsToDayHourMinute(countdown)]; | |
| } else { |
🤖 Prompt for AI Agents
In web/src/pages/Cases/CaseDetails/Timeline.tsx around lines 121 to 127, the
condition that shows "Time's up!" checks countdown === 0 which can allow
negative durations to render; change the condition to use countdown <= 0 so any
non-positive countdown displays "Time's up!" instead of a negative time string,
keeping the rest of the branch order intact.
|
PR-Codex overview
This PR focuses on refactoring the
useTimelinefunction in theTimeline.tsxfile to streamline the handling of timeline items based on the current period index instead of the current item index, improving clarity and functionality.Detailed summary
useTimelineto accept onlycurrentPeriodIndex.getSubitemsto usecurrentPeriodIndexinstead ofcurrentItemIndex.titlesarray to always include "Commit" unless hidden votes are present.titlesto useflatMapfor cleaner handling of conditional items.Summary by CodeRabbit
Bug Fixes
Refactor