Fix include done switch bug and add shimmer for table#160
Fix include done switch bug and add shimmer for table#160iamitprakash merged 3 commits intoRealDevSquad:developfrom
Conversation
- add shimmer only for table when switch is toggled
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThe changes extend several dashboard and todo table components to support a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dashboard
participant TodoListTable
participant TodoListTableBody
User->>Dashboard: Loads dashboard or toggles "Include Done"
Dashboard->>TodoListTable: Passes tasks, isPlaceholderData, includeDone
TodoListTable->>TodoListTableBody: Passes isLoading, isPlaceholderData
alt isLoading or isPlaceholderData
TodoListTableBody->>User: Show shimmer rows
else
TodoListTableBody->>User: Show actual task rows
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
| Development logging in production code ▹ view | ✅ Fix detected | |
| Unexplained Magic Number ▹ view | ||
| Debug Log in Production ▹ view | ✅ Fix detected | |
| Incomplete URL Status Parameter Management ▹ view | ✅ Fix detected | |
| Incorrect Error Handling for Invalid URL Parameters ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| modules/dashboard/components/dashboard-watchlist-table.tsx | ✅ |
| modules/dashboard/components/dashboard-tabs.tsx | ✅ |
| modules/dashboard/index.tsx | ✅ |
| components/todo-list-table.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| if (isError || isInvalidCombination) { | ||
| return <CommonPageError /> | ||
| } |
There was a problem hiding this comment.
Incorrect Error Handling for Invalid URL Parameters 
Tell me more
What is the issue?
Using CommonPageError for an invalid combination of URL parameters is inappropriate as it suggests a system error rather than a user navigation error.
Why this matters
Users encountering this error won't understand why they're seeing a generic error page when they've simply accessed an invalid URL combination. This could lead to confusion and unnecessary support tickets.
Suggested change ∙ Feature Preview
Create a specific error component for invalid URL parameters or redirect to a valid state:
if (isError) {
return <CommonPageError />
}
if (isInvalidCombination) {
return <div>Cannot view completed tasks in Watchlist view</div>
// Or redirect to a valid state:
// router.replace('/dashboard?tab=watchlist')
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/todo-list-table.tsx(5 hunks)modules/dashboard/components/dashboard-tabs.tsx(3 hunks)modules/dashboard/components/dashboard-watchlist-table.tsx(2 hunks)modules/dashboard/index.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: shubhdevelop
PR: Real-Dev-Squad/todo-frontend#35
File: components/SimmerSkeleton.tsx:0-0
Timestamp: 2025-03-16T10:34:22.088Z
Learning: The shimmer component in the todo app should maintain the app's color palette (white, indigo-50) without making colors configurable, as it's designed specifically for this application.
modules/dashboard/components/dashboard-watchlist-table.tsx (1)
Learnt from: yesyash
PR: #109
File: app/(internal-routes)/teams/[teamId]/layout.tsx:8-15
Timestamp: 2025-07-16T13:18:36.847Z
Learning: In Next.js layout components, error handling and validation for params extraction (like const { teamId } = await params) is not required in this codebase. The framework handles routing validation adequately.
components/todo-list-table.tsx (1)
Learnt from: shubhdevelop
PR: #35
File: components/SimmerSkeleton.tsx:0-0
Timestamp: 2025-03-16T10:34:22.088Z
Learning: The shimmer component in the todo app should maintain the app's color palette (white, indigo-50) without making colors configurable, as it's designed specifically for this application.
modules/dashboard/index.tsx (1)
Learnt from: yesyash
PR: #109
File: app/(internal-routes)/teams/[teamId]/layout.tsx:8-15
Timestamp: 2025-07-16T13:18:36.847Z
Learning: In Next.js layout components, error handling and validation for params extraction (like const { teamId } = await params) is not required in this codebase. The framework handles routing validation adequately.
🔇 Additional comments (14)
modules/dashboard/components/dashboard-watchlist-table.tsx (3)
1-1: LGTM: Correct client directive usage.The 'use client' directive is properly placed to enable client-side hooks usage.
7-8: LGTM: Proper Next.js navigation imports.The imports are correctly added to support URL parameter reading functionality.
30-32: LGTM: Proper error handling for invalid parameter combinations.The updated condition correctly handles both API errors and invalid URL parameter combinations, which aligns with the PR objective to fix the Include Done switch behavior.
modules/dashboard/components/dashboard-tabs.tsx (3)
15-15: LGTM: Proper prop interface extension.The
isPlaceholderDataprop is correctly added to the component interface and destructuring, enabling shimmer control functionality.Also applies to: 23-23
34-41: LGTM: Well-implemented URL parameter synchronization.The enhanced tab change logic properly manages URL parameters:
- Removes
statusparameter when switching to WatchList tab- Sets
statusto 'Done' for other tabs whenincludeDoneis true- Maintains clean URL state management
This correctly implements the PR objective to fix Include Done switch behavior.
66-66: LGTM: Proper prop propagation for shimmer control.The
isPlaceholderDataprop is correctly passed toTodoListTableto enable table-level shimmer loading control.modules/dashboard/index.tsx (5)
7-9: LGTM: Proper imports for enhanced functionality.The imports correctly support URL parameter management, placeholder data handling, and React hooks usage.
16-22: LGTM: Proper URL-driven state initialization.The implementation correctly:
- Reads URL parameters to initialize state
- Uses useRef to track first load for shimmer behavior
- Initializes
includeDoneTasksbased on URLstatusparameterThis aligns with the PR objective to fix Include Done switch rendering.
23-33: LGTM: Well-implemented bidirectional URL synchronization.The change handler properly:
- Updates local state
- Synchronizes URL parameters with state changes
- Maintains clean URL parameter management
This ensures consistent state between URL and component state.
48-50: LGTM: Correct loading condition for first load shimmer.The condition properly shows full-page shimmer only on the first load, implementing the PR objective to improve shimmer behavior for better UX.
67-69: LGTM: Proper prop passing for enhanced functionality.The props are correctly passed to enable:
- Table-level shimmer control via
isPlaceholderData- Bidirectional state management via the updated change handler
components/todo-list-table.tsx (3)
88-88: LGTM: Proper shimmer control implementation.The
isPlaceholderDataprop is correctly added to the interface and the shimmer rendering logic properly includes both loading and placeholder data states. This enables table-level shimmer during data refetches as intended.Also applies to: 95-98
146-146: LGTM: Correct prop propagation through component hierarchy.The
isPlaceholderDataprop is properly:
- Added to the
TodoListTableinterface- Destructured in the component
- Passed down to
TodoListTableBodyThis enables the shimmer control functionality throughout the table component tree.
Also applies to: 155-155, 219-219
199-199: LGTM: Fixed Include Done switch visibility condition.The condition change from
currentTab === DashboardTasksTableTabs.AlltocurrentTab !== DashboardTasksTableTabs.WatchListcorrectly implements the PR objective to fix the Include Done switch bug. The switch now appears on all tabs except WatchList, which aligns with the URL parameter logic implemented in other components.
Date: 26 Jul 2025
Developer Name: @Hariom01010
Issue Ticket Number
Closes #159
Description
This PR addresses two key issues on the Dashboard:
Fix: "Include Done" switch not rendered on initial load
Previously, the switch to include completed tasks was not rendered due to an incorrect condition check during the initial load. This has now been fixed to ensure the toggle always appears as expected.
Improved: Shimmer loading behavior
On initial page load, the full-page shimmer is displayed as intended.
On subsequent data refetches (e.g., when toggling the switch), only the table area shows the shimmer. This avoids unnecessary UI flicker and improves the perceived performance and UX.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
include-done-bug-fix.mp4
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Fix the bug related to the "include done" switch in the To-Do list and add a shimmer loading effect for the table UI for better user experience during data loading.
Why are these changes being made?
The bug was causing incorrect status handling when switching tasks views, and the shimmer effect allows users to perceive loading activity, enhancing user experience by indicating that content is being loaded asynchronously. This approach improves UI responsiveness and addresses specific conditions where task views were not properly updating due to faulty status management.