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.

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] Refactor: create base Popper #16362

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/data/data-grid/filtering/CustomFilterPanelPosition.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function CustomFilterPanelPosition() {
slots={{ toolbar: CustomToolbar }}
slotProps={{
panel: {
anchorEl: filterButtonEl,
target: filterButtonEl,
},
toolbar: { setFilterButtonEl },
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default function CustomFilterPanelPosition() {
slots={{ toolbar: CustomToolbar }}
slotProps={{
panel: {
anchorEl: filterButtonEl,
target: filterButtonEl,
},
toolbar: { setFilterButtonEl },
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
slots={{ toolbar: CustomToolbar }}
slotProps={{
panel: {
anchorEl: filterButtonEl,
target: filterButtonEl,
},
toolbar: { setFilterButtonEl },
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function GridHeaderFilterMenu({
}

return (
<GridMenu placement="bottom-end" open={open} target={target} onClose={hideMenu}>
<GridMenu position="bottom-end" open={open} target={target} onClose={hideMenu}>
<rootProps.slots.baseMenuList
aria-labelledby={labelledBy}
id={id}
Expand Down
51 changes: 12 additions & 39 deletions packages/x-data-grid/src/components/menu/GridMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import ClickAwayListener, { ClickAwayListenerProps } from '@mui/material/ClickAwayListener';
import {
unstable_composeClasses as composeClasses,
unstable_useEnhancedEffect as useEnhancedEffect,
HTMLElementType,
} from '@mui/utils';
import Grow, { GrowProps } from '@mui/material/Grow';
import Paper from '@mui/material/Paper';
import Popper, { PopperProps } from '@mui/material/Popper';
import { styled } from '@mui/material/styles';
import { PopperProps } from '../../models/gridBaseSlots';
import { GridSlotProps } from '../../models/gridSlotsComponent';
import { getDataGridUtilityClass, gridClasses } from '../../constants/gridClasses';
import type { DataGridProcessedProps } from '../../models/props/DataGridProps';
import { useGridRootProps } from '../../hooks/utils/useGridRootProps';
import { useGridApiContext } from '../../hooks/utils/useGridApiContext';
import { NotRendered } from '../../utils/assert';

type MenuPosition =
| 'bottom-end'
Expand Down Expand Up @@ -43,31 +42,24 @@ const useUtilityClasses = (ownerState: OwnerState) => {
return composeClasses(slots, getDataGridUtilityClass, classes);
};

const GridMenuRoot = styled(Popper, {
const GridMenuRoot = styled(NotRendered<GridSlotProps['basePopper']>, {
Copy link
Member

@oliviertassinari oliviertassinari Jan 31, 2025

Choose a reason for hiding this comment

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

I find this diff strange. What if we were to import https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Popper/BasePopper.tsx and style this one instead.

But even then, I'm unclear about the need. Maybe what confuses me is in the order in which we do this. Shouldn't we first move GridMenuRoot out of the unstyled version? Once we do, we can stick to import and wrap Popper. The fewer React component wrappers, the fastest the grid is.

Unless maybe the whole GridMenu component is a styled API, so maybe move the whole thing out of the unstyled data grid? If I'm using Shadcn UI, would I want to replace it, or add sub slots? I think it's how we determine if it's for the unstyled API or Material UI API.

Copy link
Contributor Author

@romgrk romgrk Jan 31, 2025

Choose a reason for hiding this comment

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

What we need to do here is wrap rootProps.slots.basePopper, but it's not available at initialization. I'm using this NotRendered component just to make the typechecking work. At runtime, as={slots.basePopper} needs to be used.

As for the styled/unstyled changes, I'm still evaluating how to do them. Ideally, we'll have a styling solution that doesn't require tangling styles to components at initialization time.

name: 'MuiDataGrid',
slot: 'Menu',
overridesResolver: (_, styles) => styles.menu,
})<{ ownerState: OwnerState }>(({ theme }) => ({
zIndex: theme.zIndex.modal,
[`& .${gridClasses.menuList}`]: {
outline: 0,
},
}));

export interface GridMenuProps extends Omit<PopperProps, 'onKeyDown' | 'children'> {
export interface GridMenuProps extends Pick<PopperProps, 'className' | 'onExited'> {
open: boolean;
target: HTMLElement | null;
onClose: (event?: Event) => void;
position?: MenuPosition;
onExited?: GrowProps['onExited'];
children: React.ReactNode;
}

const transformOrigin = {
'bottom-start': 'top left',
'bottom-end': 'top right',
};

function GridMenu(props: GridMenuProps) {
const { open, target, onClose, children, position, className, onExited, ...other } = props;
const apiRef = useGridApiContext();
Expand All @@ -91,17 +83,7 @@ function GridMenu(props: GridMenuProps) {
apiRef.current.publishEvent(eventName, { target });
}, [apiRef, open, target]);

const handleExited = (popperOnExited: (() => void) | undefined) => (node: HTMLElement) => {
if (popperOnExited) {
popperOnExited();
}

if (onExited) {
onExited(node);
}
};

const handleClickAway: ClickAwayListenerProps['onClickAway'] = (event) => {
const handleClickAway = (event: MouseEvent | TouchEvent) => {
if (event.target && (target === event.target || target?.contains(event.target as Node))) {
return;
}
Expand All @@ -114,23 +96,16 @@ function GridMenu(props: GridMenuProps) {
className={clsx(classes.root, className)}
ownerState={rootProps}
open={open}
anchorEl={target as any}
target={target}
transition
placement={position}
onClickAway={handleClickAway}
onExited={onExited}
clickAwayMouseEvent="onMouseDown"
{...other}
{...rootProps.slotProps?.basePopper}
>
{({ TransitionProps, placement }) => (
<ClickAwayListener onClickAway={handleClickAway} mouseEvent="onMouseDown">
<Grow
{...TransitionProps}
style={{ transformOrigin: transformOrigin[placement as keyof typeof transformOrigin] }}
onExited={handleExited(TransitionProps?.onExited)}
>
<Paper>{children}</Paper>
</Grow>
</ClickAwayListener>
)}
{children}
</GridMenuRoot>
);
}
Expand All @@ -141,11 +116,9 @@ GridMenu.propTypes = {
// | To update them edit the TypeScript types and run "pnpm proptypes" |
// ----------------------------------------------------------------------
children: PropTypes.node,
className: PropTypes.string,
onClose: PropTypes.func.isRequired,
onExited: PropTypes.func,
/**
* If `true`, the component is shown.
*/
open: PropTypes.bool.isRequired,
position: PropTypes.oneOf([
'bottom-end',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function GridColumnHeaderMenu({

return (
<GridMenu
placement={`bottom-${colDef!.align === 'right' ? 'start' : 'end'}` as any}
position={`bottom-${colDef!.align === 'right' ? 'start' : 'end'}` as any}
open={open}
target={target}
onClose={hideMenu}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import { GridPanelWrapper, GridPanelWrapperProps } from './GridPanelWrapper';
import { useGridRootProps } from '../../hooks/utils/useGridRootProps';

Expand All @@ -14,12 +13,4 @@ function GridColumnsPanel(props: GridColumnsPanelProps) {
);
}

GridColumnsPanel.propTypes = {
// ----------------------------- Warning --------------------------------
// | These PropTypes are generated from the TypeScript type definitions |
// | To update them edit the TypeScript types and run "pnpm proptypes" |
// ----------------------------------------------------------------------
slotProps: PropTypes.object,
} as any;

export { GridColumnsPanel };
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { GridPanel } from '@mui/x-data-grid';
function MyPanel() {
return (
<div>
<GridPanel classes={{ paper: 'paper' }} open modifiers={[{ name: 'flip', enabled: false }]} />
<GridPanel classes={{ paper: 'paper' }} open flip={false} />
{/* @ts-expect-error foo classes doesn't exist */}
<GridPanel classes={{ foo: 'foo' }} />
</div>
Expand Down
42 changes: 0 additions & 42 deletions packages/x-data-grid/src/components/panel/GridPanel.test.tsx

This file was deleted.

Loading