feat: container query layout for ModalForm/DrawerForm mobile single-column#670
feat: container query layout for ModalForm/DrawerForm mobile single-column#670
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…le single-column - Add @container to ModalForm/DrawerForm content wrappers - Use @md:grid-cols-2 (container query) instead of md:grid-cols-2 (viewport) - Add gridClassName prop to FormSection for container-query override - Add 3 new tests for container query behavior - Update ROADMAP.md with container query support status Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…implify assertion Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the form modal/drawer layouts to use CSS container queries so multi-column form grids respond to the actual Modal/Drawer width (fixing high-res mobile viewports incorrectly triggering 2-column layouts).
Changes:
- Add
@containercontexts inModalFormandDrawerForm, and pass container-query grid classes (@md:/@2xl:/@4xl:) into the form renderer viafieldContainerClass. - Extend
FormSectionwith agridClassNameoverride to support container-query-based section grids. - Add container-query layout tests for
ModalFormand update the Mobile UX section inROADMAP.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-form/src/tests/MobileUX.test.tsx | Adds tests asserting @container context and container-query grid usage in ModalForm. |
| packages/plugin-form/src/ModalForm.tsx | Adds container-query grid class mapping and applies @container to the scroll area; passes fieldContainerClass for flat-field layouts and section overrides. |
| packages/plugin-form/src/FormSection.tsx | Adds gridClassName prop to allow overriding default viewport breakpoint grid classes. |
| packages/plugin-form/src/DrawerForm.tsx | Mirrors ModalForm container-query approach for drawers and adds @container wrapper. |
| ROADMAP.md | Updates roadmap notes to reflect the container-query-based layout behavior. |
| <div className={cn('grid gap-4', gridClassName || gridCols[columns])}> | ||
| {children} |
There was a problem hiding this comment.
gridClassName is treated as an override, but it’s still combined with the hard-coded 'grid gap-4'. This both prevents callers from changing the gap and can produce duplicated classes (e.g. passing 'grid gap-4 ...' from ModalForm/DrawerForm results in 'grid gap-4 grid gap-4 ...'). Consider making gridClassName a full override (use it instead of the default cn('grid gap-4', gridCols[columns])), or rename it to indicate it only overrides the grid-cols portion and strip grid/gap from the values passed in.
| expect(gridEl).not.toBeNull(); | ||
| expect(gridEl!.className).toContain('@md:grid-cols-2'); | ||
| // Should NOT use viewport-based md:grid-cols-2 (without @ prefix) | ||
| expect(gridEl!.className).not.toContain(' md:grid-cols-2'); |
There was a problem hiding this comment.
This assertion can miss a regression because it only checks for ' md:grid-cols-2' (with a leading space). If the class list contains 'md:grid-cols-2' at the beginning (or without a preceding space), the test will still pass. Suggest checking for 'md:grid-cols-2' more robustly (e.g. tokenizing classes and ensuring the md:grid-cols-2 token is absent, while still allowing @md:grid-cols-2).
| expect(gridEl!.className).not.toContain(' md:grid-cols-2'); | |
| const classTokens = gridEl!.className.split(/\s+/); | |
| expect(classTokens).not.toContain('md:grid-cols-2'); |
| /** | ||
| * Container-query-based grid classes for form field layout. | ||
| * Uses @container / @md: / @2xl: / @4xl: variants so that the grid | ||
| * responds to the modal's actual width instead of the viewport, | ||
| * ensuring single-column on narrow mobile modals regardless of viewport size. | ||
| */ | ||
| const CONTAINER_GRID_COLS: Record<number, string | undefined> = { | ||
| 1: undefined, // let the form renderer use its default (space-y-4) | ||
| 2: 'grid gap-4 grid-cols-1 @md:grid-cols-2', | ||
| 3: 'grid gap-4 grid-cols-1 @md:grid-cols-2 @2xl:grid-cols-3', | ||
| 4: 'grid gap-4 grid-cols-1 @md:grid-cols-2 @2xl:grid-cols-3 @4xl:grid-cols-4', | ||
| }; |
There was a problem hiding this comment.
CONTAINER_GRID_COLS is duplicated in both ModalForm and DrawerForm. To avoid the two mappings drifting over time (breakpoints/classes changing in one place but not the other), consider extracting this constant to a shared module (e.g. packages/plugin-form/src/containerGridCols.ts) and importing it from both components.
ModalForm uses viewport-based
md:grid-cols-2(768px breakpoint) for multi-column layout. On high-res phones where viewport ≥ 768px, the modal renders two columns despite being ~350px wide — unusable on mobile.Switches to CSS Container Queries so the form grid responds to the modal's actual width, not the viewport.
Changes
@containeron content wrapper; passfieldContainerClasswith container-query grid classes (@md:grid-cols-2at container ≥ 448px) to SchemaRenderergridClassNameprop to override viewport-based grid classes from parent context@containerpresence, container-query grid for multi-col, no override for single-colBreakpoint behavior
@md)@2xl)@4xl)Tailwind v4 supports container queries natively — no plugin needed. The existing
fieldContainerClassprop on the form renderer is used to override the default viewport-based grid, keeping the change fully backward-compatible.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.