[DataGrid] Export option can be disable

[alexfauquette]

Fix #3064

[m4theushw]

Maybe it's too premature, but this attribute could be extract into an interface so every export option must inherit it and support disable.

[alexfauquette]

Done in this commit: 9cbc962

I took the common options:

• fields
• allColumns
• disable

The only problem is for documentation comment, in which I have to replace "Print export" and "CSV export" by "this export"

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[flaviendelangle]

What should be the behavior if the user calls exportDataAsCsv with disable: true ? (same for print)
Should be raise an error ? Ignore it as you currently do ? Skip the export ?

The most coherent behavior would probably be to split the interfaces between the imperative method and the React component, to only have disable on the React component.

This would be an argument in favor of csvOptions={false} instead of csvOptions={{ disable: false }}

[alexfauquette]

Yes, if GridCsvExportOptions is used by imperative methods, it does not make sens to put user interface customisation in it.

disable could be defined in type GridExportOption. But it does not make sens to add an option disable that will disable all the other options.

Does someone else has an argument in favour of the disable logic?

[m4theushw]

I'm not very in favor of csvOptions={false}, since the prop was meant to be an object initially. We have been using slots to allow simple customizations without forcing to override an entire component. For instance, the options of the column menu can be easily changed doing that: https://mui.com/components/data-grid/components/#column-menu. The same approach could be done here maybe. It would even allow users to add their export options.

[alexfauquette]

So you propose to export a <GridCsvExportItem /> and <GridPrintExportItem /> such that developers will define the Toolbar as follow:

<GridToolbarContainer>
    <GridToolbarColumnsButton />
    <GridToolbarFilterButton />
    <GridToolbarDensitySelector />
    <GridToolbarExportContainer>
		<GridCsvExportItem options={...} />
		<GridPrintExportItem options={...} />
		<MyCustomExportItem options={...} />
    </GridToolbarExportContainer>
</GridToolbarContainer>

Why not have both? Having a GridToolbarExportContainer container is good if dev wants to do advanced customization, but if she only wants to remove a button, setting its props to false seems natural.

I feel it like a pain when in react-admin I have to redefine a component to remove an option.

[m4theushw]

My idea is not exactly as you proposed. We add a new toolbarExportButton slot which has by default:

<GridToolbarExportContainer>
  <GridCsvExportItem options={...} />
  <GridPrintExportItem options={...} />
  <MyCustomExportItem options={...} />
</GridToolbarExportContainer>

If the user wants to disable one of the items, he removes the respective option. My argument against passing false is that the options prop is meant to be object. We're changing its meaning depending on its value. If the slot approach is too overkill, we could also add an allowedFormats prop to the GridToolbarExportContainer.

[alexfauquette]

In fact, the problem is the naming disable, because we do not want to disable, the export. We want to remove its button. So we could simply rename the field display and add it only in the <GridToolbarExport/> props type

[m4theushw]

Or disableToolbarButton

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[m4theushw]

I left some minor suggestions. Otherwise, looks good.

[m4theushw]

Prefer interfaces always.

Suggested change

	type GridExportDisplayOptions = {
	interface GridExportDisplayOptions {

Same for GridExportOption.

[m4theushw]

It fits in one line.

Suggested change

	formatOptions?:
	| (GridCsvExportOptions & GridExportDisplayOptions)
	| (GridPrintExportOptions & GridExportDisplayOptions);
	formatOptions?: (GridCsvExportOptions | GridPrintExportOptions) & GridExportDisplayOptions;

[m4theushw]

Suggested change

	### Remove export button
	### Remove the export button

[m4theushw]

Suggested change

	### Remove print button
	### Remove the print button

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.