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] Make GridPreferencesPanel private #1855

Closed
1 task done
Tracked by #7902
sebastianfrey opened this issue Jun 8, 2021 · 13 comments · Fixed by #11228
Closed
1 task done
Tracked by #7902

[DataGrid] Make GridPreferencesPanel private #1855

sebastianfrey opened this issue Jun 8, 2021 · 13 comments · Fixed by #11228
Assignees
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! deprecation New deprecation message feature: Filtering Related to the data grid Filtering feature v7.x

Comments

@sebastianfrey
Copy link
Contributor

sebastianfrey commented Jun 8, 2021

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

It seems, that the current GridPreferencesPanel implementation hard coded the rendering of the ColumnSelectorComponent and the FilterPanelComponent. This makes it impossible for a developer to leverage the built-in GridPreferencesPanel to render additional actions along side the default grid preferences (ColumnSelectorComponent and FilterPanelComponent) without providing a custom GridPreferencesPanel implementation. Therefore it would be nice when a developer would be able to inject custom components for rendering in the default GridPreferncesPanel.

Examples 🌈

When the GridApi#showPreferences() method would accept a component instead of the current string constant, this feature would be fairly easy to support.

function CustomPreferencesComponent() {
   // The custom component to show in the grids built-in preferences panel
   return ...;
}

function CustomToolbarButton() {
  const apiRef = React.useContext(GridApiContext);
  const { open, >openedPanelComponent< } = useGridSelector(apiRef, gridPreferencePanelStateSelector);
  
  const togglePreferences = () => {
    if (open && openedPanelComponent === CustomPreferencesComponent) {
      apiRef!.current.hidePreferences();
    } else {
      apiRef!.current.showPreferences(CustomPreferencesComponent); 
    }
  };
    
  return <Button ref={ref} onClick={togglePreferences} />
}

Motivation 🔦

Allow developers to leverage the built-in GridPreferencesPanel implementation to inject custom components.

Order id 💳

22075

@sebastianfrey sebastianfrey added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 8, 2021
@sebastianfrey
Copy link
Contributor Author

FYI: I was able to use the built in GridPreferncesPanel by doing the following:

  1. Augment the type definitions of GridPreferencesPanelValue
import * as xgrid from '@material-ui/data-grid';

declare module '@material-ui/data-grid' {
  declare enum GridPreferencePanelsValue {
    customPrefrences = 'customPrefrences',
  }
}
  1. Define the content to show in the PreferencesPanel as CustomPreferencesComponent:
import * as React from 'react';

export function CustomPreferencesComponent() {

   return ...;
}
  1. Define a custom PreferencesPanel component which contains code copy & pasted from the built-in PreferencesPanel implementation.
import * as React from 'react';
import {
  allGridColumnsSelector,
  GridApiContext,
  gridPreferencePanelStateSelector,
  GridPreferencePanelsValue,
  GridPreferencesPanel,
  useGridSelector,
} from '@material-ui/data-grid';
import CustomPreferencesComponent from './CustomPreferencesComponent';

export const CustomPreferencesPanel = React.forwardRef<
  HTMLDivElement,
  React.HTMLAttributes<HTMLDivElement>
>(function CustomPreferencesPanel(props, ref) {
  const apiRef = React.useContext(GridApiContext);
  const columns = useGridSelector(apiRef, allGridColumnsSelector);
  const preferencePanelState = useGridSelector(apiRef, gridPreferencePanelStateSelector);

  const isCustomPreferencesPanelOpen =
    preferencePanelState.openedPanelValue === GridPreferencePanelsValue.customPreferences;

  const Panel = apiRef!.current.components.Panel!;

  return isCustomPreferencesPanelOpen ? (
    <Panel
      ref={ref}
      open={columns.length > 0 && preferencePanelState.open}
      {...apiRef?.current.componentsProps?.panel}
      {...props}
    >
      {isCustomPreferencesPanelOpen && (
        <CustomPreferencesComponent
          {...apiRef?.current.componentsProps?.preferencesPanel?.customComponent}
        />
      )}
    </Panel>
  ) : (
    <GridPreferencesPanel ref={ref} {...props} />
  );
});
  1. Define a custom Toolbar Button, which toggles the CustomPreferencesComponent.
import * as React from 'react';
import { Button } from '@material-ui/core';
import {
  GridApiContext,
  GridPreferencePanelsValue,
  gridPreferencePanelStateSelector,
  useGridSelector,
} from '@material-ui/data-grid';

export const CustomPreferencesButton = React.forwardRef<
  HTMLButtonElement,
  React.HTMLAttributes<HTMLButtonElement>
>(function CustomPreferencesButton(props, ref) {
  const apiRef = React.useContext(GridApiContext);
  const { open, openedPanelValue } = useGridSelector(apiRef, gridPreferencePanelStateSelector);

  const togglePreferences = () => {
    if (open && openedPanelValue === GridPreferencePanelsValue.customPreferences) {
      apiRef!.current.hidePreferences();
    } else {
      apiRef!.current.showPreferences(GridPreferencePanelsValue.customPreferences);
    }
  };

  return (
    <Button
      variant="text"
      ref={ref}
      color="primary"
      size="small"
      onClick={togglePreferences}
    >
      Custom Preferences
    </Button>
  );
});
  1. Define a custom Toolbar component:
import * as React from 'react';
import {
  GridToolbarColumnsButton,
  GridToolbarFilterButton,
  GridToolbarContainer,
  GridToolbarDensitySelector,
  GridToolbarExport,
} from '@material-ui/data-grid';

import CustomPreferencesComponent from './CustomPreferencesComponent';

export function CustomToolbar() {
  return (
    <GridToolbarContainer>
      <GridToolbarColumnsButton />
      <GridToolbarFilterButton />
      <GridToolbarDensitySelector />
      <GridToolbarExport />
      <CustomPreferencesComponent />
    </GridToolbarContainer>
  );
}
  1. Glue all together
<DataGrid
    ...
    components={{
      Toolbar: CustomToolbar,
      PreferencesPanel: CustomPreferencesPanel,
    }}
/>

Although the above shown solution works, I think it smells a bit, since I would have to rely on some DataGrid internals, which may change in the future, which likley will break my CustomPreferencesPanel.

How do you think about the proposed change of the GridApi#showPreferences() method? If desired, I am willing to provide a PR for this, if you think this enhancment might be useful.

@dtassone
Copy link
Member

dtassone commented Jun 9, 2021

Thanks for raising this issue, I have also noticed issues with the current implementation of the preferencePanel. I think we should get rid of it as a component. We should keep a panel state to control which panel are currently displayed but we should dissociate the filter panel and column selector of the preference panel, to be able to control how to render those separately. Also it would allow to render more component and panels.

Demo on how to render the column selector outside the grid.
As you can see, it creates issues with the filter panel as I want to keep it within the default panel hence why we should remove the preference panel and manage them independently.
https://material-ui-x.netlify.app/storybook/?path=/story/x-grid-demos-custom-components--outside-columns-panel

@dtassone dtassone added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 9, 2021
@sebastianfrey
Copy link
Contributor Author

@dtassone Ok. So what you suggest would also solve the problem raised in #1808, since I would be able to have more control about how the preferences are renderd, e.g. define a custom maxHeight on the dialog meeting my applications constraints.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 10, 2021

@sebastianfrey Could you describe the concrete problem you are trying to solve?

to render additional actions along side the default grid preferences

Do you have a visual illustration? What would be wrong with rendering your own pop-up or us allowing to extend the column selector panel and the filter selector panel specifically, making PreferencesPanel private/removing it. It seems that in #1855 (comment) you actually don't want to even know of the existant of the GridPreferencePanel but render a Panel directly.

So far, the description of the GitHub issue seems to focus too much on "what" we should do, but not enough on "why". I mean, I think that we should focus first on what is the problem, second on what are the options we have to solve the problem, and last why a specific one is superior to the others solutions/options.

@sebastianfrey
Copy link
Contributor Author

@oliviertassinari I am building a custom toolbar button, which opens a panel where users can kick off backend workflows for further data processing. I thought it would be nice to have the ability to use the built-in PreferencesPanel, so I do not have to care that the positioning of my custom panel matches exactly the positioning of the built-in panel in order to give my users a fluid UX. I hope this clarifies the why a bit.

The suggestion of @dtassone to have the ability as a developer to control how and especially where the filter panel or the column panel should be rendered, would make the requirement to use the built-in PreferencesPanel null and void.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 10, 2021

@sebastianfrey OK, thanks for the extra details. I think that it makes sense to make it easy to display custom panels. For the next step on this issue. I would propose we

  1. make the GridPreferencesPanel & slot private. It doesn't seem to really make sense to have developers customizing it.
  2. look at how we can make it easy to create a similar panel, maybe using GridPanel or maybe using the state.preferencePanel.

The current position of the grid preference is KO (IMHO). I think that we should rework it to be as close as possible to the button that triggers it (either close to the column menu or the toolbar button). The fewer end-users have to move the cursor, the better the UX.

@oliviertassinari oliviertassinari changed the title [DataGrid] Allow to leverage the built-in Preferences Panel for custom components [DataGrid] Make GridPreferencesPanel private Jul 17, 2021
@oliviertassinari oliviertassinari added breaking change feature: Filtering Related to the data grid Filtering feature labels Jan 29, 2022
@zivgit
Copy link

zivgit commented Jun 8, 2022

At the end of the discussion, there still seems to be no apparent reason to make the GridPreferencesPanel private.

@joserodolfofreitas joserodolfofreitas removed the new feature New feature or request label Jun 24, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 13, 2023

At the end of the discussion, there still seems to be no apparent reason to make the GridPreferencesPanel private.

@zivgit I think that the reason is that there is nothing to customize with the PreferencesPanel slot.

This is the source:

<rootProps.components.PreferencesPanel {...rootProps.componentsProps?.preferencesPanel} />

I see no use cases for using:

PreferencesPanel: GridPreferencesPanel,

that can't be done by using slots.Panel.

@oliviertassinari oliviertassinari added the deprecation New deprecation message label Jan 13, 2023
@cherniavskii cherniavskii self-assigned this Oct 17, 2023
@cherniavskii
Copy link
Member

While making GridPreferencesPanel private makes sense to me, the problem of adding a custom panel still needs a solution.

In addition to the panel slot, we can also add a panelContent slot:

export function GridDefaultPanelContent({
  panelValue,
}: {
  panelValue?: GridPreferencePanelsValue;
}) {
  const rootProps = useGridRootProps();
  if (panelValue === GridPreferencePanelsValue.columns) {
    return <rootProps.slots.columnsPanel {...rootProps.slotProps?.columnsPanel} />;
  }
  if (panelValue === GridPreferencePanelsValue.filters) {
    return <rootProps.slots.filterPanel {...rootProps.slotProps?.filterPanel} />;
  }
  return null;
}

Note that this means removing the preferencePanel preprocessing.

Then, users will be able to override the slot, like this:

function CustomPanelContent({ panelValue }) {
  if (panelValue === 'custom') {
    return <div>Custom panel content</div>;
  }
  return <GridDefaultPanelContent panelValue={panelValue} />;
}

<DataGrid slots={{ panelContent: CustomPanelContent }} />

Finally, opening the custom panel content can be done using apiRef:

<Button onClick={() => apiRef.current.showPreferences('custom')}>
  Open custom panel
</Button>

This will work in JS runtime, for TS we will need to allow extending GridPreferencePanelsValue.
From what I can tell, it's not possible to use module augmentation to extend the enum.
Not sure how to solve this, this is the missing piece in my proposal.

@cherniavskii
Copy link
Member

cherniavskii commented Oct 24, 2023

@cherniavskii to create a separate issue with a proposal for adding custom panels. We estimated the complexity as 3.

@sebastianfrey
Copy link
Contributor Author

@cherniavskii Regarding your concerns about extending enum with module augmentation. I have already done this before and it worked as expected.

When someone wants to extend GridPreferencesPanelValue with a prop custom, an additional type declaration named mui-x-data-grid-prop.d.ts with the following content must be created:

export * from '@mui/x-data-grid-pro';

declare module '@mui/x-data-grid-pro' {
  export enum GridPreferencePanelsValue {
    custom = 'custom',
  }
}

The declaration file than must be referenced in tsconfig.json.

@sebastianfrey
Copy link
Contributor Author

@cherniavskii Unfortunately I have to correct my statement here:

I did check again the source code of the project where I used the above approach. It seems that the approach worked well with previous version of typescript, with an up to date typescript version, you will get proper typing, but at runtime the enum is still the original one. Which means someone would have to monkey patch the GridPreferencesPanelValue enum to make it work:

import { GridPreferencesPanelValue } from '@mui/x-data-grid-pro';

(GridPreferencesPanelValue as any).custom = 'custom';

Also accessing the custom enum type is a bit annoying. As I have said, you will get proper typing, but when you try to compare enum values like the following

const { openedPanelValue } = useGridSelector(apiRef, gridPreferencePanelStateSelector);

const isCustom = openedPanelValue === GridPreferencesPanelValue.custom;

typescript starts complaining

This comparison appears to be unintentional because the types 'GridPreferencePanelsValue' and 'GridPreferencePanelsValue.workflows' have no overlap.ts(2367)

To workaround you would have to use again an ugly any cast:

const { openedPanelValue } = useGridSelector(apiRef, gridPreferencePanelStateSelector);

const isCustom = openedPanelValue === (GridPreferencesPanelValue as any).custom;

@cherniavskii
Copy link
Member

@sebastianfrey thanks for your input!
It looks like enum is not the best choice in this case, I'll check if we can get a better DX with something else instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! deprecation New deprecation message feature: Filtering Related to the data grid Filtering feature v7.x
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants