Skip to content

Commit bbddffa

Browse files
authored
Merge pull request #1035 from objectstack-ai/copilot/fix-mobile-detail-page-layout
2 parents 061cdfa + f609464 commit bbddffa

File tree

3 files changed

+166
-25
lines changed

3 files changed

+166
-25
lines changed

ROADMAP.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,6 +1474,20 @@ All 313 `@object-ui/fields` tests pass.
14741474

14751475
---
14761476

1477+
### DetailSection — Mobile Responsive col-span Fix (March 2026)
1478+
1479+
> **Bug:** On mobile devices, detail page fields displayed in 3-column layout instead of single column, causing content to squeeze together and severely impacting readability.
1480+
1481+
**Root Cause:** When `applyAutoSpan` set `span: 3` on wide fields (textarea, markdown, etc.) in a 3-column layout, the resulting `col-span-3` CSS class was applied without responsive prefixes. In CSS Grid, when an item has `col-span-3` in a `grid-cols-1` layout, the browser creates 2 implicit column tracks — and subsequent auto-placed items flow into those implicit columns, producing a 3-column layout even on mobile.
1482+
1483+
**Fix:**
1484+
1485+
- **DetailSection** (`packages/plugin-detail/src/DetailSection.tsx`): Added `getResponsiveSpanClass()` helper that generates responsive col-span classes matching the grid breakpoints. For a 3-column layout: no col-span at base (mobile single-column), `md:col-span-2` at tablet, `lg:col-span-3` at desktop. For a 2-column layout: no col-span at base, `md:col-span-2` at tablet+. This ensures col-span never exceeds the visible column count at each breakpoint, preventing implicit grid columns on mobile.
1486+
1487+
**Tests:** 11 new tests (8 getResponsiveSpanClass unit tests + 3 DetailSection integration tests verifying no bare col-span-N classes appear). All 52 plugin-detail tests pass.
1488+
1489+
---
1490+
14771491
### RecordDetailView — Action Button Full-Chain Integration (March 2026)
14781492

14791493
> **Issue #107:** All Action buttons on record detail pages (Change Stage, Mark as Won, etc.) clicked with zero response — no dialogs, no API calls, no toast, no data refresh.

packages/plugin-detail/src/DetailSection.tsx

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,28 @@ import { applyDetailAutoLayout } from './autoLayout';
3131
import { useDetailTranslation } from './useDetailTranslation';
3232
import { useSafeFieldLabel } from '@object-ui/react';
3333

34+
/**
35+
* Compute responsive col-span classes so that col-span never exceeds the
36+
* visible column count at each Tailwind breakpoint.
37+
*
38+
* For columns=1: no span class (always single column)
39+
* For columns=2: md:col-span-{min(span,2)}
40+
* For columns>=3: md:col-span-{min(span,2)} lg:col-span-{min(span,3)}
41+
*/
42+
export function getResponsiveSpanClass(span: number | undefined, columns: number): string {
43+
if (!span || span <= 1 || columns <= 1) return '';
44+
45+
if (columns === 2) {
46+
return span >= 2 ? 'md:col-span-2' : '';
47+
}
48+
49+
// columns >= 3: grid-cols-1 md:grid-cols-2 lg:grid-cols-3
50+
if (span === 2) return 'md:col-span-2';
51+
if (span >= 3) return 'md:col-span-2 lg:col-span-3';
52+
53+
return '';
54+
}
55+
3456
export interface DetailSectionProps {
3557
section: DetailViewSectionType;
3658
data?: any;
@@ -67,6 +89,23 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
6789
});
6890
}, []);
6991

92+
// Filter out empty fields when hideEmpty is set
93+
const visibleFields = section.hideEmpty
94+
? section.fields.filter((field) => {
95+
const value = data?.[field.name] ?? field.value;
96+
return value !== null && value !== undefined && value !== '';
97+
})
98+
: section.fields;
99+
100+
// Hide entire section when all fields are empty
101+
if (visibleFields.length === 0) return null;
102+
103+
// Apply auto-layout: infer columns and auto-span wide fields
104+
const { fields: layoutFields, columns: effectiveColumns } = applyDetailAutoLayout(
105+
visibleFields,
106+
section.columns
107+
);
108+
70109
const renderField = (field: DetailViewField) => {
71110
const value = data?.[field.name] ?? field.value;
72111

@@ -75,13 +114,9 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
75114
return <SchemaRenderer schema={field.render} data={{ ...data, value }} />;
76115
}
77116

78-
// Calculate span class based on field.span value
79-
const spanClass = field.span === 1 ? 'col-span-1' :
80-
field.span === 2 ? 'col-span-2' :
81-
field.span === 3 ? 'col-span-3' :
82-
field.span === 4 ? 'col-span-4' :
83-
field.span === 5 ? 'col-span-5' :
84-
field.span === 6 ? 'col-span-6' : '';
117+
// Calculate responsive span class so col-span never exceeds the visible
118+
// column count at each breakpoint, preventing implicit columns on mobile.
119+
const spanClass = getResponsiveSpanClass(field.span, effectiveColumns);
85120

86121
const displayValue = (() => {
87122
if (value === null || value === undefined) return <span className="text-muted-foreground/50 text-xs italic"></span>;
@@ -178,23 +213,6 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
178213
);
179214
};
180215

181-
// Filter out empty fields when hideEmpty is set
182-
const visibleFields = section.hideEmpty
183-
? section.fields.filter((field) => {
184-
const value = data?.[field.name] ?? field.value;
185-
return value !== null && value !== undefined && value !== '';
186-
})
187-
: section.fields;
188-
189-
// Hide entire section when all fields are empty
190-
if (visibleFields.length === 0) return null;
191-
192-
// Apply auto-layout: infer columns and auto-span wide fields
193-
const { fields: layoutFields, columns: effectiveColumns } = applyDetailAutoLayout(
194-
visibleFields,
195-
section.columns
196-
);
197-
198216
const content = (
199217
<div
200218
className={cn(

packages/plugin-detail/src/__tests__/DetailSection.test.tsx

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { describe, it, expect } from 'vitest';
1010
import { render, screen } from '@testing-library/react';
11-
import { DetailSection } from '../DetailSection';
11+
import { DetailSection, getResponsiveSpanClass } from '../DetailSection';
1212

1313
describe('DetailSection', () => {
1414
it('should render text fields as plain text', () => {
@@ -317,4 +317,113 @@ describe('DetailSection', () => {
317317
// Should use 'text' renderer, not 'number'
318318
expect(screen.getByText('Alice')).toBeInTheDocument();
319319
});
320+
321+
it('should use responsive span classes for wide fields in 3-column layout', () => {
322+
const section = {
323+
title: 'Wide Fields',
324+
fields: Array.from({ length: 12 }, (_, i) => ({
325+
name: `field_${i}`,
326+
label: `Field ${i}`,
327+
type: i === 5 ? 'textarea' : 'text',
328+
})),
329+
};
330+
const { container } = render(
331+
<DetailSection section={section} data={{}} />
332+
);
333+
const grid = container.querySelector('.grid');
334+
expect(grid).toBeTruthy();
335+
expect(grid!.className).toContain('lg:grid-cols-3');
336+
// Wide field (textarea) should have responsive span, not bare col-span-3
337+
const fields = container.querySelectorAll('[class*="col-span"]');
338+
fields.forEach((field) => {
339+
// No bare col-span-3 at base level — must be lg: prefixed
340+
const classes = field.className.split(/\s+/);
341+
const hasBareSpan3 = classes.some((c: string) => c === 'col-span-3');
342+
expect(hasBareSpan3).toBe(false);
343+
});
344+
});
345+
346+
it('should use responsive span classes for wide fields in 2-column layout', () => {
347+
const section = {
348+
title: 'Wide Fields',
349+
fields: [
350+
{ name: 'a', label: 'A', type: 'text' },
351+
{ name: 'b', label: 'B', type: 'text' },
352+
{ name: 'c', label: 'C', type: 'text' },
353+
{ name: 'd', label: 'D', type: 'text' },
354+
{ name: 'notes', label: 'Notes', type: 'textarea' },
355+
],
356+
};
357+
const { container } = render(
358+
<DetailSection section={section} data={{}} />
359+
);
360+
const grid = container.querySelector('.grid');
361+
expect(grid!.className).toContain('md:grid-cols-2');
362+
// Wide field should have md:col-span-2, not bare col-span-2
363+
const fields = container.querySelectorAll('[class*="col-span"]');
364+
fields.forEach((field) => {
365+
const classes = field.className.split(/\s+/);
366+
const hasBareSpan2 = classes.some((c: string) => c === 'col-span-2');
367+
expect(hasBareSpan2).toBe(false);
368+
});
369+
});
370+
371+
it('should not apply col-span at base breakpoint to prevent implicit grid columns on mobile', () => {
372+
const section = {
373+
title: 'Mobile Safe',
374+
fields: Array.from({ length: 15 }, (_, i) => ({
375+
name: `field_${i}`,
376+
label: `Field ${i}`,
377+
type: i === 0 ? 'textarea' : 'text',
378+
})),
379+
};
380+
const { container } = render(
381+
<DetailSection section={section} data={{}} />
382+
);
383+
// Ensure no bare col-span-N (N>1) classes without responsive prefix
384+
const allElements = container.querySelectorAll('*');
385+
allElements.forEach((el) => {
386+
const classes = el.className?.split?.(/\s+/) || [];
387+
classes.forEach((cls: string) => {
388+
if (cls.match(/^col-span-[2-9]$/)) {
389+
throw new Error(`Found bare "${cls}" class without responsive prefix — would break mobile single-column layout`);
390+
}
391+
});
392+
});
393+
});
394+
});
395+
396+
describe('getResponsiveSpanClass', () => {
397+
it('should return empty string for no span', () => {
398+
expect(getResponsiveSpanClass(undefined, 2)).toBe('');
399+
});
400+
401+
it('should return empty string for span=1', () => {
402+
expect(getResponsiveSpanClass(1, 3)).toBe('');
403+
});
404+
405+
it('should return empty string for 1-column layout', () => {
406+
expect(getResponsiveSpanClass(3, 1)).toBe('');
407+
});
408+
409+
it('should return md:col-span-2 for span=2 in 2-column layout', () => {
410+
expect(getResponsiveSpanClass(2, 2)).toBe('md:col-span-2');
411+
});
412+
413+
it('should cap span to 2 in 2-column layout', () => {
414+
expect(getResponsiveSpanClass(3, 2)).toBe('md:col-span-2');
415+
expect(getResponsiveSpanClass(6, 2)).toBe('md:col-span-2');
416+
});
417+
418+
it('should return md:col-span-2 for span=2 in 3-column layout', () => {
419+
expect(getResponsiveSpanClass(2, 3)).toBe('md:col-span-2');
420+
});
421+
422+
it('should return responsive classes for span=3 in 3-column layout', () => {
423+
expect(getResponsiveSpanClass(3, 3)).toBe('md:col-span-2 lg:col-span-3');
424+
});
425+
426+
it('should cap span to 3 in 3-column layout', () => {
427+
expect(getResponsiveSpanClass(6, 3)).toBe('md:col-span-2 lg:col-span-3');
428+
});
320429
});

0 commit comments

Comments
 (0)