fix: incorrect placeholder for start of week #23402
Conversation
Signed-off-by: Anirban Singha <[email protected]>
|
@SinghaAnirban005 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe change modifies how the GeneralView form sets the default label for the "Start of week" field. Instead of deriving the label via a locale-based day name function that defaulted to Monday for non-Sunday selections, it now finds the matching option in weekStartOptions using user.weekStart and uses that option’s label (with a fallback to an empty string). No exported/public signatures were altered. Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![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 (2)
apps/web/modules/settings/my-account/general-view.tsx (2)
117-120: Use the exact option object to avoid identity mismatches in Select.Some Select implementations compare by object identity. Passing the found option directly is more robust than reconstructing
{ value, label }.Apply this diff:
weekStart: { - value: user.weekStart, - label: weekStartOptions.find((option) => option.value === user.weekStart)?.label || "", - }, + // Use the exact option object; fallback to Monday, then first option. + // This avoids object-identity mismatches in Select implementations. + : weekStartOptions.find((option) => option.value === user.weekStart) + ?? weekStartOptions.find((option) => option.value === "Monday") + ?? weekStartOptions[0],
367-367: Prefer named export (optional).To align with the repo guideline favoring named exports, consider exporting
GeneralViewas named. Defer if this component is intentionally the module’s primary export.-export default GeneralView; +export { GeneralView };Note: This requires updating import sites.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/modules/settings/my-account/general-view.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/modules/settings/my-account/general-view.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/modules/settings/my-account/general-view.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/modules/settings/my-account/general-view.tsx
🔇 Additional comments (2)
apps/web/modules/settings/my-account/general-view.tsx (2)
117-120: Good fix: label now reliably reflects the selected start-of-week.Switching to look up the label from
weekStartOptionsremoves the Monday-default bug and aligns the placeholder with the chosen day.
293-299: Select equality semantics verified — no action requiredShort: I checked the codebase — our Select is a thin wrapper around react-select and does not override equality. react-select uses getOptionValue (default: option.value) when marking options as selected, so reconstructing the selected option with the same .value (e.g. { ...event }) will still match the options list.
Checked files:
- packages/ui/components/form/select/Select.tsx — wraps ReactSelect; no isOptionEqualToValue/getOptionValue overrides.
- packages/ui/components/form/select/selectTheme.ts — injects custom components only.
- Location under review: apps/web/modules/settings/my-account/general-view.tsx (the <Select value={value} ... /> snippet).
Conclusion: No change required; the original concern (object-identity matching causing placeholder drift) is not applicable given react-select’s default equality.
E2E results are ready! |
2 participants
What does this PR do?
This PR correctly sets the start of week placeholder based on the chosen day
Visual Demo (For contributors especially)
Screencast.from.2025-08-28.03-08-58.webm
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?