feature (desktop): add border color picker for projects#625
Conversation
|
Warning Rate limit exceeded@AviPeltz has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThese changes implement project color customization. New projects default to a fixed "default" color instead of random selection. Components are updated to accept and display project colors, with a new color picker UI in the project header that allows users to select and persist custom colors via API. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.ts:
- Around line 7-9: The function assignRandomColor is misleading because it
deterministically returns PROJECT_COLOR_DEFAULT; rename it (e.g.,
getDefaultProjectColor or projectColorDefault) to reflect its behavior, update
its export in index.ts, and update all call sites to use the new name so
callers/readers aren't misled into expecting randomness.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx:
- Around line 18-33: The hexToRgba function currently assumes a valid 6-digit
hex and can emit invalid CSS; validate the input in hexToRgba by checking it
matches a full 6-digit hex pattern (e.g. /^#([0-9A-Fa-f]{6})$/) before parsing,
and if invalid return a safe fallback CSS value (like a transparent rgba or a
configured default) or throw a clear error; also tighten isCustomColor to only
treat values as custom when they both differ from PROJECT_COLOR_DEFAULT and
match the same 6-digit hex pattern so downstream code never receives malformed
colors.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx (1)
54-63: Extract magic number to a named constant.The alpha value
0.6is hardcoded. Extracting it to a named constant improves maintainability and makes the intent clearer.As per coding guidelines, extract magic numbers and hardcoded values to named constants at module top.
🔎 Proposed refactor
At the module top, add:
import { PROJECT_COLOR_DEFAULT } from "shared/constants/project-colors"; +const CUSTOM_BORDER_ALPHA = 0.6; + interface ProjectThumbnailProps {Then update the usage:
const borderStyle = hasCustomColor - ? { borderColor: hexToRgba(projectColor, 0.6) } + ? { borderColor: hexToRgba(projectColor, CUSTOM_BORDER_ALPHA) } : undefined;apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx (1)
100-106: Add query invalidation and consider optimistic updates.The
updateProjectmutation handles errors but lacks anonSuccesscallback to invalidate related queries. Users won't see immediate visual feedback, and stale data may persist in the cache.🔎 Proposed improvements
Add query invalidation on success:
const updateProject = useUpdateProject({ + onSuccess: () => { + utils.workspaces.getAllGrouped.invalidate(); + utils.projects.getRecents.invalidate(); + }, onError: (error) => toast.error(`Failed to update color: ${error.message}`), });For better UX, consider adding optimistic updates so the color changes immediately before the server responds. This would require checking if
useUpdateProjectsupports anonMutatecallback with optimistic data.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/shared/constants/project-colors.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsxapps/desktop/src/shared/constants/project-colors.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsxapps/desktop/src/shared/constants/project-colors.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsxapps/desktop/src/shared/constants/project-colors.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsxapps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsxapps/desktop/src/shared/constants/project-colors.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx (5)
packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/context-menu.tsx (4)
ContextMenuSub(248-248)ContextMenuSubTrigger(250-250)ContextMenuSubContent(249-249)ContextMenuItem(240-240)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/constants.ts (1)
STROKE_WIDTH(2-2)apps/desktop/src/shared/constants/project-colors.ts (2)
PROJECT_COLORS(4-14)PROJECT_COLOR_DEFAULT(2-2)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/lib/trpc/routers/projects/utils/colors/colors.ts (2)
apps/desktop/src/lib/trpc/routers/projects/utils/colors/index.ts (1)
assignRandomColor(1-1)apps/desktop/src/shared/constants/project-colors.ts (1)
PROJECT_COLOR_DEFAULT(2-2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx (3)
apps/desktop/src/shared/constants/project-colors.ts (1)
PROJECT_COLOR_DEFAULT(2-2)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/index.ts (1)
ProjectThumbnail(1-1)packages/ui/src/lib/utils.ts (1)
cn(4-6)
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (6)
apps/desktop/src/shared/constants/project-colors.ts (1)
1-5: LGTM!The introduction of
PROJECT_COLOR_DEFAULTas a sentinel value is well-documented and clearly distinguishes the default state from custom color selections. The Default option is appropriately positioned as the first entry in the color palette.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx (1)
66-84: LGTM!The conditional border styling for the avatar container correctly applies the custom color when available and gracefully handles image load errors.
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectHeader.tsx (2)
108-140: LGTM!The color picker submenu is well-implemented with clear visual feedback (color preview circles and checkmarks), proper keyboard navigation support via
ContextMenuSub, and consistent styling for both default and custom colors.
157-163: LGTM!The
projectColorprop is correctly threaded toProjectThumbnailin the collapsed sidebar view.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebar.tsx (1)
40-40: LGTM!The
projectColorprop is correctly threaded from the grouped data to theProjectSectioncomponent, enabling color customization to flow through the component tree.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx (1)
24-24: LGTM! Clean prop threading implementation.The
projectColorprop is correctly added to the interface, accepted in the component parameters, and consistently passed toProjectHeaderin both the collapsed and expanded render paths. This straightforward middleware implementation properly propagates the color value through the component hierarchy without introducing any logic or side effects.Also applies to: 38-38, 76-76, 128-128
| /** | ||
| * Converts a hex color to rgba with the specified alpha. | ||
| */ | ||
| function hexToRgba(hex: string, alpha: number): string { | ||
| const r = Number.parseInt(hex.slice(1, 3), 16); | ||
| const g = Number.parseInt(hex.slice(3, 5), 16); | ||
| const b = Number.parseInt(hex.slice(5, 7), 16); | ||
| return `rgba(${r}, ${g}, ${b}, ${alpha})`; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a color value is a custom hex color (not the "default" value). | ||
| */ | ||
| function isCustomColor(color: string): boolean { | ||
| return color !== PROJECT_COLOR_DEFAULT && color.startsWith("#"); | ||
| } |
There was a problem hiding this comment.
Add input validation to prevent invalid CSS values.
The hexToRgba function doesn't validate the hex color format. If an invalid hex string is passed (e.g., "#gg0000" or "#12"), Number.parseInt will return NaN, resulting in invalid CSS like "rgba(NaN, 0, 0, 0.6)". While isCustomColor checks for the # prefix, it doesn't validate the full hex format.
🔎 Proposed fix with validation
+const HEX_COLOR_REGEX = /^#[0-9A-Fa-f]{6}$/;
+
/**
* Converts a hex color to rgba with the specified alpha.
*/
function hexToRgba(hex: string, alpha: number): string {
+ if (!HEX_COLOR_REGEX.test(hex)) {
+ console.warn("[ProjectThumbnail/hexToRgba] Invalid hex color:", hex);
+ return "rgba(128, 128, 128, 0.6)"; // fallback to gray
+ }
const r = Number.parseInt(hex.slice(1, 3), 16);
const g = Number.parseInt(hex.slice(3, 5), 16);
const b = Number.parseInt(hex.slice(5, 7), 16);
return `rgba(${r}, ${g}, ${b}, ${alpha})`;
}
/**
* Checks if a color value is a custom hex color (not the "default" value).
*/
function isCustomColor(color: string): boolean {
- return color !== PROJECT_COLOR_DEFAULT && color.startsWith("#");
+ return color !== PROJECT_COLOR_DEFAULT && HEX_COLOR_REGEX.test(color);
}📝 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.
| /** | |
| * Converts a hex color to rgba with the specified alpha. | |
| */ | |
| function hexToRgba(hex: string, alpha: number): string { | |
| const r = Number.parseInt(hex.slice(1, 3), 16); | |
| const g = Number.parseInt(hex.slice(3, 5), 16); | |
| const b = Number.parseInt(hex.slice(5, 7), 16); | |
| return `rgba(${r}, ${g}, ${b}, ${alpha})`; | |
| } | |
| /** | |
| * Checks if a color value is a custom hex color (not the "default" value). | |
| */ | |
| function isCustomColor(color: string): boolean { | |
| return color !== PROJECT_COLOR_DEFAULT && color.startsWith("#"); | |
| } | |
| const HEX_COLOR_REGEX = /^#[0-9A-Fa-f]{6}$/; | |
| /** | |
| * Converts a hex color to rgba with the specified alpha. | |
| */ | |
| function hexToRgba(hex: string, alpha: number): string { | |
| if (!HEX_COLOR_REGEX.test(hex)) { | |
| console.warn("[ProjectThumbnail/hexToRgba] Invalid hex color:", hex); | |
| return "rgba(128, 128, 128, 0.6)"; // fallback to gray | |
| } | |
| const r = Number.parseInt(hex.slice(1, 3), 16); | |
| const g = Number.parseInt(hex.slice(3, 5), 16); | |
| const b = Number.parseInt(hex.slice(5, 7), 16); | |
| return `rgba(${r}, ${g}, ${b}, ${alpha})`; | |
| } | |
| /** | |
| * Checks if a color value is a custom hex color (not the "default" value). | |
| */ | |
| function isCustomColor(color: string): boolean { | |
| return color !== PROJECT_COLOR_DEFAULT && HEX_COLOR_REGEX.test(color); | |
| } |
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx
around lines 18 - 33, The hexToRgba function currently assumes a valid 6-digit
hex and can emit invalid CSS; validate the input in hexToRgba by checking it
matches a full 6-digit hex pattern (e.g. /^#([0-9A-Fa-f]{6})$/) before parsing,
and if invalid return a safe fallback CSS value (like a transparent rgba or a
configured default) or throw a clear error; also tighten isCustomColor to only
treat values as custom when they both differ from PROJECT_COLOR_DEFAULT and
match the same 6-digit hex pattern so downstream code never receives malformed
colors.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.