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] Allow to pass props to the TrapFocus inside the panel wrapper #7733

Merged
merged 9 commits into from
Feb 10, 2023

Conversation

Vivek-Prajapatii
Copy link
Contributor

@Vivek-Prajapatii Vivek-Prajapatii commented Jan 27, 2023

Workaround for #7044

@mui-bot
Copy link

mui-bot commented Jan 27, 2023

Messages
📖 Netlify deploy preview: https://deploy-preview-7733--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 773 1,271.3 773 1,036.98 177.405
Sort 100k rows ms 910.8 1,351.6 1,259.9 1,162.42 151.834
Select 100k rows ms 250.6 367.7 277.1 292.1 41.205
Deselect 100k rows ms 178.9 408.1 241.1 270.58 77.257

Generated by 🚫 dangerJS against 408bb8b

@Vivek-Prajapatii Vivek-Prajapatii changed the title issues fixed filter panel in custom toolbar closes the date range picker Jan 27, 2023
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Jan 28, 2023
MUIStyledCommonProps<Theme> {}

const GridPanelWrapper = React.forwardRef<HTMLDivElement, GridPanelWrapperProps>(
function GridPanelWrapper(props, ref) {
const { className, ...other } = props;
const { className, TrapFocusProps: TrapFocusPropsProps, ...other } = props;
Copy link
Member

Choose a reason for hiding this comment

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

I wonder about the convention used in this context. Would it make more sense with?

Suggested change
const { className, TrapFocusProps: TrapFocusPropsProps, ...other } = props;
const { className, slotProps, ...other } = props;

It would become:

<DataGrid
  componentsProps={{ filterPanel: { slotProps: { trapFocus: { disableRestoreFocus: true } } } }}
/>

const rootProps = useGridRootProps();
const ownerState = { classes: rootProps.classes };
const classes = useUtilityClasses(ownerState);

return (
<TrapFocus open disableEnforceFocus isEnabled={isEnabled}>
<TrapFocus open disableEnforceFocus isEnabled={isEnabled} {...slotProps}>
Copy link
Member

Choose a reason for hiding this comment

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

To support the example usage from #7733 (comment), we need to spread slotProps.trapFocus instead.

Suggested change
<TrapFocus open disableEnforceFocus isEnabled={isEnabled} {...slotProps}>
<TrapFocus open disableEnforceFocus isEnabled={isEnabled} {...slotProps.trapFocus}>

You also need to do slotProps = {} when extracting the prop from props above.

@m4theushw m4theushw changed the title filter panel in custom toolbar closes the date range picker [DataGrid] Add to pass props to TrapFocus inside panel wrapper Jan 31, 2023
@m4theushw m4theushw changed the title [DataGrid] Add to pass props to TrapFocus inside panel wrapper [DataGrid] Add to pass props to the TrapFocus` inside panel wrapper Jan 31, 2023
@m4theushw m4theushw changed the title [DataGrid] Add to pass props to the TrapFocus` inside panel wrapper [DataGrid] Allow to pass props to the TrapFocus` inside panel wrapper Jan 31, 2023
@m4theushw m4theushw changed the title [DataGrid] Allow to pass props to the TrapFocus` inside panel wrapper [DataGrid] Allow to pass props to the TrapFocus inside the panel wrapper Jan 31, 2023
@m4theushw m4theushw self-requested a review February 9, 2023 22:18
@m4theushw m4theushw added the needs cherry-pick The PR should be cherry-picked to master after merge label Feb 10, 2023
Copy link
Member

@m4theushw m4theushw left a comment

Choose a reason for hiding this comment

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

I pushed 408bb8b fixing the remaining errors. Thanks!

@m4theushw m4theushw merged commit 606826c into mui:next Feb 10, 2023
m4theushw added a commit to m4theushw/mui-x that referenced this pull request Feb 10, 2023
…apper (mui#7733)

Co-authored-by: Matheus Wichman <matheushw@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants