270 main dashboard makeover#272
Conversation
… 270-main-dashboard-makeover
…nityBowman/corates into 270-main-dashboard-makeover
…nityBowman/corates into 270-main-dashboard-makeover
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | e015681 | Commit Preview URL | Jan 10 2026, 07:22 PM |
|
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 📝 WalkthroughWalkthroughImplements a comprehensive dashboard redesign with modular component architecture, adding 12+ new dashboard components, routing updates, project stats persistence, and three settings design mocks. Includes planning documents for the redesign and study counts architecture, alongside refactored Dashboard structure, enhanced project store with caching, and improved auth loading state. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dashboard as Dashboard Container
participant useDashboardData as useDashboardData Hook
participant Auth as Auth Store
participant Subscription as Subscription API
participant ProjectStore as Project Store
participant Components as Dashboard Components
User->>Dashboard: Load Dashboard
Dashboard->>useDashboardData: Derive dashboard state
useDashboardData->>Auth: Get auth status & user
useDashboardData->>Subscription: Fetch subscription/quotas
useDashboardData->>ProjectStore: Get projects & cached stats
Subscription-->>useDashboardData: Return subscription data
Auth-->>useDashboardData: Return auth status
ProjectStore-->>useDashboardData: Return projects + stats
useDashboardData->>useDashboardData: Compute stats (projectCount,<br/>completedStudies, etc.)
useDashboardData-->>Dashboard: Return derived state
Dashboard->>Dashboard: Setup AnimationContext
Dashboard->>Components: Render conditional layout<br/>(header, stats, projects,<br/>activity, progress)
Components->>Components: Apply animations from<br/>AnimationContext
Components-->>User: Display dashboard UI
sequenceDiagram
participant ProjectStore as Project Store
participant Frontend as Frontend (setProjectData)
participant LocalStorage as localStorage
participant Components as Dashboard Components
Frontend->>ProjectStore: Update project data (studies)
ProjectStore->>ProjectStore: computeProjectStats()<br/>(studyCount,<br/>completedCount)
ProjectStore->>ProjectStore: Update projectStats[projectId]<br/>+ lastUpdated
ProjectStore->>LocalStorage: persistStats(projectStats)
LocalStorage-->>ProjectStore: Persisted ✓
ProjectStore-->>Frontend: Data updated
Components->>ProjectStore: getProjectStats(projectId)
ProjectStore-->>Components: Return cached stats
Components->>Components: Render stats in<br/>ProgressCard/ProjectCard
Components-->>User: Display updated counts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
🤖 Fix all issues with AI agents
In @packages/web/src/components/checklist/CreateLocalChecklist.jsx:
- Around line 29-30: The render-time console.log in CreateLocalChecklist.jsx
(the line logging 'typeOptions') should be removed or guarded; either delete the
console.log or wrap it in a development-only conditional (e.g., check
process.env.NODE_ENV === 'development') or move the log into a useEffect inside
the CreateLocalChecklist component so it only runs once and not on every render;
update the reference to the variable name typeOptions and ensure any remaining
debug logging uses the dev-only guard.
In @packages/web/src/components/dashboard/Dashboard.jsx:
- Around line 82-95: The activities creation in createMemo currently slices
projectList before sorting, so newer projects beyond the first five are
excluded; fix by mapping projectList to activity objects (using project.name,
project.updatedAt/createdAt) then sorting that full list by timestamp (newest
first) and finally slicing the top 5; update the createMemo block that defines
activities to perform map -> sort -> slice in that order so the activity feed
shows the most recent items.
In @packages/web/src/components/dashboard/ProjectsSection.jsx:
- Around line 116-148: The handler handleDeleteProject is using raw fetch and
manual JSON/error parsing; replace the fetch call with the shared apiFetch
helper (call apiFetch(`/api/orgs/${project.orgId}/projects/${targetProjectId}`,
{ method: 'DELETE', credentials: 'include' })) and remove the response.ok /
manual JSON parsing and throw logic, letting apiFetch surface errors/toasts;
keep the surrounding try/catch, retain queryClient.invalidateQueries({ queryKey:
queryKeys.projects.all }) and the success toast (or remove it if apiFetch
already shows success per project conventions), and still import/await
handleError in the catch block as before.
In @packages/web/src/components/mocks/DashboardMock.jsx:
- Around line 142-146: ProgressArc computes percentage and offset using
props.total which can be 0 causing NaN/Infinity; update the percentage
calculation in ProgressArc (the percentage() helper) to guard against zero or
falsy totals (e.g., return 0 when props.total is 0 or not a finite number) and
ensure offset() uses that safe percentage value (or falls back to 100/0
appropriately); keep the fix localized to the percentage() and offset() helpers
so other behavior remains unchanged.
In @packages/web/src/Routes.jsx:
- Line 44: Remove the dead standalone string literal "('');" found in Routes.jsx
(it's a no-op leftover) — locate the Routes component or top-level JSX file
where the expression appears and delete that stray string expression so no
extraneous statements remain in the module.
🧹 Nitpick comments (19)
packages/web/src/components/checklist/CreateLocalChecklist.jsx (1)
20-23: ValidatesearchParams.typeagainstgetChecklistTypeOptions()before seeding state.Right now any arbitrary
?type=value becomes the selected value even if it’s not registered (select can end up with no matching option). Consider whitelisting againsttypeOptions.Proposed fix
const { createChecklist, savePdf } = localChecklistsStore; const [name, setName] = createSignal(''); - const [checklistType, setChecklistType] = createSignal( - searchParams.type || DEFAULT_CHECKLIST_TYPE, - ); + const typeOptions = getChecklistTypeOptions(); + const urlType = searchParams.type; + const initialType = typeOptions.some(o => o.value === urlType) + ? urlType + : DEFAULT_CHECKLIST_TYPE; + const [checklistType, setChecklistType] = createSignal(initialType); const [pdfFile, setPdfFile] = createSignal(null); const [creating, setCreating] = createSignal(false); const [error, setError] = createSignal(null); - const typeOptions = getChecklistTypeOptions(); + // typeOptions now declared above (used for initialType validation)Also applies to: 28-28
packages/web/src/components/checklist/LocalChecklistView.jsx (1)
11-12:type={searchParams.type}is currently redundant (CreateLocalChecklist ignores it).Either:
- Remove
useSearchParams+ thetypeprop here, sinceCreateLocalChecklistalready readssearchParams.typeinternally; or- Update
CreateLocalChecklistto accept atypeprop and prefer it over URL state (keeping Solid’s “no props destructuring” rule). As per coding guidelines, avoid prop-drilling shared state unless it’s truly local config.Also applies to: 22-23, 178-178
packages/web/src/api/better-auth-store.js (1)
871-871: Consider documenting the dual loading state pattern.The export now uses
isAuthLoading(cache-aware) while internal code at lines 169, 199, 225, and 269 continues to use the rawauthLoading()from the session. This dual pattern is intentional and correct:
- Internal usage (raw
authLoading()): Needed to detect actual session state for deciding when to use cached data, prevent concurrent refreshes, and avoid premature cache clearing.- External usage (
isAuthLoading): Provides cache-aware loading state to UI components, preventing loading flashes when cached data exists.However, this subtle distinction could benefit from a comment near the
isAuthLoadingdefinition (around line 178) explaining why the internal implementation still references the rawauthLoading()signal for state management decisions.📝 Suggested documentation addition
// Enhanced authLoading that accounts for cached data // If we have cached user data, we're not "loading" from UI perspective + // NOTE: Internal code still uses raw authLoading() for state decisions + // (e.g., when to use cached data, prevent concurrent refreshes). + // This function is exported for UI components to avoid loading flashes. const isAuthLoading = () => {packages/web/src/components/mocks/SettingsMockMinimal.jsx (3)
93-98: Redundant transform styling in toggleThe toggle has both a Tailwind class (
translate-x-5.5) and an inlinestylewithtranslateX. The inline style overrides the class, making the Tailwind class ineffective. Consider using only one approach.🔧 Suggested fix - use only inline style (simpler for dynamic values)
<div - class={`absolute top-0.5 h-5 w-5 rounded-full bg-white shadow-sm transition-transform duration-200 ${ - props.checked ? 'left-0.5 translate-x-5.5' : 'left-0.5' - }`} + class='absolute left-0.5 top-0.5 h-5 w-5 rounded-full bg-white shadow-sm transition-transform duration-200' style={{ transform: props.checked ? 'translateX(22px)' : 'translateX(0)' }} />
343-367: Consider usingForcomponent instead ofArray.map()Per coding guidelines, prefer Solid's
Forcomponent for rendering lists. This is a mock component so it's low priority, but worth noting for consistency.🔧 Suggested refactor
- {[ - { label: 'Projects', used: 8, limit: 15 }, - { label: 'Collaborators', used: 12, limit: 25 }, - { label: 'Storage', used: 2.4, limit: 10, suffix: 'GB' }, - ].map(stat => ( + <For each={[ + { label: 'Projects', used: 8, limit: 15 }, + { label: 'Collaborators', used: 12, limit: 25 }, + { label: 'Storage', used: 2.4, limit: 10, suffix: 'GB' }, + ]}> + {stat => ( <div class='rounded-xl border border-neutral-100 bg-neutral-50/50 p-6'> ... </div> - ))} + )} + </For>
467-521: SameArray.map()pattern - considerForcomponentSame suggestion applies here for the integrations list. As per coding guidelines, use Solid's
Forcomponent for rendering lists instead ofArray.map().packages/docs/plans/dashboard-redesign.md (1)
1-528: Well-structured dashboard redesign planThis is a comprehensive planning document that covers:
- Clear user state matrix handling all authentication/subscription scenarios
- Phased implementation approach with dependencies
- Concrete file changes aligned with actual PR implementation
- Thoughtful migration strategy using feature flags
- Success metrics and testing requirements
The plan aligns well with the dashboard components introduced in this PR.
Minor markdown improvements (optional):
- Add language specifiers to code blocks at lines 72, 187, 371, 392, 400 (e.g.,
```plaintextor```bash)- Line 452: "Logged out user" → "Logged-out user" (hyphenated compound adjective)
packages/web/src/components/mocks/SettingsMockBento.jsx (2)
10-31: Several unused icon imports.Icons
FiUser,FiMail,FiUsers,FiChevronRightare imported but not used in the component. This is minor for a mock file but worth cleaning up.🧹 Remove unused imports
import { - FiUser, FiCreditCard, FiShield, FiBell, FiLink, FiMonitor, FiMoon, FiSun, FiCheck, - FiChevronRight, FiArrowLeft, - FiMail, FiCloud, FiHardDrive, FiZap, - FiUsers, FiFolder, FiLock, FiSmartphone, FiGlobe, } from 'solid-icons/fi';
480-486: Consider preloading the external font to avoid FOUC.Loading fonts via
@importinside a component's<style>tag can cause a flash of unstyled content (FOUC). For mock pages this is acceptable, but if this pattern is used in production, consider adding a<link rel="preload">in the document head or using a font-loading strategy.packages/web/src/components/dashboard/LocalAppraisalCard.jsx (1)
22-37: ExtractformatRelativeTimeto a shared utility.This function is duplicated in
ActivityFeed.jsx,ProjectCard.jsx, and nowLocalAppraisalCard.jsx. Consider extracting it to a shared utility (e.g.,@/lib/dateUtils.js) to maintain consistency and reduce duplication.📦 Suggested shared utility
Create
packages/web/src/lib/dateUtils.js:/** * Format a relative time string * @param {Date|string|number} date * @returns {string} */ export function formatRelativeTime(date) { if (!date) return ''; const now = new Date(); const then = new Date(date); const diffMs = now - then; const diffMins = Math.floor(diffMs / 60000); const diffHours = Math.floor(diffMins / 60); const diffDays = Math.floor(diffHours / 24); if (diffMins < 1) return 'Just now'; if (diffMins < 60) return `${diffMins}m ago`; if (diffHours < 24) return `${diffHours}h ago`; if (diffDays < 7) return `${diffDays}d ago`; if (diffDays < 30) return `${Math.floor(diffDays / 7)}w ago`; return then.toLocaleDateString(); }Then import in each component:
-function formatRelativeTime(date) { ... } +import { formatRelativeTime } from '@/lib/dateUtils.js';packages/web/src/components/dashboard/ActivityFeed.jsx (2)
15-28: SameformatRelativeTimeduplication.As noted in
LocalAppraisalCard.jsx, this function should be extracted to a shared utility.
33-57: Consider memoizing icon maps or moving outside component.The
iconMapandbgMapobjects are recreated on every render ofActivityIcon. Since these are static, they could be defined at module scope for a minor performance improvement.♻️ Move static maps to module scope
+const ACTIVITY_ICONS = { + study: <FiFileText class='h-4 w-4' />, + project: <FiFolder class='h-4 w-4' />, + complete: <FiCheck class='h-4 w-4' />, + user: <FiUser class='h-4 w-4' />, + default: <FiClock class='h-4 w-4' />, +}; + +const ACTIVITY_BG = { + study: 'bg-blue-100 text-blue-600', + project: 'bg-amber-100 text-amber-600', + complete: 'bg-emerald-100 text-emerald-600', + user: 'bg-violet-100 text-violet-600', + default: 'bg-stone-100 text-stone-600', +}; + function ActivityIcon(props) { - const iconMap = { ... }; - const bgMap = { ... }; - return ( <div - class={`flex h-8 w-8 shrink-0 items-center justify-center rounded-full ${bgMap[props.type] || bgMap.default}`} + class={`flex h-8 w-8 shrink-0 items-center justify-center rounded-full ${ACTIVITY_BG[props.type] || ACTIVITY_BG.default}`} > - {iconMap[props.type] || iconMap.default} + {ACTIVITY_ICONS[props.type] || ACTIVITY_ICONS.default} </div> ); }packages/web/src/components/dashboard/QuickActions.jsx (1)
53-81: Consider memoizing the actions array.The
actions()function recreates the array and JSX icons on every access. For a small static list this is acceptable, but you could usecreateMemoor move the config to module scope for consistency with the pattern used elsewhere.♻️ Optional: memoize with createMemo
+import { For, useContext, createMemo } from 'solid-js'; export function QuickActions(props) { const animation = useContext(AnimationContext); - const actions = () => [ + const actions = createMemo(() => [ { id: 'robins-i', // ... rest of array }, - ]; + ]);packages/web/src/components/dashboard/DashboardHeader.jsx (1)
20-23: Consider usingcreateMemofor derivedfirstName.For consistency with other dashboard components that use
createMemofor derived values,firstNamecould be wrapped increateMemo. This is a minor optimization since it's a simple string operation.♻️ Optional: use createMemo
+import { Show, useContext, createMemo } from 'solid-js'; export function DashboardHeader(props) { const animation = useContext(AnimationContext); - const firstName = () => { + const firstName = createMemo(() => { const name = props.user?.name || ''; return name.split(' ')[0] || ''; - }; + });packages/docs/plans/study-counts-architecture.md (1)
197-214: Add language specifier to the fenced code block.The data flow diagram code block at line 197 is missing a language specifier, which triggers a markdownlint warning (MD040). Consider adding
textorplaintextas the language.Proposed fix
-``` +```text Dashboard Load: API returns projects (no stats or stale stats from SQL cache)packages/web/src/components/mocks/SettingsMockCombined.jsx (1)
761-784: UseForcomponent instead of.map()for rendering lists.Per coding guidelines, SolidJS components should use
Forfor rendering lists instead ofArray.map().Proposed fix
{/* Other integrations */} <div class='mt-4 space-y-3'> - {[ - { name: 'Zotero', description: 'Sync your reference library', available: false }, - { - name: 'Mendeley', - description: 'Import references and annotations', - available: false, - }, - ].map(integration => ( + <For each={[ + { name: 'Zotero', description: 'Sync your reference library', available: false }, + { name: 'Mendeley', description: 'Import references and annotations', available: false }, + ]}> + {integration => ( <div class='flex items-center justify-between rounded-xl border border-gray-200 p-5 opacity-60'> ... </div> - ))} + )} + </For> </div>packages/web/src/components/dashboard/ProjectCard.jsx (1)
79-94: DuplicateformatRelativeTimeutility across multiple files.This function is duplicated in
ActivityFeed.jsx(lines 15-28) andLocalAppraisalCard.jsx(lines 23-28). Consider extracting to a shared utility in@/lib/or@primitives/.packages/web/src/stores/projectStore.js (1)
91-101: Side effect (localStorage write) insideproduce()callback.Calling
persistStats()inside theproduce()callback works but mixes a side effect with state mutation. Consider moving the persistence call aftersetStorecompletes for clearer separation.Alternative pattern
function setProjectData(projectId, data) { + let shouldPersist = false; setStore( produce(s => { // ... existing code ... if (data.studies !== undefined) { s.projects[projectId].studies = data.studies; const stats = computeProjectStats(data.studies); s.projectStats[projectId] = { ...stats, lastUpdated: Date.now(), }; - persistStats(s.projectStats); + shouldPersist = true; } }), ); + if (shouldPersist) { + persistStats(store.projectStats); + } }packages/web/src/components/mocks/DashboardMock.jsx (1)
15-15: Unused import:createEffect.
createEffectis imported but not used in this file.Proposed fix
-import { For, Show, createSignal, createEffect, onMount } from 'solid-js'; +import { For, Show, createSignal, onMount } from 'solid-js';
| console.log('typeOptions', typeOptions); | ||
|
|
There was a problem hiding this comment.
Remove (or DEV-guard) render-time console.log.
Proposed fix
- console.log('typeOptions', typeOptions);
+ if (import.meta.env.DEV) console.log('typeOptions', typeOptions);📝 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.
| console.log('typeOptions', typeOptions); | |
| if (import.meta.env.DEV) console.log('typeOptions', typeOptions); |
🤖 Prompt for AI Agents
In @packages/web/src/components/checklist/CreateLocalChecklist.jsx around lines
29 - 30, The render-time console.log in CreateLocalChecklist.jsx (the line
logging 'typeOptions') should be removed or guarded; either delete the
console.log or wrap it in a development-only conditional (e.g., check
process.env.NODE_ENV === 'development') or move the log into a useEffect inside
the CreateLocalChecklist component so it only runs once and not on every render;
update the reference to the variable name typeOptions and ensure any remaining
debug logging uses the dev-only guard.
| // Generate recent activity from projects | ||
| const activities = createMemo(() => { | ||
| const projectList = projects() || []; | ||
| return projectList | ||
| .slice(0, 5) | ||
| .map(project => ({ | ||
| type: 'project', | ||
| title: project.name, | ||
| subtitle: 'was updated', | ||
| timestamp: project.updatedAt || project.createdAt, | ||
| })) | ||
| .sort((a, b) => new Date(b.timestamp) - new Date(a.timestamp)) | ||
| .slice(0, 5); | ||
| }); |
There was a problem hiding this comment.
Activities slice occurs before sort, potentially missing recent items.
The current logic slices the first 5 projects, maps them to activities, then sorts by timestamp. If the most recently updated projects are beyond index 4, they won't appear in the activity feed.
Proposed fix
const activities = createMemo(() => {
const projectList = projects() || [];
return projectList
- .slice(0, 5)
.map(project => ({
type: 'project',
title: project.name,
subtitle: 'was updated',
timestamp: project.updatedAt || project.createdAt,
}))
.sort((a, b) => new Date(b.timestamp) - new Date(a.timestamp))
.slice(0, 5);
});🤖 Prompt for AI Agents
In @packages/web/src/components/dashboard/Dashboard.jsx around lines 82 - 95,
The activities creation in createMemo currently slices projectList before
sorting, so newer projects beyond the first five are excluded; fix by mapping
projectList to activity objects (using project.name,
project.updatedAt/createdAt) then sorting that full list by timestamp (newest
first) and finally slicing the top 5; update the createMemo block that defines
activities to perform map -> sort -> slice in that order so the activity feed
shows the most recent items.
| const handleDeleteProject = async targetProjectId => { | ||
| const confirmed = await confirmDialog.open({ | ||
| title: 'Delete Project', | ||
| description: | ||
| 'Are you sure you want to delete this entire project? This action cannot be undone.', | ||
| confirmText: 'Delete Project', | ||
| variant: 'danger', | ||
| }); | ||
| if (!confirmed) return; | ||
|
|
||
| const project = projects()?.find(p => p.id === targetProjectId); | ||
| if (!project?.orgId) { | ||
| showToast.error('Error', 'Unable to find project organization'); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const response = await fetch( | ||
| `${API_BASE}/api/orgs/${project.orgId}/projects/${targetProjectId}`, | ||
| { method: 'DELETE', credentials: 'include' }, | ||
| ); | ||
| if (!response.ok) { | ||
| const data = await response.json().catch(() => ({})); | ||
| throw new Error(data.error || 'Failed to delete project'); | ||
| } | ||
|
|
||
| queryClient.invalidateQueries({ queryKey: queryKeys.projects.all }); | ||
| showToast.success('Project Deleted', 'The project has been deleted successfully'); | ||
| } catch (err) { | ||
| const { handleError } = await import('@/lib/error-utils.js'); | ||
| await handleError(err, { toastTitle: 'Delete Failed' }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refactor to use apiFetch instead of raw fetch.
The deletion handler uses raw fetch and manually handles JSON parsing and error responses. As per coding guidelines, all frontend API calls should use apiFetch to automatically handle JSON parsing, errors, and toast notifications.
♻️ Refactor to use apiFetch
const handleDeleteProject = async targetProjectId => {
const confirmed = await confirmDialog.open({
title: 'Delete Project',
description:
'Are you sure you want to delete this entire project? This action cannot be undone.',
confirmText: 'Delete Project',
variant: 'danger',
});
if (!confirmed) return;
const project = projects()?.find(p => p.id === targetProjectId);
if (!project?.orgId) {
showToast.error('Error', 'Unable to find project organization');
return;
}
try {
- const response = await fetch(
- `${API_BASE}/api/orgs/${project.orgId}/projects/${targetProjectId}`,
- { method: 'DELETE', credentials: 'include' },
- );
- if (!response.ok) {
- const data = await response.json().catch(() => ({}));
- throw new Error(data.error || 'Failed to delete project');
- }
+ const { apiFetch } = await import('@/lib/api-utils.js');
+ await apiFetch(`${API_BASE}/api/orgs/${project.orgId}/projects/${targetProjectId}`, {
+ method: 'DELETE',
+ });
queryClient.invalidateQueries({ queryKey: queryKeys.projects.all });
showToast.success('Project Deleted', 'The project has been deleted successfully');
} catch (err) {
const { handleError } = await import('@/lib/error-utils.js');
await handleError(err, { toastTitle: 'Delete Failed' });
}
};As per coding guidelines.
🤖 Prompt for AI Agents
In @packages/web/src/components/dashboard/ProjectsSection.jsx around lines 116 -
148, The handler handleDeleteProject is using raw fetch and manual JSON/error
parsing; replace the fetch call with the shared apiFetch helper (call
apiFetch(`/api/orgs/${project.orgId}/projects/${targetProjectId}`, { method:
'DELETE', credentials: 'include' })) and remove the response.ok / manual JSON
parsing and throw logic, letting apiFetch surface errors/toasts; keep the
surrounding try/catch, retain queryClient.invalidateQueries({ queryKey:
queryKeys.projects.all }) and the success toast (or remove it if apiFetch
already shows success per project conventions), and still import/await
handleError in the catch block as before.
| function ProgressArc(props) { | ||
| const percentage = () => Math.round((props.completed / props.total) * 100); | ||
| const circumference = 2 * Math.PI * 36; | ||
| const offset = () => circumference - (percentage() / 100) * circumference; | ||
|
|
There was a problem hiding this comment.
Potential division by zero in ProgressArc.
If props.total is 0, the percentage calculation will result in NaN or Infinity.
Proposed fix
function ProgressArc(props) {
- const percentage = () => Math.round((props.completed / props.total) * 100);
+ const percentage = () => {
+ if (!props.total || props.total === 0) return 0;
+ return Math.round((props.completed / props.total) * 100);
+ };
const circumference = 2 * Math.PI * 36;🤖 Prompt for AI Agents
In @packages/web/src/components/mocks/DashboardMock.jsx around lines 142 - 146,
ProgressArc computes percentage and offset using props.total which can be 0
causing NaN/Infinity; update the percentage calculation in ProgressArc (the
percentage() helper) to guard against zero or falsy totals (e.g., return 0 when
props.total is 0 or not a finite number) and ensure offset() uses that safe
percentage value (or falls back to 100/0 appropriately); keep the fix localized
to the percentage() and offset() helpers so other behavior remains unchanged.
| const SettingsMockMinimal = lazy(() => import('@/components/mocks/SettingsMockMinimal.jsx')); | ||
| const SettingsMockCombined = lazy(() => import('@/components/mocks/SettingsMockCombined.jsx')); | ||
| const DashboardMock = lazy(() => import('@/components/mocks/DashboardMock.jsx')); | ||
| (''); |
There was a problem hiding this comment.
Remove dead code: standalone string literal.
Line 44 contains a standalone string expression (''); which does nothing and appears to be a typo or leftover from editing.
🧹 Remove dead code
const DashboardMock = lazy(() => import('@/components/mocks/DashboardMock.jsx'));
-('');
// Code-split admin routes - loaded only when navigating to /admin/*🤖 Prompt for AI Agents
In @packages/web/src/Routes.jsx at line 44, Remove the dead standalone string
literal "('');" found in Routes.jsx (it's a no-op leftover) — locate the Routes
component or top-level JSX file where the expression appears and delete that
stray string expression so no extraneous statements remain in the module.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Performance
✏️ Tip: You can customize this high-level summary in your review settings.