fix: invalid default color in label creation dialog#1547
fix: invalid default color in label creation dialog#1547vcashwin wants to merge 2 commits intoMail-0:stagingfrom
Conversation
WalkthroughThis change introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LabelDialog
participant Form
User->>LabelDialog: Open dialog (new/edit label)
LabelDialog->>Form: Initialize/reset with defaultColor
User->>LabelDialog: Close dialog
LabelDialog->>Form: Reset to defaultColor
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/mail/components/labels/label-dialog.tsx (1)
49-57: Consider updating form defaultValues for consistency.While the current implementation works since the form gets reset in the
useEffect, consider updating the form'sdefaultValuesto usedefaultColorfor better consistency:const form = useForm<LabelType>({ defaultValues: { name: '', - color: { - backgroundColor: '', - textColor: '', - }, + color: defaultColor, }, });This would eliminate the brief moment where the form has empty color values before the
useEffectruns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/labels/label-dialog.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: danteissaias
PR: Mail-0/Zero#902
File: apps/mail/components/connection/add.tsx:77-77
Timestamp: 2025-05-07T16:55:46.513Z
Learning: For the "Upgrade" link in AddConnectionDialog, using a proper <button> element instead of a <span> with onClick is recognized as an accessibility improvement but was deferred as out of scope in PR #902 (CSS variables PR).
apps/mail/components/labels/label-dialog.tsx (1)
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
🧬 Code Graph Analysis (1)
apps/mail/components/labels/label-dialog.tsx (2)
apps/mail/lib/label-colors.ts (1)
LABEL_COLORS(1-8)apps/mail/components/ui/dialog.tsx (1)
DialogTitle(115-115)
🔇 Additional comments (4)
apps/mail/components/labels/label-dialog.tsx (4)
60-60: LGTM: Dynamic default color correctly addresses the bug.The
defaultColorconstant ensures the default color is always valid by using the first color fromLABEL_COLORS. This prevents the toast error that occurred when using the hardcoded color that wasn't in the allowed colors array.
68-68: LGTM: Consistent replacement of hardcoded color values.All instances of the hardcoded
{ backgroundColor: '#E2E2E2', textColor: '#000000' }have been correctly replaced withdefaultColor, ensuring consistent behavior across form initialization, editing, and dialog close scenarios.Also applies to: 73-73, 89-89
21-21: Minor formatting improvement.The import reordering improves code organization without affecting functionality.
Also applies to: 23-23
98-100: JSX formatting improvement.The multi-line formatting of
DialogTitleimproves readability without changing functionality.
| import { Label } from '@/components/ui/label'; | ||
| import { Input } from '@/components/ui/input'; | ||
| import { useState, useEffect } from 'react'; | ||
| import { useTranslations } from 'use-intl'; |
|
5 days and no update, closing for now |
2 participants
Description
Previously, the default color was hardcoded and not dynamically picked up from the list of allowed label colors thereby throwing a toast error when creation. This PR adds a default color to be set for label creation dialog.
Screenshots
Before:
After:
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
Add screenshots or recordings here if applicable.
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit