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

column type "detailPanelToggle" not supported #4769

Closed
2 tasks done
linojon opened this issue May 4, 2022 · 21 comments · Fixed by #8227
Closed
2 tasks done

column type "detailPanelToggle" not supported #4769

linojon opened this issue May 4, 2022 · 21 comments · Fixed by #8227
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!

Comments

@linojon
Copy link

linojon commented May 4, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Receiving warning message in console when adding a detail panel column to columns. Message attached in screen capture. Here's the column definition

    columns.push({
      ...GRID_DETAIL_PANEL_TOGGLE_COL_DEF,
      renderCell: (params) => (
        <DatagridCustomPanelToggle id={params.id} value={params.value} />
      )
    })

image

Expected behavior 🤔

No response

Steps to reproduce 🕹

Steps:

Context 🔦

"@mui/material": "^5.6.4",
"@mui/styles": "^5.6.2",
"@mui/x-data-grid-pro": "^5.10.0",

Your environment 🌎

`npx @mui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Order ID 💳 (optional)

41465

@linojon linojon added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 4, 2022
@cherniavskii
Copy link
Member

Hey @linojon
Can you provide a minimal example reproducing the issue?
I cannot reproduce it in this custom detail panel toggle demo: https://codesandbox.io/s/7rh5jo?file=/demo.tsx

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 4, 2022
@linojon
Copy link
Author

linojon commented May 4, 2022

I see it in your codesandbox. All I needed to do was click your link, and view the Console panel.

image

@cherniavskii
Copy link
Member

cherniavskii commented May 4, 2022

Ah right, I didn't notice it.
Same happens with checkbox selection column type: https://codesandbox.io/s/checkboxselectiongrid-material-demo-forked-ww7clr

import * as React from "react";
import { DataGrid, GRID_CHECKBOX_SELECTION_COL_DEF } from "@mui/x-data-grid";
import { useDemoData } from "@mui/x-data-grid-generator";

export default function CheckboxSelectionGrid() {
  const { data } = useDemoData({
    dataSet: "Commodity",
    rowLength: 10,
    maxColumns: 5
  });

  const columns = [
    {
      ...GRID_CHECKBOX_SELECTION_COL_DEF,
      width: 100
    },
    ...data.columns
  ];

  return (
    <div style={{ width: "100%", height: 400 }}>
      <DataGrid checkboxSelection {...data} columns={columns} />
    </div>
  );
}

The reason is that those columns are not included in gridDefaultColumnTypes:

export const getGridDefaultColumnTypes: () => GridColumnTypesRecord = () => {
const nativeColumnTypes = {
string: GRID_STRING_COL_DEF,
number: GRID_NUMERIC_COL_DEF,
date: GRID_DATE_COL_DEF,
dateTime: GRID_DATETIME_COL_DEF,
boolean: GRID_BOOLEAN_COL_DEF,
singleSelect: GRID_SINGLE_SELECT_COL_DEF,
[GRID_ACTIONS_COLUMN_TYPE]: GRID_ACTIONS_COL_DEF,
[DEFAULT_GRID_COL_TYPE_KEY]: GRID_STRING_COL_DEF,
};

And I'm not sure we can (or should) include detailPanelToggle there.
@m4theushw what do you think?

@cherniavskii cherniavskii added new feature New feature or request and removed status: waiting for author Issue with insufficient information labels May 4, 2022
@m4theushw
Copy link
Member

@cherniavskii I think we should include them in getGridDefaultColumnTypes. Don't forget the reorder type too: https://mui.com/x/react-data-grid/rows/#customizing-the-row-reordering-icon

@flaviendelangle
Copy link
Member

These column types are defined in pro and premium packages and getGridDefaultColumnTypes is called on community.
If we want to support those columns we could

  1. Deprecate getGridDefaultColumnTypes
  2. Add an apiRef.current.getBuiltInColumnTypes
  3. Add a pre-processor to let the plugins add there column types when calling getBuiltInColumnTypes

@cherniavskii
Copy link
Member

@flaviendelangle yeah, that's the only way we can support pro and premium column types there

@flaviendelangle
Copy link
Member

By the way, we have a slightly related topic with the default column type properties of pro / premium features.
They must be defined on the community package (groupable for instance).

@m4theushw
Copy link
Member

I was leaning into the least effort approach. Adding the pre-processor might be overkill to only make the warning work properly. There's a suggestion to remove the columnTypes prop which would make the warning useless: #242. For now we could do a dirty fix only to snooze the warning when a Pro column type is used. The column reordering docs advises to spread the GRID_REORDER_COL_DEF constant so the benefit of merging the column type properties is not used.

@flaviendelangle
Copy link
Member

There's a suggestion to remove the columnTypes prop which would make the warning useless

100% in favor of that one 😆

@cherniavskii
Copy link
Member

I'm adding columnTypes removal to v6 changes list then.

@linojon As a workaround, to avoid console warning you can use the following:

<DataGridPro
  columnTypes={{
    [GRID_DETAIL_PANEL_TOGGLE_COL_DEF.type]: GRID_DETAIL_PANEL_TOGGLE_COL_DEF
  }}
/>

Codesandbox: https://codesandbox.io/s/customizedetailpaneltoggle-material-demo-forked-vs15s3?file=/demo.tsx

@m4theushw
Copy link
Member

I'm reopening because in v6 we only plan to remove the columnTypes prop, we still have the warning to figure out what to do.

@m4theushw m4theushw reopened this Sep 9, 2022
@flaviendelangle
Copy link
Member

flaviendelangle commented Sep 9, 2022

I think we had an ongoing discussion about columnTypes and we did not reach any conclusion.
We had 2 use cases:

@Janpot
Copy link
Member

Janpot commented Sep 9, 2022

We looked into it for a while for Toolpad but in the end didn't use it for the reason that we had something that worked for us already, and it didn't look like it was going to stay a feature. It also didn't seem quite as powerful as we'd need it to be over time. So no blockers from our side for removing columnTypes.

@linojon

This comment was marked as duplicate.

@keemor
Copy link

keemor commented Oct 17, 2022

Hi,

When using

   columnTypes={{
          [GRID_DETAIL_PANEL_TOGGLE_COL_DEF.type]:
            GRID_DETAIL_PANEL_TOGGLE_COL_DEF,
        }}

I get:

image

So instead of //@ts-ignore on this line I added as string to make TS happy

 <DataGridPro
          columnTypes={{
            [GRID_DETAIL_PANEL_TOGGLE_COL_DEF.type as string]:
              GRID_DETAIL_PANEL_TOGGLE_COL_DEF,
          }}
....

Looks like type is string | undefined

image

Cheers,

@conerye
Copy link

conerye commented Oct 18, 2022

adding the columnTypes prop:
columnTypes={{ [GRID_CHECKBOX_SELECTION_COL_DEF.type]: GRID_CHECKBOX_SELECTION_COL_DEF }}
prevents the warning reported in #6519
Thanks!

@cherniavskii
Copy link
Member

We can remove the warning to reduce the confusion

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 19, 2023

There's a suggestion to remove the columnTypes prop which would make the warning useless

I think that we should stay attentive in cases where developers have typos in their type property, trying to use one of https://mui.com/x/react-data-grid/column-definition/#column-types

Screenshot 2023-07-19 at 20 42 34

If we assume that TypeScript is meant to give coverage, then indeed, the warning has the potential to be useless. However, TypeScript doesn't fail today:

Screenshot 2023-07-19 at 20 52 56

https://codesandbox.io/s/happy-wood-mp3mt6?file=/demo.tsx

I imagine because of the permissive type (string):

type LiteralUnion<LiteralType, BaseType> = LiteralType | (BaseType & Record<never, never>);
export type GridNativeColTypes =
| 'string'
| 'number'
| 'date'
| 'dateTime'
| 'boolean'
| 'singleSelect'
| 'actions';
// Use `LiteralUnion` to get autocompletion for literal types.
export type GridColType = LiteralUnion<GridNativeColTypes, string>;

Should we narrow it down?

@cherniavskii
Copy link
Member

I imagine because of the permissive type (string):

Yes, this is exactly the reason.

Should we narrow it down?

I think we can change this in v7. The challenge here is that we use custom column types internally for column identification, like here:

I imagine we could use field instead in the use case above, but custom column types could still be useful.

Maybe Instead of accepting any string we could use TS module augmentation?

export interface GridColTypeOverrides {}
export type GridColType = GridNativeColTypes | keyof GridColTypeOverrides;

declare module '@mui/x-data-grid' {
  export interface GridColTypeOverrides {
    customType: string;
  }
}

@cherniavskii
Copy link
Member

Extracted to #9735

@metruzanca
Copy link

Hey @cherniavskii, thanks for posting that example Codesandbox. That was just what I needed to get going!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants