fix: EventTypeSettings Checkbox booking field label #23891
Conversation
WalkthroughFormBuilder’s FieldEditDialog now imports Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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. Comment |
keithwillcode
added
core
![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 (3)
.changeset/config.json (1)
10-10: Confirm ignoring @calcom/api-v2 is intended.Adding "@calcom/api-v2" to ignore prevents version bumps when only that package changes. Verify this aligns with release policy. Also consider sorting entries for readability.
Apply this diff to sort:
- "ignore": ["@calcom/platform-libraries", "@calcom/api-v2"], + "ignore": ["@calcom/api-v2", "@calcom/platform-libraries"],.changeset/cold-dragons-chew.md (1)
1-5: Changeset looks good.Patch bump scoped to @calcom/atoms with clear note. Optional: reference CAL-6421 in the message for traceability.
Suggested tweak:
-fix: EventTypeSettings Checkbox booking field label +fix: EventTypeSettings Checkbox booking field label (CAL-6421)packages/features/form-builder/FormBuilder.tsx (1)
574-574: Hook usage OK; consider lifting to parent only if reused.Tiny nit: If multiple children need platform detection later, compute once and pass down.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/cold-dragons-chew.md(1 hunks).changeset/config.json(1 hunks)packages/features/form-builder/FormBuilder.tsx(3 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:
packages/features/form-builder/FormBuilder.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:
packages/features/form-builder/FormBuilder.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:
packages/features/form-builder/FormBuilder.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
📚 Learning: 2025-08-29T22:57:30.382Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/form-builder/FormBuilder.tsx:11-11
Timestamp: 2025-08-29T22:57:30.382Z
Learning: FormBuilder.tsx in packages/features/form-builder/ does not have "use client" directive at the top despite using client-side React hooks and event handlers, which suggests it should be a client component.
Applied to files:
packages/features/form-builder/FormBuilder.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
.changeset/cold-dragons-chew.md
🧬 Code graph analysis (1)
packages/features/form-builder/FormBuilder.tsx (1)
packages/platform/atoms/index.ts (1)
useIsPlatform(7-7)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
packages/features/form-builder/FormBuilder.tsx (2)
652-665: Correct conditional fix for platform.On platform, boolean label now uses InputField instead of Editor—matches CAL-6421 and restores editability/border. Please verify UX quickly in both contexts.
Manual check steps:
- Platform render: add boolean booking field → edit label → confirm border and typing work; placeholder shows defaultLabel.
- Non‑platform render: boolean uses rich Editor; others unchanged.
7-7: Import path OK — hook exists; verify client usage & exports
- packages/platform/atoms/hooks/useIsPlatform.ts exports
useIsPlatformand is imported/mocked across the repo.- No immediate circular-dep concern from a quick grep — the hook only imports
useAtomsContextfrom the same atoms package.- Action: ensure FormBuilder.tsx is a client component (add "use client" as the very first line if it calls hooks/useIsPlatform) and confirm package export / tsconfig path mappings for any non-TS or external build targets.
…heckbox-booking-field
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/form-builder/FormBuilder.tsx (2)
653-667: Good platform fallback; add RHF validation to truly enforce required labelThe UI
requiredattr is set, but RHF isn’t enforcing it. Add arequiredrule (and trim) inregisterso submission can’t proceed with an empty/whitespace label.Apply:
- {formFieldType === "boolean" && !isPlatform ? ( + {formFieldType === "boolean" && !isPlatform ? ( <CheckboxFieldLabel fieldForm={fieldForm} /> ) : ( <InputField - {...fieldForm.register("label")} + {...fieldForm.register("label", { + required: !["system", "system-but-optional"].includes( + fieldForm.getValues("editable") || "" + ), + setValueAs: (v) => (typeof v === "string" ? v.trim() : v), + })} // System fields have a defaultLabel, so there a label is not required required={ !["system", "system-but-optional"].includes(fieldForm.getValues("editable") || "") } + aria-required={ + !["system", "system-but-optional"].includes(fieldForm.getValues("editable") || "") + } placeholder={t(fieldForm.getValues("defaultLabel") || "")} containerClassName="mt-6" label={t("label")} /> )}
7-7: Prefer importing useIsPlatform from the package entrypoint (@calcom/atoms)packages/platform/atoms/index.ts re-exports useIsPlatform (line 7) and package.json also exposes the subpath; current deep import will work, but prefer changing to
import { useIsPlatform } from "@calcom/atoms";.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/form-builder/FormBuilder.tsx(3 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:
packages/features/form-builder/FormBuilder.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:
packages/features/form-builder/FormBuilder.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:
packages/features/form-builder/FormBuilder.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
📚 Learning: 2025-08-29T22:57:30.382Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/form-builder/FormBuilder.tsx:11-11
Timestamp: 2025-08-29T22:57:30.382Z
Learning: FormBuilder.tsx in packages/features/form-builder/ does not have "use client" directive at the top despite using client-side React hooks and event handlers, which suggests it should be a client component.
Applied to files:
packages/features/form-builder/FormBuilder.tsx
🧬 Code graph analysis (1)
packages/features/form-builder/FormBuilder.tsx (1)
packages/platform/atoms/index.ts (1)
useIsPlatform(7-7)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/features/form-builder/FormBuilder.tsx (1)
575-575: LGTM: correct use of platform check inside dialog scopeHook usage is appropriate and scoped; no concerns.
supalarry
deleted the
lauris/cal-6421-fix-eventtypesettings-atom-checkbox-booking-field
branch
E2E results are ready! |
Linear CAL-6421