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

[DataGridPro] Fix double top border in header filters #14375

Merged
merged 4 commits into from
Aug 29, 2024
+37 −13
Merged

[DataGridPro] Fix double top border in header filters #14375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a72336
[DataGridPro] Fix double top border in header filters
MBilalShafi Aug 28, 2024
829a6d9
Add a bottom border to the scrollbar filler
MBilalShafi Aug 28, 2024
b0cc110
A few changes to adjust row pinning styles
MBilalShafi Aug 29, 2024
84f4c17
Lint
MBilalShafi Aug 29, 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
4 changes: 4 additions & 0 deletions packages/x-data-grid/src/components/GridScrollbarFillerCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ const classes = {
root: gridClasses.scrollbarFiller,
header: gridClasses['scrollbarFiller--header'],
borderTop: gridClasses['scrollbarFiller--borderTop'],
borderBottom: gridClasses['scrollbarFiller--borderBottom'],
pinnedRight: gridClasses['scrollbarFiller--pinnedRight'],
};

function GridScrollbarFillerCell({
header,
borderTop = true,
borderBottom,
pinnedRight,
}: {
header?: boolean;
borderTop?: boolean;
borderBottom?: boolean;
pinnedRight?: boolean;
}) {
return (
Expand All @@ -25,6 +28,7 @@ function GridScrollbarFillerCell({
classes.root,
header && classes.header,
borderTop && classes.borderTop,
borderBottom && classes.borderBottom,
pinnedRight && classes.pinnedRight,
)}
/>
Expand Down
16 changes: 12 additions & 4 deletions packages/x-data-grid/src/components/containers/GridRootStyles.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor issue, if you look close enough the border doesn't extend all the way over the vertical scrollbar. If you address this, also make sure it looks fine with top pinned rows.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

With the new changes it messes the border in bottom pinned rows. I don't think we should be removing the top-border where there is one, it seems like we should be adding a bottom-border where one is needed.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

With the new changes it messes the border in bottom pinned rows. I don't think we should be removing the top-border where there is one, it seems like we should be adding a bottom-border where one is needed.

Yes, I noted it too. On it. Thanks for identifying.

Copy link
Member Author

@MBilalShafi MBilalShafi Aug 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Did some updates, and it looks good to me now, tested in https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-nf5rcj?file=%2Fsrc%2Fdemo.tsx%3A75%2C1

Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ export const GridRootStyles = styled('div', {
[`& .${c.headerFilterRow}`]: {
[`& .${c.columnHeader}`]: {
boxSizing: 'border-box',
borderTop: '1px solid var(--DataGrid-rowBorderColor)',
borderBottom: '1px solid var(--DataGrid-rowBorderColor)',
},
},

Expand Down Expand Up @@ -491,6 +491,11 @@ export const GridRootStyles = styled('div', {
[`& .${c['virtualScrollerContent--overflowed']} .${c['row--lastVisible']} .${c.cell}`]: {
borderTopColor: 'transparent',
},
[`& .${c['pinnedRows--top']} :first-of-type`]: {
[`& .${c.cell}, .${c.scrollbarFiller}`]: {
borderTop: 'none',
},
},
[`&.${c['root--disableUserSelection']} .${c.cell}`]: {
userSelect: 'none',
},
Expand Down Expand Up @@ -674,7 +679,10 @@ export const GridRootStyles = styled('div', {
minWidth: 'calc(var(--DataGrid-hasScrollY) * var(--DataGrid-scrollbarSize))',
alignSelf: 'stretch',
[`&.${c['scrollbarFiller--borderTop']}`]: {
borderTop: '1px solid var(--rowBorderColor)',
borderTop: '1px solid var(--DataGrid-rowBorderColor)',
},
[`&.${c['scrollbarFiller--borderBottom']}`]: {
borderBottom: '1px solid var(--DataGrid-rowBorderColor)',
},
[`&.${c['scrollbarFiller--pinnedRight']}`]: {
backgroundColor: 'var(--DataGrid-pinnedBackground)',
Expand All @@ -686,8 +694,8 @@ export const GridRootStyles = styled('div', {
[`& .${c.filler}`]: {
flex: 1,
},
[`& .${c['filler--borderTop']}`]: {
borderTop: '1px solid var(--DataGrid-rowBorderColor)',
[`& .${c['filler--borderBottom']}`]: {
borderBottom: '1px solid var(--DataGrid-rowBorderColor)',
},

/* Hide grid rows, row filler, and vertical scrollbar when skeleton overlay is visible */
Expand Down
12 changes: 9 additions & 3 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 @@ -297,10 +297,10 @@ export interface GridClasses {
*/
filler: string;
/**
* Styles applied to the filler row with top border.
* Styles applied to the filler row with bottom border.
* @ignore - do not document.
*/
'filler--borderTop': string;
'filler--borderBottom': string;
/**
* Styles applied to the filler row pinned left section.
* @ignore - do not document.
Expand Down Expand Up @@ -546,6 +546,11 @@ export interface GridClasses {
* Styles applied to the scrollbar filler cell, with a border top.
*/
'scrollbarFiller--borderTop': string;
/**
* @ignore - do not document.
* Styles applied to the scrollbar filler cell, with a border bottom.
*/
'scrollbarFiller--borderBottom': string;
/**
* @ignore - do not document.
* Styles applied to the scrollbar filler cell.
Expand Down Expand Up @@ -709,7 +714,7 @@ export const gridClasses = generateUtilityClasses<GridClassKey>('MuiDataGrid', [
'editBooleanCell',
'editInputCell',
'filler',
'filler--borderTop',
'filler--borderBottom',
'filler--pinnedLeft',
'filler--pinnedRight',
'filterForm',
Expand Down Expand Up @@ -764,6 +769,7 @@ export const gridClasses = generateUtilityClasses<GridClassKey>('MuiDataGrid', [
'scrollbarFiller',
'scrollbarFiller--header',
'scrollbarFiller--borderTop',
'scrollbarFiller--borderBottom',
'scrollbarFiller--pinnedRight',
'selectedRowCount',
'sortIcon',
Expand Down
18 changes: 12 additions & 6 deletions packages/x-data-grid/src/hooks/features/columnHeaders/useGridColumnHeaders.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
gridColumnPositionsSelector,
gridVisiblePinnedColumnDefinitionsSelector,
} from '../columns';
import { gridPinnedRowsSelector } from '../rows/gridRowsSelector';
import { GridGroupingStructure } from '../columnGrouping/gridColumnGroupsInterfaces';
import { gridColumnGroupsUnwrappedModelSelector } from '../columnGrouping/gridColumnGroupsSelector';
import { GridScrollbarFillerCell as ScrollbarFiller } from '../../../components/GridScrollbarFillerCell';
Expand Down Expand Up @@ -106,7 +105,6 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
const columnPositions = useGridSelector(apiRef, gridColumnPositionsSelector);
const renderContext = useGridSelector(apiRef, gridRenderContextColumnsSelector);
const pinnedColumns = useGridSelector(apiRef, gridVisiblePinnedColumnDefinitionsSelector);
const pinnedRows = useGridSelector(apiRef, gridPinnedRowsSelector);
const offsetLeft = computeOffsetLeft(
columnPositions,
renderContext,
Expand Down Expand Up @@ -183,7 +181,7 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
params: GetHeadersParams | undefined,
children: React.ReactNode,
leftOverflow: number,
borderTop: boolean = false,
borderBottom: boolean = false,
) => {
const isPinnedRight = params?.position === GridPinnedColumnPosition.RIGHT;
const isNotPinned = params?.position === undefined;
Expand All @@ -201,11 +199,19 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
{isNotPinned && (
<div
role="presentation"
className={clsx(gridClasses.filler, borderTop && gridClasses['filler--borderTop'])}
className={clsx(
gridClasses.filler,
borderBottom && gridClasses['filler--borderBottom'],
)}
/>
)}
{hasScrollbarFiller && (
<ScrollbarFiller header borderTop={borderTop} pinnedRight={isPinnedRight} />
<ScrollbarFiller
header
pinnedRight={isPinnedRight}
borderBottom={borderBottom}
borderTop={false}
/>
)}
</React.Fragment>
);
Expand Down Expand Up @@ -300,7 +306,7 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
role="row"
aria-rowindex={headerGroupingMaxDepth + 1}
ownerState={rootProps}
className={pinnedRows.top.length === 0 ? gridClasses['row--borderBottom'] : undefined}
className={gridClasses['row--borderBottom']}
>
{leftRenderContext &&
getColumnHeaders(
Expand Down