-
-
Notifications
You must be signed in to change notification settings - Fork 491
feat(editor): use hex format for checking if a color is selected #239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(editor): use hex format for checking if a color is selected #239
Conversation
|
@gerardbalaoro is attempting to deploy a commit to the tweakcn OSS program Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughReplaces direct string equality in color comparison with hex-normalized comparison using culori's Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ColorSelector UI
participant Fn as isColorSelected
participant Culori as culori (parse/formatHex)
UI->>Fn: "is this color selected?"
Fn->>Culori: parse(currentColor) → formatHex
Culori-->>Fn: normalizedHexA
Fn->>Culori: parse(targetColor) → formatHex
Culori-->>Fn: normalizedHexB
alt both parsed successfully
Fn->>Fn: compare normalizedHexA == normalizedHexB
Fn-->>UI: return true/false
else parse/format error
Fn->>Fn: fallback compare currentColor === targetColor
Fn-->>UI: return true/false
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/editor/color-selector-popover.tsx (1)
49-49: Consider extracting the helper function for minor performance improvement.The
toHexhelper is redefined on everyisColorSelectedinvocation. While likely negligible in practice, you could extract it outside the callback or memoize it separately for a micro-optimization.+const toHex = (c: string) => formatHex(parse(c)); + const isColorSelected = useCallback( (color: string) => { try { - const toHex = (c: string) => formatHex(parse(c)); - return toHex(currentColor) === toHex(color); } catch { return currentColor === color; } }, [currentColor] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
components/editor/color-selector-popover.tsx(2 hunks)
⏰ 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: Vercel Agent Review
🔇 Additional comments (2)
components/editor/color-selector-popover.tsx (2)
46-57: Solid approach with appropriate fallback handling.The hex normalization correctly solves the color format mismatch issue. The try/catch with fallback to string equality provides a safe and practical solution when color parsing fails.
21-21: No issues found — implementation correctly guards against undefined inputs.The try/catch wrapper at lines 48–52 provides appropriate error handling for edge cases where
parse()returns undefined orformatHex()fails. culori.parse supports CSS Color Level 4 formats used by Tailwind v4, but returns undefined for unrecognized strings, and formatHex expects a valid color. The code safely handles this by wrapping both calls in a try/catch block and falling back to string comparison when hex conversion fails.
781d852 to
0942db3
Compare
0942db3 to
3cd7e01
Compare
What
Added a minor enhancement to the color comparison logic in the
ColorSelectorPopovercomponent. When checking if a Tailwind v4 color is selected, we convert thecurrentColorandcolorto hex format. This would cover cases where colors are entered in different formats.Why
I wanted to customize the "Blue" theme from the shadcn website which clearly uses the "Blue" palette from Tailwind v4. However, importing it to the editor converts the colors to hex format which prevents the color popover from correctly identifying any Tailwind v4 colors.
Changes
culoripackage which is already installed and has functions to parse colors from any format and convert it to hex.Summary by CodeRabbit
Bug Fixes
Chores