-
Notifications
You must be signed in to change notification settings - Fork 2
feat: container query layout for ModalForm/DrawerForm mobile single-column #670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,19 @@ const modalSizeClasses: Record<string, string> = { | |
| full: 'max-w-[95vw] w-full', | ||
| }; | ||
|
|
||
| /** | ||
| * 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', | ||
| }; | ||
|
Comment on lines
+108
to
+119
|
||
|
|
||
| export const ModalForm: React.FC<ModalFormProps> = ({ | ||
| schema, | ||
| dataSource, | ||
|
|
@@ -364,36 +377,44 @@ export const ModalForm: React.FC<ModalFormProps> = ({ | |
| if (schema.sections?.length) { | ||
| return ( | ||
| <div className="space-y-6"> | ||
| {schema.sections.map((section, index) => ( | ||
| <FormSection | ||
| key={section.name || section.label || index} | ||
| label={section.label} | ||
| description={section.description} | ||
| columns={section.columns || 1} | ||
| > | ||
| <SchemaRenderer | ||
| schema={{ | ||
| ...baseFormSchema, | ||
| fields: buildSectionFields(section), | ||
| // Actions are in the sticky footer, not inside sections | ||
| }} | ||
| /> | ||
| </FormSection> | ||
| ))} | ||
| {schema.sections.map((section, index) => { | ||
| const sectionCols = section.columns || 1; | ||
| return ( | ||
| <FormSection | ||
| key={section.name || section.label || index} | ||
| label={section.label} | ||
| description={section.description} | ||
| columns={sectionCols} | ||
| gridClassName={CONTAINER_GRID_COLS[sectionCols]} | ||
| > | ||
| <SchemaRenderer | ||
| schema={{ | ||
| ...baseFormSchema, | ||
| fields: buildSectionFields(section), | ||
| // Actions are in the sticky footer, not inside sections | ||
| }} | ||
| /> | ||
| </FormSection> | ||
| ); | ||
| })} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Reuse pre-computed auto-layout result for flat fields | ||
| const layoutResult = autoLayoutResult ?? applyAutoLayout(formFields, objectSchema, schema.columns, schema.mode); | ||
|
|
||
| // Flat fields layout | ||
| // Flat fields layout — use container-query grid classes so the form | ||
| // responds to the modal width, not the viewport width. | ||
| const containerFieldClass = CONTAINER_GRID_COLS[layoutResult.columns || 1]; | ||
|
|
||
| return ( | ||
| <SchemaRenderer | ||
| schema={{ | ||
| ...baseFormSchema, | ||
| fields: layoutResult.fields, | ||
| columns: layoutResult.columns, | ||
| ...(containerFieldClass ? { fieldContainerClass: containerFieldClass } : {}), | ||
| }} | ||
| /> | ||
| ); | ||
|
|
@@ -411,7 +432,7 @@ export const ModalForm: React.FC<ModalFormProps> = ({ | |
| </DialogHeader> | ||
| )} | ||
|
|
||
| <div className="flex-1 overflow-y-auto px-4 sm:px-6 py-4"> | ||
| <div className="@container flex-1 overflow-y-auto px-4 sm:px-6 py-4"> | ||
| {renderContent()} | ||
| </div> | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -191,3 +191,133 @@ describe('ModalForm Mobile UX', () => { | |||||||
| expect(screen.queryByTestId('modal-form-footer')).not.toBeInTheDocument(); | ||||||||
| }); | ||||||||
| }); | ||||||||
|
|
||||||||
| describe('ModalForm Container Query Layout', () => { | ||||||||
| /** CSS selector for the @container query context element */ | ||||||||
| const CONTAINER_SELECTOR = '.\\@container'; | ||||||||
|
|
||||||||
| it('applies @container class on scrollable content area', async () => { | ||||||||
| const mockDataSource = createMockDataSource(); | ||||||||
|
|
||||||||
| render( | ||||||||
| <ModalForm | ||||||||
| schema={{ | ||||||||
| type: 'object-form', | ||||||||
| formType: 'modal', | ||||||||
| objectName: 'events', | ||||||||
| mode: 'create', | ||||||||
| title: 'Create Event', | ||||||||
| open: true, | ||||||||
| }} | ||||||||
| dataSource={mockDataSource as any} | ||||||||
| /> | ||||||||
| ); | ||||||||
|
|
||||||||
| await waitFor(() => { | ||||||||
| expect(screen.getByText('Create Event')).toBeInTheDocument(); | ||||||||
| }); | ||||||||
|
|
||||||||
| // The scrollable content wrapper should be a @container query context | ||||||||
| const dialogContent = document.querySelector('[role="dialog"]'); | ||||||||
| expect(dialogContent).not.toBeNull(); | ||||||||
| const scrollArea = dialogContent!.querySelector(CONTAINER_SELECTOR); | ||||||||
| expect(scrollArea).not.toBeNull(); | ||||||||
| expect(scrollArea!.className).toContain('overflow-y-auto'); | ||||||||
| }); | ||||||||
|
|
||||||||
| it('uses container-query grid classes for multi-column flat fields', async () => { | ||||||||
| // Mock schema with enough fields to trigger auto-layout 2-column | ||||||||
| const manyFieldsSchema = { | ||||||||
| name: 'contacts', | ||||||||
| fields: { | ||||||||
| name: { label: 'Name', type: 'text', required: true }, | ||||||||
| email: { label: 'Email', type: 'email', required: false }, | ||||||||
| phone: { label: 'Phone', type: 'phone', required: false }, | ||||||||
| company: { label: 'Company', type: 'text', required: false }, | ||||||||
| department: { label: 'Department', type: 'text', required: false }, | ||||||||
| title: { label: 'Title', type: 'text', required: false }, | ||||||||
| }, | ||||||||
| }; | ||||||||
| const mockDataSource = createMockDataSource(); | ||||||||
| mockDataSource.getObjectSchema.mockResolvedValue(manyFieldsSchema); | ||||||||
|
|
||||||||
| render( | ||||||||
| <ModalForm | ||||||||
| schema={{ | ||||||||
| type: 'object-form', | ||||||||
| formType: 'modal', | ||||||||
| objectName: 'contacts', | ||||||||
| mode: 'create', | ||||||||
| title: 'Create Contact', | ||||||||
| open: true, | ||||||||
| }} | ||||||||
| dataSource={mockDataSource as any} | ||||||||
| /> | ||||||||
| ); | ||||||||
|
|
||||||||
| await waitFor(() => { | ||||||||
| expect(screen.getByText('Create Contact')).toBeInTheDocument(); | ||||||||
| }); | ||||||||
|
|
||||||||
| // Wait for fields to render | ||||||||
| await waitFor(() => { | ||||||||
| expect(screen.getByText('Name')).toBeInTheDocument(); | ||||||||
| }); | ||||||||
|
|
||||||||
| // The form field container should use container-query classes (@md:grid-cols-2) | ||||||||
| // instead of viewport-based classes (md:grid-cols-2) | ||||||||
| const dialogContent = document.querySelector('[role="dialog"]'); | ||||||||
| const containerEl = dialogContent!.querySelector(CONTAINER_SELECTOR); | ||||||||
| expect(containerEl).not.toBeNull(); | ||||||||
|
|
||||||||
| // Look for the grid container with @md:grid-cols-2 | ||||||||
| const gridEl = containerEl!.querySelector('[class*="@md:grid-cols-2"]'); | ||||||||
| 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'); | ||||||||
|
||||||||
| expect(gridEl!.className).not.toContain(' md:grid-cols-2'); | |
| const classTokens = gridEl!.className.split(/\s+/); | |
| expect(classTokens).not.toContain('md:grid-cols-2'); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gridClassNameis 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 makinggridClassNamea full override (use it instead of the defaultcn('grid gap-4', gridCols[columns])), or rename it to indicate it only overrides the grid-cols portion and stripgrid/gapfrom the values passed in.