Skip to content
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

Jump to bottom

[DataGrid] Column header design updates #14293

Merged
merged 18 commits into from
Sep 3, 2024
Merged

[DataGrid] Column header design updates #14293

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
77250ac
[DataGrid] Column header design updates
KenanYusuf Aug 22, 2024
6867cc1
fix shrinking separator
KenanYusuf Aug 22, 2024
ec97638
adjust column header button positions
KenanYusuf Aug 22, 2024
59e4b1f
update column separator style and behavior
KenanYusuf Aug 23, 2024
c30292b
restore column header moving style
KenanYusuf Aug 23, 2024
dc03720
restore showColumnVerticalBorder style
KenanYusuf Aug 23, 2024
89154b3
undo last change
KenanYusuf Aug 23, 2024
bfcb7f6
improve touch styles
KenanYusuf Aug 23, 2024
0ef49b3
reduce repetition in styles
KenanYusuf Aug 27, 2024
81a44dd
move static styles
KenanYusuf Aug 27, 2024
14345bb
hide overflow when there is horizontal scrolling
KenanYusuf Aug 27, 2024
129f30f
add transition to icon separator
KenanYusuf Aug 27, 2024
035e9cf
Merge branch 'master' into column-header-design-updates
KenanYusuf Aug 27, 2024
b194f5d
update tests
KenanYusuf Aug 27, 2024
4303b0e
add ignore to new class
KenanYusuf Aug 27, 2024
d18c885
hide separator for pinned right columns
KenanYusuf Aug 28, 2024
7a37ce8
fix showColumnVerticalBorder prop
KenanYusuf Aug 28, 2024
8051e7c
improve prop descriptions
KenanYusuf Aug 28, 2024
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
9 changes: 3 additions & 6 deletions packages/x-data-grid-premium/src/components/GridAggregationHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,9 @@ const GridAggregationFunctionLabel = styled('div', {
})<{ ownerState: OwnerState }>(({ theme }) => {
return {
fontSize: theme.typography.caption.fontSize,
lineHeight: theme.typography.caption.fontSize,
position: 'absolute',
bottom: 4,
fontWeight: theme.typography.fontWeightMedium,
color: (theme.vars || theme).palette.primary.dark,
textTransform: 'uppercase',
lineHeight: 'normal',
Copy link
Member

@MBilalShafi MBilalShafi Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm wondering if there is a possibility some users customizing it using theme.*.* variables. What do you think? 🤔

Copy link
Contributor Author

@KenanYusuf KenanYusuf Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm purposefully deviating from the theme.typography.caption.lineHeight value here because it is too large. normal line-height should keep the spacing looking ok between the column header title and aggregation label I think.

If users want to override it, they have the class I guess?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, but I'm only worried if some existing users' styles would break with this.
It's obviously not very likely but theoretically, some users may be using theme.typography.caption.lineHeight to override this style.

It's a nitpick though. Feel free to merge it in current shape.

color: theme.palette.text.secondary,
marginTop: -1,
};
});

Expand Down
16 changes: 8 additions & 8 deletions packages/x-data-grid-pro/src/tests/columns.DataGridPro.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,11 @@ describe('<DataGridPro /> - Columns', () => {
await microtasks();
expect(onColumnWidthChange.callCount).to.be.at.least(2);
const widthArgs = onColumnWidthChange.args.map((arg) => arg[0].width);
const isWidth116Present = widthArgs.some((width) => width === 116);
expect(isWidth116Present).to.equal(true);
const isWidth114Present = widthArgs.some((width) => width === 114);
expect(isWidth114Present).to.equal(true);
const colDefWidthArgs = onColumnWidthChange.args.map((arg) => arg[0].colDef.width);
const isColDefWidth116Present = colDefWidthArgs.some((width) => width === 116);
expect(isColDefWidth116Present).to.equal(true);
const isColDefWidth114Present = colDefWidthArgs.some((width) => width === 114);
expect(isColDefWidth114Present).to.equal(true);
});

it('should not affect other cell elements that are not part of the main DataGrid instance', () => {
Expand Down Expand Up @@ -473,7 +473,7 @@ describe('<DataGridPro /> - Columns', () => {
render(<Test rows={rows} columns={columns} />);
await apiRef.current.autosizeColumns();
await microtasks();
expect(getWidths()).to.deep.equal([213, 235]);
expect(getWidths()).to.deep.equal([211, 233]);
});

it('should work through double-clicking the separator', async () => {
Expand All @@ -483,14 +483,14 @@ describe('<DataGridPro /> - Columns', () => {
)[1];
fireEvent.doubleClick(separator);
await microtasks();
expect(getWidths()).to.deep.equal([100, 235]);
expect(getWidths()).to.deep.equal([100, 233]);
});

it('should work on mount', async () => {
render(<Test rows={rows} columns={columns} autosizeOnMount />);
await microtasks(); /* first effect after render */
await microtasks(); /* async autosize operation */
expect(getWidths()).to.deep.equal([213, 235]);
expect(getWidths()).to.deep.equal([211, 233]);
});

describe('options', () => {
Expand All @@ -506,7 +506,7 @@ describe('<DataGridPro /> - Columns', () => {
});

it('.includeHeaders works', async () => {
await autosize({ includeHeaders: true }, [213, 235]);
await autosize({ includeHeaders: true }, [211, 233]);
});

it('.includeOutliers works', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/x-data-grid/src/components/columnHeaders/ColumnHeaderMenuIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const ColumnHeaderMenuIcon = React.memo((props: ColumnHeaderMenuIconProps
id={columnMenuButtonId}
{...rootProps.slotProps?.baseIconButton}
>
<rootProps.slots.columnMenuIcon fontSize="small" />
<rootProps.slots.columnMenuIcon fontSize="inherit" />
</rootProps.slots.baseIconButton>
</rootProps.slots.baseTooltip>
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/x-data-grid/src/components/columnHeaders/GridColumnHeaderSeparator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function GridColumnHeaderSeparatorRaw(props: GridColumnHeaderSeparatorProps) {
// eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions
<div
className={classes.root}
style={{ minHeight: height, opacity: rootProps.showColumnVerticalBorder ? 0 : 1 }}
style={{ minHeight: height, opacity: rootProps.showColumnVerticalBorder ? 0 : undefined }}
MBilalShafi marked this conversation as resolved.
Show resolved Hide resolved
{...other}
onClick={stopClick}
>
Expand Down
1 change: 1 addition & 0 deletions packages/x-data-grid/src/components/columnHeaders/GridColumnHeaderTitle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const GridColumnHeaderTitleRoot = styled('div', {
overflow: 'hidden',
whiteSpace: 'nowrap',
fontWeight: 'var(--unstable_DataGrid-headWeight)',
lineHeight: 'normal',
});

const ColumnHeaderInnerTitle = React.forwardRef<
Expand Down
110 changes: 71 additions & 39 deletions packages/x-data-grid/src/components/containers/GridRootStyles.ts
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Side note: Sometimes Argos diff surprises me – it doesn't pick up the separator 😅

Screen.Recording.2024-08-23.at.23.36.55.mov

Copy link
Member

Choose a reason for hiding this comment

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

Also, this PR warrants a minor release.

Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ function getBorderColor(theme: Theme) {
return darken(alpha(theme.palette.divider, 1), 0.68);
}

const columnHeadersStyles = {
[`.${c.columnSeparator}, .${c['columnSeparator--resizing']}`]: {
visibility: 'visible',
width: 'auto',
},
};

const columnHeaderStyles = {
[`& .${c.iconButtonContainer}`]: {
visibility: 'visible',
Expand All @@ -45,6 +38,12 @@ const columnHeaderStyles = {
},
};

const separatorIconDragStyles = {
width: 3,
rx: 1.5,
x: 10.5,
};

// Emotion thinks it knows better than us which selector we should use.
// https://github.com/emotion-js/emotion/issues/1105#issuecomment-1722524968
const ignoreSsrWarning =
Expand Down Expand Up @@ -188,6 +187,10 @@ export const GridRootStyles = styled('div', {
},
};

const focusOutlineWidth = 1;
const columnSeparatorTargetSize = 10;
const columnSeparatorOffset = -5;

const gridStyle: CSSInterpolation = {
'--unstable_DataGrid-radius': typeof radius === 'number' ? `${radius}px` : radius,
'--unstable_DataGrid-headWeight': t.typography.fontWeightMedium,
Expand Down Expand Up @@ -264,7 +267,6 @@ export const GridRootStyles = styled('div', {
},
[`& .${c.columnHeader}, & .${c.cell}`]: {
WebkitTapHighlightColor: 'transparent',
lineHeight: null,
padding: '0 10px',
boxSizing: 'border-box',
},
Expand All @@ -273,12 +275,35 @@ export const GridRootStyles = styled('div', {
t.vars
? `rgba(${t.vars.palette.primary.mainChannel} / 0.5)`
: alpha(t.palette.primary.main, 0.5)
} 1px`,
outlineWidth: 1,
outlineOffset: -1,
} ${focusOutlineWidth}px`,
outlineOffset: focusOutlineWidth * -1,
},
[`& .${c.columnHeader}:focus, & .${c.cell}:focus`]: {
outline: `solid ${t.palette.primary.main} 1px`,
outline: `solid ${t.palette.primary.main} ${focusOutlineWidth}px`,
outlineOffset: focusOutlineWidth * -1,
},
// Hide the column separator when:
// - the column is focused and has an outline
// - the next column is focused and has an outline
// - the column has a right border
// - the next column has a right border
[`& .${c.columnHeader}:focus,
& .${c.columnHeader}:focus-within,
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus),
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus-within),
& .${c['columnHeader--withRightBorder']},
& .${c.columnHeader}:has(+ .${c['columnHeader--withRightBorder']})`]: {
[`& .${c.columnSeparator}`]: {
opacity: 0,
'@media (hover: none)': {
opacity: 1,
color: (t.vars || t).palette.primary.main,
},
},
// Show resizable separators again when the column is hovered
[`& .${c['columnSeparator--resizable']}:hover`]: {
opacity: 1,
},
},
[`&.${c['root--noToolbar']} [aria-rowindex="1"] [aria-colindex="1"]`]: {
borderTopLeftRadius: 'calc(var(--unstable_DataGrid-radius) - 1px)',
Expand All @@ -299,7 +324,7 @@ export const GridRootStyles = styled('div', {
display: 'flex',
alignItems: 'center',
},
[`& .${c['columnHeader--last']}`]: {
[`& .${c['virtualScroller--hasScrollX']} .${c['columnHeader--last']}`]: {
overflow: 'hidden',
},
KenanYusuf marked this conversation as resolved.
Show resolved Hide resolved
[`& .${c['columnHeader--sorted']} .${c.iconButtonContainer}, & .${c['columnHeader--filtered']} .${c.iconButtonContainer}`]:
Expand All @@ -316,12 +341,11 @@ export const GridRootStyles = styled('div', {
[`& .${c.columnHeaderTitleContainer}`]: {
display: 'flex',
alignItems: 'center',
gap: t.spacing(0.25),
minWidth: 0,
flex: 1,
whiteSpace: 'nowrap',
overflow: 'hidden',
// to anchor the aggregation label
position: 'relative',
},
[`& .${c.columnHeaderTitleContainerContent}`]: {
overflow: 'hidden',
Expand All @@ -346,16 +370,13 @@ export const GridRootStyles = styled('div', {
{
flexDirection: 'row-reverse',
},
[`& .${c['columnHeader--alignCenter']} .${c.menuIcon}, & .${c['columnHeader--alignRight']} .${c.menuIcon}`]:
{
marginRight: 'auto',
marginLeft: -6,
},
[`& .${c['columnHeader--alignRight']} .${c.menuIcon}, & .${c['columnHeader--alignRight']} .${c.menuIcon}`]:
{
marginRight: 'auto',
marginLeft: -10,
},
[`& .${c['columnHeader--alignCenter']} .${c.menuIcon}`]: {
marginLeft: 'auto',
},
[`& .${c['columnHeader--alignRight']} .${c.menuIcon}`]: {
marginRight: 'auto',
marginLeft: -5,
},
[`& .${c['columnHeader--moving']}`]: {
backgroundColor: (t.vars || t).palette.action.hover,
},
Expand All @@ -365,59 +386,70 @@ export const GridRootStyles = styled('div', {
background: 'var(--DataGrid-pinnedBackground)',
},
[`& .${c.columnSeparator}`]: {
visibility: 'hidden',
position: 'absolute',
overflow: 'hidden',
zIndex: 3,
display: 'flex',
flexDirection: 'column',
justifyContent: 'center',
alignItems: 'center',
maxWidth: columnSeparatorTargetSize,
color: borderColor,
},
[`& .${c.columnHeaders}`]: {
width: 'var(--DataGrid-rowWidth)',
},
'@media (hover: hover)': {
[`& .${c.columnHeaders}:hover`]: columnHeadersStyles,
[`& .${c.columnHeader}:hover`]: columnHeaderStyles,
[`& .${c.columnHeader}:not(.${c['columnHeader--sorted']}):hover .${c.sortIcon}`]: {
opacity: 0.5,
},
},
'@media (hover: none)': {
[`& .${c.columnHeaders}`]: columnHeadersStyles,
[`& .${c.columnHeader}`]: columnHeaderStyles,
},
[`& .${c['columnSeparator--sideLeft']}`]: {
left: -12,
left: columnSeparatorOffset,
},
[`& .${c['columnSeparator--sideRight']}`]: {
right: -12,
right: columnSeparatorOffset,
},
[`& .${c['columnHeader--withRightBorder']} .${c['columnSeparator--sideLeft']}`]: {
left: columnSeparatorOffset - 0.5,
},
[`& .${c['columnHeader--withRightBorder']} .${c['columnSeparator--sideRight']}`]: {
right: columnSeparatorOffset - 0.5,
},
[`& .${c['columnSeparator--resizable']}`]: {
cursor: 'col-resize',
touchAction: 'none',
'&:hover': {
color: (t.vars || t).palette.text.primary,
// Reset on touch devices, it doesn't add specificity
// Always appear as draggable on touch devices
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

'@media (hover: none)': {
[`& .${c.iconSeparator} rect`]: separatorIconDragStyles,
color: borderColor,
},
[`&:hover, &.${c['columnSeparator--resizing']}`]: {
color: (t.vars || t).palette.primary.main,
KenanYusuf marked this conversation as resolved.
Show resolved Hide resolved
[`& .${c.iconSeparator} rect`]: separatorIconDragStyles,
'@media (hover: none)': {
color: borderColor,
},
},
[`&.${c['columnSeparator--resizing']}`]: {
color: (t.vars || t).palette.text.primary,
},
'& svg': {
pointerEvents: 'none',
},
},
[`& .${c.iconSeparator}`]: {
color: 'inherit',
transition: t.transitions.create(['color', 'width'], {
duration: t.transitions.duration.shortest,
}),
},
[`& .${c.menuIcon}`]: {
width: 0,
visibility: 'hidden',
fontSize: 20,
marginRight: -10,
marginRight: -5,
display: 'flex',
alignItems: 'center',
},
Expand Down Expand Up @@ -511,8 +543,8 @@ export const GridRootStyles = styled('div', {
boxShadow: t.shadows[2],
backgroundColor: (t.vars || t).palette.background.paper,
'&:focus-within': {
outline: `solid ${(t.vars || t).palette.primary.main} 1px`,
outlineOffset: '-1px',
outline: `${focusOutlineWidth}px solid ${(t.vars || t).palette.primary.main}`,
outlineOffset: focusOutlineWidth * -1,
},
},
[`& .${c['row--editing']}`]: {
Expand Down
3 changes: 1 addition & 2 deletions packages/x-data-grid/src/components/virtualization/GridVirtualScroller.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ const useUtilityClasses = (
loadingOverlayVariant: GridLoadingOverlayVariant | null,
) => {
const { classes } = ownerState;

const slots = {
root: [
'main',
dimensions.rightPinnedWidth > 0 && 'main--hasPinnedRight',
loadingOverlayVariant === 'skeleton' && 'main--hasSkeletonLoadingOverlay',
],
scroller: ['virtualScroller'],
scroller: ['virtualScroller', dimensions.hasScrollX && 'virtualScroller--hasScrollX'],
};

return composeClasses(slots, getDataGridUtilityClass, classes);
Expand Down
6 changes: 6 additions & 0 deletions packages/x-data-grid/src/constants/gridClasses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,11 @@ export interface GridClasses {
* Styles applied to the virtualization container.
*/
virtualScroller: string;
/**
* Styles applied to the virtualization container when it is scrollable in the horizontal direction.
* @ignore - do not document.
*/
'virtualScroller--hasScrollX': string;
/**
* Styles applied to the virtualization content.
*/
Expand Down Expand Up @@ -770,6 +775,7 @@ export const gridClasses = generateUtilityClasses<GridClassKey>('MuiDataGrid', [
'toolbarContainer',
'toolbarFilterList',
'virtualScroller',
'virtualScroller--hasScrollX',
'virtualScrollerContent',
'virtualScrollerContent--overflowed',
'virtualScrollerRenderZone',
Expand Down
5 changes: 4 additions & 1 deletion packages/x-data-grid/src/material/icons/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ export const GridColumnIcon = createSvgIcon(
'ColumnIcon',
);

export const GridSeparatorIcon = createSvgIcon(<path d="M11 19V5h2v14z" />, 'Separator');
export const GridSeparatorIcon = createSvgIcon(
<rect width="1" height="24" x="11.5" rx="0.5" />,
'Separator',
);

export const GridViewHeadlineIcon = createSvgIcon(
<path d="M4 15h16v-2H4v2zm0 4h16v-2H4v2zm0-8h16V9H4v2zm0-6v2h16V5H4z" />,
Expand Down