Adding Chrome Extension #6
Conversation
|
Visit the preview URL for this PR (updated for commit b9f7714): https://my-productivity-hub-5a3ba--pr6-refactor-architectur-4kp75zq8.web.app (expires Fri, 24 Apr 2026 13:33:21 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 4ef75aef15076a8cc05555b91f31935d9a13db8e |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Chrome side panel extension for ProdHub, adding a manifest, background worker, and extension-specific authentication. The UI has been refactored for responsiveness with mobile-friendly headers, sidebar toggles, and compact views for tasks and habits. Feedback highlights the need to narrow broad extension permissions for security, replace fragile hardcoded layout values, and fix a potential timezone bug in date parsing. Additionally, the code should handle Firestore batch limits during habit deletions and simplify redundant error checks in the AI service retry logic.
| "host_permissions": [ | ||
| "<all_urls>" | ||
| ], |
There was a problem hiding this comment.
The use of <all_urls> in host_permissions is extremely broad and generally discouraged unless the extension specifically needs to interact with any website the user visits. For a productivity hub that primarily interacts with Firebase and Google APIs, it is a security best practice to narrow these permissions to specific domains (e.g., https://*.firebaseio.com/*, https://generativelanguage.googleapis.com/*). Broad permissions can also lead to longer review times or rejection during the Chrome Web Store publishing process.
| </div> | ||
|
|
||
| {/* Sidebar Container */} | ||
| <div className={`${isSidebarOpen ? 'block' : 'hidden'} md:block md:w-64 flex-shrink-0 h-[calc(100vh-73px)] md:h-screen w-full`}> |
There was a problem hiding this comment.
Using a hardcoded magic number like 73px for layout calculations is fragile. This value likely represents the height of the mobile header, but if that header's content, padding, or font size changes, this calculation will break. Consider using a more robust CSS layout strategy, such as making the parent container a flexbox column and letting the sidebar container take the remaining height with flex-1.
| <span className={`flex-1 truncate ${task.completed ? 'line-through text-gray-600' : 'text-gray-300'}`}>{task.title}</span> | ||
| <div className="flex items-center gap-1.5 flex-shrink-0 text-gray-500"> | ||
| {task.priority && <div className={`w-1.5 h-1.5 rounded-full ${priorityDot[task.priority]}`} />} | ||
| {task.dueDate && <span>{formatDate(new Date(task.dueDate))}</span>} |
There was a problem hiding this comment.
Passing a string in YYYY-MM-DD format directly to new Date() results in a Date object interpreted as midnight UTC. When formatted using local time utilities (like formatDate likely does), this often causes the date to appear as the previous day for users in western timezones (e.g., UTC-5). To ensure the date is treated as local time, append a time component or use a more robust parsing method.
| {task.dueDate && <span>{formatDate(new Date(task.dueDate))}</span>} | |
| {task.dueDate && <span>{formatDate(new Date(task.dueDate + 'T00:00:00'))}</span>} |
| const batch = writeBatch(db); | ||
| snapshot.forEach((d) => batch.delete(d.ref)); | ||
| await batch.commit(); | ||
| await deleteDoc(doc(db, `artifacts/${appId}/users/${userId}/habits`, habit.id)); |
There was a problem hiding this comment.
Firestore's writeBatch has a strict limit of 500 operations per batch. Since a habit tracker can accumulate hundreds of entries over time, deleting a habit and all its history might exceed this limit and cause the operation to fail. Consider chunking the deletions if the number of entries is unknown or potentially large.
| if (error.message.includes("Rate limit") || error.message.includes("Service unavailable")) { | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
This check is redundant and potentially problematic. If a non-retryable error (like a 400 Bad Request) occurs and the error message happens to contain the string 'Rate limit', the function will throw immediately and stop retrying, even if retries < maxRetries. The retry logic for 429 and 503 status codes is already handled in the try block. It is better to let the catch block focus specifically on network-level exceptions like TypeError.
1 participant
No description provided.