Skip to content

[DataTableToolbar] Fix mobile single-row layout and add compactTrailing #1790

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.

Sign up for GitHub

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
KhushamBansal merged 3 commits into from
Aug 14, 2026
+150 −31
Merged

[DataTableToolbar] Fix mobile single-row layout and add compactTrailing #1790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e170fb2
[DataTableToolbar] Fix mobile single-row layout and add compactTrailing
KhushamBansal Aug 11, 2026
c786863
fix(DataTableToolbar): remove dead guard, add testids, internalize mo…
KhushamBansal Aug 11, 2026
70ede88
Merge branch 'master' into fix/data-table-toolbar-mobile-layout
KhushamBansal Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 79 additions & 17 deletions src/__testing__/DataTableToolbar.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,19 @@ jest.mock('@sistent/mui-datatables', () => ({
default: () => null
}));

let mockViewportWidth = 1200;

jest.mock('../custom/Helpers/Dimension', () => ({
useWindowDimensions: () => ({ width: mockViewportWidth, height: 800 })
}));

const renderWithTheme = (ui: React.ReactElement) =>
render(<SistentThemeProvider>{ui}</SistentThemeProvider>);

describe('DataTableToolbar', () => {
beforeEach(() => {
mockViewportWidth = 1200;
});
it('renders primaryActions content', () => {
renderWithTheme(<DataTableToolbar primaryActions={<button>Add</button>} />);
expect(screen.getByRole('button', { name: 'Add' })).toBeTruthy();
Expand Down Expand Up @@ -135,19 +144,13 @@ describe('DataTableToolbar', () => {
describe('layout positioning', () => {
it('pushes right section to the right when only right content is present', () => {
renderWithTheme(<DataTableToolbar search={<span data-testid="right-content">Search</span>} />);
const rightContent = screen.getByTestId('right-content');
const rightSection = rightContent.parentElement as HTMLElement;
// RightSection has marginLeft: auto — check via computed style
expect(rightSection).toBeTruthy();
const rightSection = screen.getByTestId('data-table-toolbar-right-section');
expect(window.getComputedStyle(rightSection).marginLeft).toBe('auto');
});

it('keeps left content on the left when only left content is present', () => {
renderWithTheme(<DataTableToolbar primaryActions={<button data-testid="left-content">Add</button>} />);
const leftContent = screen.getByTestId('left-content');
const leftSection = leftContent.parentElement as HTMLElement;
// Default Section has no marginLeft override
expect(leftSection).toBeTruthy();
const leftSection = screen.getByTestId('data-table-toolbar-left-section');
expect(window.getComputedStyle(leftSection).marginLeft).not.toBe('auto');
});

Expand All @@ -158,17 +161,76 @@ describe('DataTableToolbar', () => {
search={<span data-testid="right-content">Search</span>}
/>
);
const leftContent = screen.getByTestId('left-btn');
const rightContent = screen.getByTestId('right-content');
const leftSection = leftContent.parentElement as HTMLElement;
const rightSection = rightContent.parentElement as HTMLElement;

expect(leftSection).toBeTruthy();
expect(rightSection).toBeTruthy();
// Left section has no auto margin
const leftSection = screen.getByTestId('data-table-toolbar-left-section');
const rightSection = screen.getByTestId('data-table-toolbar-right-section');

expect(window.getComputedStyle(leftSection).marginLeft).not.toBe('auto');
// Right section has auto margin to push it right
expect(window.getComputedStyle(rightSection).marginLeft).toBe('auto');
});

it('hides trailing controls when compactTrailing is true', () => {
renderWithTheme(
<DataTableToolbar
search={<span data-testid="search-slot">Search</span>}
filter={<span data-testid="filter-slot">Filter</span>}
viewSwitch={<span data-testid="view-switch">Grid/Table</span>}
compactTrailing
/>
);
expect(screen.getByTestId('search-slot')).toBeTruthy();
expect(screen.queryByTestId('filter-slot')).toBeNull();
expect(screen.queryByTestId('view-switch')).toBeNull();
expect(screen.queryByTestId('data-table-toolbar-trailing-controls')).toBeNull();
});

it('groups search and trailing controls in the right controls group', () => {
renderWithTheme(
<DataTableToolbar
primaryActions={<button data-testid="left-btn">Add</button>}
search={<span data-testid="search-slot">Search</span>}
viewSwitch={<span data-testid="view-switch">Grid/Table</span>}
compactTrailing={false}
/>
);

const controlsGroup = screen.getByTestId('data-table-toolbar-right-controls');
const rightSection = screen.getByTestId('data-table-toolbar-right-section');

expect(controlsGroup.contains(screen.getByTestId('search-slot'))).toBe(true);
expect(controlsGroup.contains(screen.getByTestId('view-switch'))).toBe(true);
expect(rightSection.contains(controlsGroup)).toBe(true);
expect(window.getComputedStyle(rightSection).marginLeft).toBe('auto');
});

it('auto-hides trailing controls on narrow viewports when compactTrailing is omitted', () => {
mockViewportWidth = 400;

renderWithTheme(
<DataTableToolbar
search={<span data-testid="search-slot">Search</span>}
viewSwitch={<span data-testid="view-switch">Grid/Table</span>}
/>
);

expect(screen.getByTestId('search-slot')).toBeTruthy();
expect(screen.queryByTestId('view-switch')).toBeNull();
expect(screen.queryByTestId('data-table-toolbar-trailing-controls')).toBeNull();
});

it('keeps trailing controls visible on narrow viewports when compactTrailing is false', () => {
mockViewportWidth = 400;

renderWithTheme(
<DataTableToolbar
search={<span data-testid="search-slot">Search</span>}
viewSwitch={<span data-testid="view-switch">Grid/Table</span>}
compactTrailing={false}
/>
);

expect(screen.getByTestId('search-slot')).toBeTruthy();
expect(screen.getByTestId('view-switch')).toBeTruthy();
expect(screen.getByTestId('data-table-toolbar-trailing-controls')).toBeTruthy();
});
});
});
77 changes: 63 additions & 14 deletions src/custom/DataTableToolbar/DataTableToolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ const ToolbarRoot = styled(Box)(({ theme }) => ({
boxShadow: theme.shadows[2],

[theme.breakpoints.down('sm')]: {
height: 'auto',
flexWrap: 'wrap',
flexWrap: 'nowrap',
padding: theme.spacing(1),
gap: theme.spacing(1)
}
Expand All @@ -30,13 +29,43 @@ const ToolbarRoot = styled(Box)(({ theme }) => ({
const Section = styled(Box)(({ theme }) => ({
display: 'flex',
alignItems: 'center',
gap: theme.spacing(1),
minWidth: 0
}));

const RightSection = styled(Section)(({ theme }) => ({
marginLeft: 'auto',
flexWrap: 'nowrap',
flexShrink: 1,
minWidth: 0,
justifyContent: 'flex-end',

[theme.breakpoints.down('sm')]: {
paddingLeft: theme.spacing(1)
}
}));

const RightControlsGroup = styled(Box)(({ theme }) => ({
display: 'flex',
alignItems: 'center',
flexShrink: 1,
minWidth: 0,
gap: theme.spacing(1)
}));

const RightSection = styled(Section)({
marginLeft: 'auto'
const SearchSlot = styled(Box)({
flex: '0 1 auto',
minWidth: 0,
maxWidth: '15rem'

@pontusringblom pontusringblom Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did we arrive at 15rem?

Should this be using a theme.spacing or breakpoint size instead?

@KhushamBansal KhushamBansal Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pontusringblom That’s the expanded width we already use for SearchBar.

});

const TrailingControls = styled(Box)(({ theme }) => ({
display: 'flex',
alignItems: 'center',
flexShrink: 0,
gap: theme.spacing(1)
}));

export function DataTableToolbar({
primaryActions,
secondaryActions,
Expand All @@ -45,6 +74,7 @@ export function DataTableToolbar({
filter,
columnVisibility,
viewSwitch,
compactTrailing,
searchHelperText,
tabs,
columns,
Expand All @@ -54,6 +84,9 @@ export function DataTableToolbar({
}: DataTableToolbarProps): JSX.Element {
const theme = useTheme();
const { width: viewportWidth } = useWindowDimensions();
const isNarrowViewport =
viewportWidth > 0 && viewportWidth < theme.breakpoints.values.sm;
const effectiveCompactTrailing = compactTrailing ?? isNarrowViewport;
Comment on lines +87 to +89

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reset the column menu state when compact mode hides the control.

effectiveCompactTrailing removes columnControl from the tree, but it does not reset dropdownOpen or anchorEl. If the menu is open before a narrow resize, the next wide render mounts PopperListener with open={true} and a detached anchor. The menu can reopen at an incorrect position.

Clear both states when effectiveCompactTrailing becomes true. Add a resize regression test.

Proposed state reset 
   const [dropdownOpen, setDropdownOpen] = React.useState(false);
   const [anchorEl, setAnchorEl] = React.useState<HTMLElement | null>(null);
+
+  React.useEffect(() => {
+    if (effectiveCompactTrailing) {
+      setAnchorEl(null);
+      setDropdownOpen(false);
+    }
+  }, [effectiveCompactTrailing]);

Also applies to: 199-201, 221-225

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/custom/DataTableToolbar/DataTableToolbar.tsx` around lines 87 - 89, Reset
the column menu state when compact mode hides its control: in the
DataTableToolbar logic around effectiveCompactTrailing, clear both dropdownOpen
and anchorEl whenever effectiveCompactTrailing is true, preventing a detached
menu from reopening after the viewport widens. Add a regression test covering an
open menu followed by a narrow resize and subsequent wide render.


// Compute auto-hide visibility from columns config + viewport width
const autoHideVisibility = React.useMemo(() => {
Expand Down Expand Up @@ -155,27 +188,43 @@ export function DataTableToolbar({
columnVisibility
);

const trailingControls = (
<>
{filter}
{columnControl}
{viewSwitch}
</>
);

const hasTrailingControls =
!effectiveCompactTrailing &&
(Boolean(filter) || Boolean(columnControl) || Boolean(viewSwitch));

const hasLeftContent = Boolean(primaryActions);
const hasRightContent =
Boolean(bulkOperations) ||
Boolean(secondaryActions) ||
Boolean(filter) ||
Boolean(search) ||
Boolean(columnControl) ||
Boolean(viewSwitch);
hasTrailingControls;

return (
<>
<ToolbarRoot sx={sx}>
{hasLeftContent && <Section>{primaryActions}</Section>}
<ToolbarRoot data-testid="data-table-toolbar" sx={sx}>
{hasLeftContent && <Section data-testid="data-table-toolbar-left-section">{primaryActions}</Section>}
{hasRightContent && (
<RightSection>
<RightSection data-testid="data-table-toolbar-right-section">
{bulkOperations}
{secondaryActions}
{search}
{filter}
{columnControl}
{viewSwitch}
{(search || hasTrailingControls) && (
<RightControlsGroup data-testid="data-table-toolbar-right-controls">
{search && <SearchSlot>{search}</SearchSlot>}
{hasTrailingControls && (
<TrailingControls data-testid="data-table-toolbar-trailing-controls">
{trailingControls}
</TrailingControls>
)}
</RightControlsGroup>
)}
</RightSection>
)}
</ToolbarRoot>
Expand Down
8 changes: 8 additions & 0 deletions src/custom/DataTableToolbar/DataTableToolbar.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ export interface DataTableToolbarProps {
/** Right side: Grid/table view toggle */
viewSwitch?: React.ReactNode;

/**
* Controls visibility of filter, column visibility, and view switch on narrow
* viewports. When omitted, trailing controls auto-hide below the MUI `sm`
* breakpoint. Pass `true` to force-hide, or `false` to keep them visible even
* on narrow viewports (may overflow with single-row layout).
*/
compactTrailing?: boolean;

/** Helper text displayed below the search bar (e.g., "Search by name, kind, category") */
searchHelperText?: string;

Expand Down
Loading