Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…WCAG touch targets - Update base DialogContent to use inset-0 h-[100dvh] on mobile, centered on desktop (sm+) - Add min-h-[44px] min-w-[44px] close button touch target per WCAG 2.5.5 - Larger close icon on mobile (h-5 w-5), standard size on desktop (sm:h-4 sm:w-4) - Update snapshot test for new dialog structure - Update ROADMAP with mobile UX optimization details Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the base Dialog primitive to behave as a mobile-first full-screen modal (with WCAG-sized close button), and refreshes snapshots/roadmap to reflect the new styling.
Changes:
- Adjust
DialogContentlayout to full-screen on mobile and centered modal onsm+, plus larger close-button touch targets. - Update snapshot to match the new
DialogContentclassName output. - Document the completed mobile UX milestone in
ROADMAP.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/components/src/ui/dialog.tsx | Changes base DialogContent positioning/sizing and close button sizing for mobile-first dialogs. |
| packages/components/src/tests/snapshots/snapshot-critical.test.tsx.snap | Updates snapshots for the new DialogContent class strings. |
| ROADMAP.md | Notes the dialog/mobile UX improvements under completed milestones. |
| // Mobile-first: full-screen | ||
| "fixed inset-0 z-50 grid w-full bg-background p-4 shadow-lg duration-200", | ||
| "h-[100dvh]", | ||
| // Desktop (sm+): centered dialog with border + rounded corners | ||
| "sm:inset-auto sm:left-[50%] sm:top-[50%] sm:translate-x-[-50%] sm:translate-y-[-50%]", | ||
| "sm:max-w-lg sm:h-auto sm:max-h-[90vh] sm:rounded-lg sm:border sm:p-6", |
There was a problem hiding this comment.
packages/components/src/ui/** is treated as Shadcn-synced “no-touch” code in this repo, but this PR changes ui/dialog.tsx styles/behavior. This is likely to be overwritten by shadcn-sync and makes future upstream updates harder. Consider reverting ui/dialog.tsx to the upstream version and implementing this mobile-first layout via a wrapper in packages/components/src/custom/ (similar to existing MobileDialogContent), then migrate call sites to use the wrapper (or re-export the wrapper from a stable entrypoint). Reference: .github/copilot-instructions.md:129-136, packages/components/src/custom/mobile-dialog-content.tsx:17.
| className={cn( | ||
| "fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg", | ||
| // Mobile-first: full-screen | ||
| "fixed inset-0 z-50 grid w-full bg-background p-4 shadow-lg duration-200", |
There was a problem hiding this comment.
The new base DialogContent class list drops gap-4 from the grid container. Several dialogs rely on the default gap for spacing between DialogHeader / body / footer (e.g. ActionParamDialog doesn’t add spacing between header and body). Reintroduce gap-4 (at least as the default) to avoid UI regressions.
| "fixed inset-0 z-50 grid w-full bg-background p-4 shadow-lg duration-200", | |
| "fixed inset-0 z-50 grid w-full gap-4 bg-background p-4 shadow-lg duration-200", |
| "fixed inset-0 z-50 grid w-full bg-background p-4 shadow-lg duration-200", | ||
| "h-[100dvh]", | ||
| // Desktop (sm+): centered dialog with border + rounded corners | ||
| "sm:inset-auto sm:left-[50%] sm:top-[50%] sm:translate-x-[-50%] sm:translate-y-[-50%]", | ||
| "sm:max-w-lg sm:h-auto sm:max-h-[90vh] sm:rounded-lg sm:border sm:p-6", | ||
| // Animations |
There was a problem hiding this comment.
Switching to fixed inset-0 aims to make dialogs full-screen on mobile, but consumer-provided max-w-* classes without a breakpoint can now prevent full-screen behavior (e.g. LookupField uses DialogContent className="max-w-md"). If this change is kept, consider updating affected call sites to use sm:max-w-* (or otherwise ensure mobile ignores max-width) so the “full-screen on mobile” behavior is consistent across the app.
Base
DialogContentindialog.tsxstill used fixed centered positioning (left-[50%] top-[50%]), wasting viewport on mobile and leaving the close button at 16×16px. WhileMobileDialogContentalready solved this forModalForm, all other dialogs (command palette, import wizard, lookup fields, action dialogs, etc.) were unoptimized.Changes
packages/components/src/ui/dialog.tsx— Mobile-firstDialogContent:inset-0 h-[100dvh]on mobile →sm:inset-auto sm:left-[50%] sm:top-[50%] sm:max-w-lg sm:h-auto sm:max-h-[90vh] sm:rounded-lg sm:borderon desktopmin-h-[44px] min-w-[44px]touch target on mobile (WCAG 2.5.5), resets atsm:h-5 w-5on mobile,sm:h-4 sm:w-4on desktopAlready implemented (verified, no changes needed)
MobileDialogContent+ ModalForm flex layout (sticky header/footer)md:grid-cols-2breakpoint)type="date",type="datetime-local")ModalFormSectionConfigOriginal prompt
This section details on the original issue you should resolve
<issue_title>[Mobile UX] 优化移动端 ModalForm 弹窗的表单交互和布局</issue_title>
<issue_description>### 背景
在移动端 "Create Event" 类型弹窗中,表单存在多处可优化体验:如弹窗没有全屏覆盖、主操作按钮易被内容遮挡、部分字段(如日期)缺乏原生输入体验等。
主要问题
<input type="datetime-local">,提升录入效率。优化建议与实施点:
inset-0 h-[100dvh]并顶部/底部圆角移至 sm 断点。影响组件及文件
参考 ROADMAP 相关目标 & 已完成项:
h-[100dvh] sm:h-auto sm:max-h-[90vh]Comments on the Issue (you are @copilot in this section)
@hotlongAnalysis of the "Create Event" Form UI
Let me first examine the current codebase to understand the existing form/dialog implementation before providing optimization recommendations.Let me explore the current form/dialog implementation in the repository to understand the codebase before providing optimization recommendations.Now let me get more details about the auto-layout logic and the form rendering within the modal:Now let me look at the full
ModalForm.tsxrendering to understand the complete picture, and also theFormSectionand the DialogContent styling:Now let me also check the ROADMAP for any existing mobile UX items related to this form dialog and what open issues may be relevant:Now let me also look at the DialogContent base component to understand the current mobile styles:Now I have a thorough understanding of the codebase. Let me provide a comprehensive UX optimization analysis of this "Create Event" modal form on mobile.📱 "Create Event" ModalForm Mobile UX Optimization Analysis
Based on the screenshot from
demo.objectui.organd deep analysis of the codebase, I've identified 8 optimization areas across 3 layers: DialogContent (primitives), ModalForm (plugin), and Form Renderer (components).🔍 Issue Summary
dialog.tsx+ModalForm.tsxModalForm.tsxModalForm.tsxauto-layoutdialog.tsxModalForm.tsxrenderModalForm.tsx1. 🔴 Modal Should Be Full-Screen on Mobile
Current state: The
DialogContentuses a centeredfixed left-[50%] top-[50%] translatepositioning withmax-w-lg, which wastes significant viewport space on small screens. The ROADMAP already marks this as[x] Full-screen modal on mobile, but the currentdialog.tsxdoesn't implement it:And in ModalForm:
Recommendation: Update
DialogContentto be full-screen on mobile and centered on desktop: