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

[DataGrid] Workaround for failing jsdom tests caused by :has selectors #14559

Merged
merged 8 commits into from
Sep 12, 2024
Merged

[DataGrid] Workaround for failing jsdom tests caused by :has selectors #14559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2ff55cd
[DataGrid] Replace has pseudo-class with modifier classes to work aro…
KenanYusuf Sep 10, 2024
fac91b3
gen proptypes
KenanYusuf Sep 10, 2024
8dc82f8
Merge branch 'mui:master' into replace-has-selector
KenanYusuf Sep 10, 2024
bcca78e
add comment
KenanYusuf Sep 11, 2024
2778120
Merge branch 'mui:master' into replace-has-selector
KenanYusuf Sep 11, 2024
c44e589
fix hiding last unpinned column
KenanYusuf Sep 11, 2024
78993d9
remove unnecessary subtraction
KenanYusuf Sep 11, 2024
fb005c4
Merge branch 'mui:master' into replace-has-selector
KenanYusuf Sep 12, 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
10 changes: 10 additions & 0 deletions packages/x-data-grid/src/components/columnHeaders/GridColumnHeaderItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ interface GridColumnHeaderItemProps {
indexInSection: number;
sectionLength: number;
gridHasFiller: boolean;
isLastUnpinned: boolean;
isSiblingFocused: boolean;
}

type OwnerState = GridColumnHeaderItemProps & {
Expand All @@ -56,6 +58,8 @@ const useUtilityClasses = (ownerState: OwnerState) => {
showLeftBorder,
filterItemsCounter,
pinnedPosition,
isLastUnpinned,
isSiblingFocused,
} = ownerState;

const isColumnSorted = sortDirection != null;
Expand All @@ -79,6 +83,10 @@ const useUtilityClasses = (ownerState: OwnerState) => {
showLeftBorder && 'columnHeader--withLeftBorder',
pinnedPosition === 'left' && 'columnHeader--pinnedLeft',
pinnedPosition === 'right' && 'columnHeader--pinnedRight',
// TODO: Remove classes below and restore `:has` selectors when they are supported in jsdom
// See https://github.com/mui/mui-x/pull/14559
isLastUnpinned && 'columnHeader--lastUnpinned',
isSiblingFocused && 'columnHeader--siblingFocused',
],
draggableContainer: ['columnHeaderDraggableContainer'],
titleContainer: ['columnHeaderTitleContainer'],
Expand Down Expand Up @@ -326,7 +334,9 @@ GridColumnHeaderItem.propTypes = {
indexInSection: PropTypes.number.isRequired,
isDragging: PropTypes.bool.isRequired,
isLast: PropTypes.bool.isRequired,
isLastUnpinned: PropTypes.bool.isRequired,
isResizing: PropTypes.bool.isRequired,
isSiblingFocused: PropTypes.bool.isRequired,
pinnedPosition: PropTypes.oneOf(['left', 'right']),
sectionLength: PropTypes.number.isRequired,
separatorSide: PropTypes.oneOf(['left', 'right']),
Expand Down
10 changes: 3 additions & 7 deletions packages/x-data-grid/src/components/containers/GridRootStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,10 @@ export const GridRootStyles = styled('div', {
// - the column has a left or right border
// - the next column is pinned right and has a left border
[`& .${c.columnHeader}:focus,
& .${c.columnHeader}:focus-within,
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus),
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus-within),
Copy link
Member Author

Choose a reason for hiding this comment

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

I did not attempt to work around the focus-within selectors, as I don't think it is worth the added JS to replicate this effect.

& .${c['columnHeader--withLeftBorder']},
& .${c['columnHeader--withRightBorder']},
& .${c.columnHeader}:has(+ .${c.filler} + .${c['columnHeader--withLeftBorder']}),
& .${c['columnHeader--siblingFocused']},
& .${c['virtualScroller--hasScrollX']} .${c['columnHeader--lastUnpinned']},
& .${c['virtualScroller--hasScrollX']} .${c['columnHeader--last']}
`]: {
[`& .${c.columnSeparator}`]: {
Expand Down Expand Up @@ -413,9 +411,7 @@ export const GridRootStyles = styled('div', {
'@media (hover: none)': {
[`& .${c.columnHeader}`]: columnHeaderStyles,
[`& .${c.columnHeader}:focus,
& .${c.columnHeader}:focus-within,
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus),
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus-within)`]: {
& .${c['columnHeader--siblingFocused']}`]: {
[`.${c['columnSeparator--resizable']}`]: {
color: (t.vars || t).palette.primary.main,
},
Expand Down
13 changes: 13 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 @@ -163,6 +163,17 @@ export interface GridClasses {
* Styles applied to the last column header element.
*/
'columnHeader--last': string;
/**
* Styles applied to the last unpinned column header item.
* @ignore - do not document.
*/
'columnHeader--lastUnpinned': string;
/**
* Styles applied to a column header item when its sibling with a bordering separator is focused.
* @ignore - do not document.
*/
'columnHeader--siblingFocused': string;

/**
* Styles applied to the header checkbox cell element.
*/
Expand Down Expand Up @@ -686,6 +697,8 @@ export const gridClasses = generateUtilityClasses<GridClassKey>('MuiDataGrid', [
'columnHeader--pinnedLeft',
'columnHeader--pinnedRight',
'columnHeader--last',
'columnHeader--lastUnpinned',
'columnHeader--siblingFocused',
'columnHeaderCheckbox',
'columnHeaderDraggableContainer',
'columnHeaderTitle',
Expand Down
13 changes: 13 additions & 0 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 @@ -274,6 +274,17 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
computedWidth: colDef.computedWidth,
});

const siblingWithBorderingSeparator =
pinnedPosition === GridPinnedColumnPosition.RIGHT
? renderedColumns[i - 1]
: renderedColumns[i + 1];
const isSiblingFocused = siblingWithBorderingSeparator
? columnHeaderFocus !== null &&
columnHeaderFocus.field === siblingWithBorderingSeparator.field
: false;
const isLastUnpinned =
columnIndex + 1 === columnPositions.length - pinnedColumns.right.length;

Copy link
Member

Choose a reason for hiding this comment

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

Should the conditions above be different for LTR and RTL?

Copy link
Member Author

Choose a reason for hiding this comment

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

@cherniavskii Good point, though from testing this, it looks like we've got a bigger problem to solve with pinned columns + RTL 😬

Screenshot 2024-09-11 at 09 31 08

https://codesandbox.io/p/sandbox/rtl-pinned-columns-4k9sdh

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like we have an issue open for this already #14245

I'll take a look at that next.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cherniavskii tested this with the changes from #14586 and these conditions are working as expected as they are - I don't think we need to adjust these for RTL.

columns.push(
<GridColumnHeaderItem
key={colDef.field}
Expand All @@ -295,6 +306,8 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
indexInSection={i}
sectionLength={renderedColumns.length}
gridHasFiller={gridHasFiller}
isLastUnpinned={isLastUnpinned}
isSiblingFocused={isSiblingFocused}
{...other}
/>,
);
Expand Down