fix: improve mobile positioning for link dialog in form builder checkbox editor#24268
Conversation
|
|
|
@kartik-212004 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe link editor positioning was made viewport-aware in ToolbarPlugin.tsx: top/left are computed locally, horizontal position is clamped to a 10px margin, and vertical placement is adjusted to avoid bottom overflow (moving the editor above the anchor when needed and enforcing a minimum top inset). Calculations use window.innerWidth/innerHeight and editor dimensions with fallbacks. Two Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ui/components/editor/plugins/ToolbarPlugin.tsx (1)
59-86: Prefer consistent spelling + typo fix.Comment
// tgis willhas a typo. Please fix to// this will.- // tgis will calculate the initial position + // this will calculate the initial position
📜 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)
packages/features/form-builder/FormBuilder.tsx(2 hunks)packages/ui/components/editor/plugins/ToolbarPlugin.tsx(2 hunks)packages/ui/components/editor/stylesEditor.css(2 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/ui/components/editor/plugins/ToolbarPlugin.tsxpackages/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/ui/components/editor/plugins/ToolbarPlugin.tsxpackages/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/ui/components/editor/plugins/ToolbarPlugin.tsxpackages/features/form-builder/FormBuilder.tsx
🧠 Learnings (1)
📚 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 (2)
packages/features/form-builder/fieldTypes.ts (1)
fieldTypesConfigMap(170-170)packages/ui/components/dialog/Dialog.tsx (2)
Dialog(35-38)DialogContent(56-117)
⏰ 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: Codacy Static Code Analysis
kartik-212004
force-pushed
the
fix/mobile-link-dialog-positioning
branch
from
b026a84 to
036f84a
Compare
dhairyashiil
left a comment
dhairyashiil
left a comment
There was a problem hiding this comment.
LGTM 👍🏼 , Thank you for your contribution.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/components/editor/plugins/ToolbarPlugin.tsx (1)
400-400: Acceptable error handling cleanup.Changing from
catch (e: unknown)to genericcatchis fine since the error parameter is unused. The catch block still handles the specific "topLevelElement is root node" error as intended per the comment.
📜 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/ui/components/editor/plugins/ToolbarPlugin.tsx(2 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/ui/components/editor/plugins/ToolbarPlugin.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/ui/components/editor/plugins/ToolbarPlugin.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/ui/components/editor/plugins/ToolbarPlugin.tsx
🔇 Additional comments (1)
packages/ui/components/editor/plugins/ToolbarPlugin.tsx (1)
60-83: LGTM! Comprehensive viewport-aware positioning logic.The positioning implementation correctly addresses the mobile/tablet positioning issue with robust edge-case handling:
- Horizontal clamping (lines 68-72) ensures the dialog stays within viewport bounds with 10px margins
- Vertical positioning (lines 74-80) intelligently repositions above the anchor when bottom overflow is detected
- Top clamping (lines 77-79) prevents the dialog from being pushed above the viewport when the editor is taller than the viewport, addressing the concern from the previous review
The fallback dimensions (300px width, 100px height) align well with the CSS max-width constraint mentioned in the PR.
Please verify the fix works as expected on mobile/tablet devices following the testing instructions:
- Test at viewport widths 320px, 480px, and 768px
- Confirm the dialog stays within viewport and maintains 200px minimum width
- Verify no regressions on desktop
E2E results are ready! |
…box editor (calcom#24268) * Updated positionEditorElement function to respect viewport boundaries * cleanup
4 participants
What does this PR do?
This PR fixes a mobile positioning issue where the link attachment dialog in the checkbox input type within the "Add a question" feature opens outside screen boundaries on mobile and tablet devices. Users were unable to access or interact with the link dialog when editing checkbox question labels in event type advanced settings.
Visual Demo (For contributors especially)
Video Demo (if applicable):
Before Fix:
2025-10-04.10-32-35.mp4
*After Fix:
2025-10-04.10-42-05.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Environment Setup:
Test Steps:
Desktop Testing:
Mobile/Tablet Testing (Critical):
Expected Results:
Test Data: