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.

Sign up for GitHub

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

Jump to bottom

[data grid] GridFilterInputMultipleValue's prop types are incorrectly typed #13409

Open
karudedios opened this issue Jun 7, 2024 · 4 comments · May be fixed by #13411
Open

[data grid] GridFilterInputMultipleValue's prop types are incorrectly typed #13409

karudedios opened this issue Jun 7, 2024 · 4 comments · May be fixed by #13411
Labels
component: data grid This is the name of the generic UI component, not the React module! customization: extend Logic customizability feature: Filtering Related to the data grid Filtering feature good first issue Great for first contributions. Enable to learn the contribution process. typescript

Comments

@karudedios
Copy link

karudedios commented Jun 7, 2024 •
edited
Loading

Steps to reproduce

Link to live example: (required)
https://codesandbox.io/p/sandbox/dark-fire-x498pg?file=%2Fsrc%2FDemo.tsx

Steps:

  1. Open the Filters Panel
  2. Select Created On to filter by
  3. Observe console error about date not being supported while input works perfectly with a type = "date"

Current behavior

GridFilterInputMultipleValue's prop types validation produces noisy error logs when a type that isn't "text" | "number" is supported for the underlying input, while in truth any valid HTML input type is supported out of the box.

Expected behavior

Just like GridFilterInputValue does, GridFilterInputMultipleValue should expose all supported type values, not just "text "| "number"

Context

I'm trying not to have noisy errors (oftentimes reported to 3rd party telemetry) for using a component's feature that is actually supported and works out of the box just because its prop types definition is unaware of it.

Your environment

npx @mui/envinfo 
  System:
    OS: Linux 5.15 Ubuntu 20.04.5 LTS (Focal Fossa)
  Binaries:
    Node: 18.16.1 - ~/.nvm/versions/node/v18.16.1/bin/node
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.1/bin/npm
    pnpm: 9.2.0 - ~/.nvm/versions/node/v18.16.1/bin/pnpm
  Browsers:
    Chrome: 125.0.6422.142
  npmPackages:
    @emotion/react: ^11.7.1 => 11.11.1
    @emotion/styled: ^11.6.0 => 11.11.0
    @mui/icons-material: ^5.3.1 => 5.14.3
    @mui/lab: ^5.0.0-alpha.102 => 5.0.0-alpha.140
    @mui/material: ^5.3.1 => 5.14.5
    @mui/styles: ^5.3.0 => 5.12.3
    @mui/utils: ^5.11.2 => 5.14.16
    @mui/x-data-grid-pro: ^5.17.4 => 6.2.1
    @mui/x-date-pickers-pro: ^6.2.1 => 6.2.1
    @mui/x-license-pro: ^5.17.0 => 6.0.4
    @types/react: ^18.0.21 => 18.2.20
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript: ~5.2.2 => 5.2.2

Search keywords: GridFilterInputMultipleValue, GridFilterInputValue, unsupported, type, date
@karudedios karudedios added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 7, 2024
@karudedios karudedios linked a pull request Jun 7, 2024 that will close this issue
Open
1 task
@michelengelen michelengelen changed the title GridFilterInputMultipleValue's prop types are incorrectly typed [data grid] GridFilterInputMultipleValue's prop types are incorrectly typed Jun 7, 2024
@michelengelen michelengelen added component: data grid This is the name of the generic UI component, not the React module! typescript feature: Filtering Related to the data grid Filtering feature customization: extend Logic customizability labels Jun 7, 2024
@michelengelen
Copy link
Member

Hey @karudedios it seems the example you linked is incomplete.
The Created on column is a date and therefore cannot have a multiple value input.
At least not in the implementation you linked.

Did you mean to provide a custom filtering mechanism for dates, to select multiple values?

@michelengelen michelengelen added 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 Jun 7, 2024
@karudedios
Copy link
Author

karudedios commented Jun 7, 2024 •
edited
Loading

Hi @michelengelen, thanks for noting it- I had copied the link from original sandbox pointed at one of MUI's demos instead of the version I forked/modified.

The correct Code Sandbox is https://codesandbox.io/p/sandbox/dark-fire-x498pg, which I've now updated in the description as well.

Did you mean to provide a custom filtering mechanism for dates, to select multiple values?

Yes, I added a custom isAnyOf filter as the sole filter operator for any type: 'date' column for easy demoing

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Jun 7, 2024
@michelengelen
Copy link
Member

Thanks for that.
Indeed I would say as well that we could allow for 'date' and 'datetime-local' to be accepted as well on the input.

diff --git a/packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputMultipleValue.tsx b/packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputMultipleValue.tsx
index 0d39447e6..a7f3e805e 100644
--- a/packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputMultipleValue.tsx
+++ b/packages/x-data-grid/src/components/panel/filterPanel/GridFilterInputMultipleValue.tsx
@@ -6,7 +6,7 @@ import { useGridRootProps } from '../../../hooks/utils/useGridRootProps';
 import { GridFilterInputValueProps } from './GridFilterInputValueProps';

 export type GridFilterInputMultipleValueProps = {
-  type?: 'text' | 'number';
+  type?: 'text' | 'number' | 'date' | 'datetime-local';
 } & GridFilterInputValueProps &
   Omit<AutocompleteProps<string, true, false, true>, 'options' | 'renderInput'>;

@@ -113,7 +113,6 @@ GridFilterInputMultipleValue.propTypes = {
     operator: PropTypes.string.isRequired,
     value: PropTypes.any,
   }).isRequired,
-  type: PropTypes.oneOf(['number', 'text']),
 } as any;

 export { GridFilterInputMultipleValue };

objections @mui/xgrid ?

@michelengelen michelengelen added good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 10, 2024
@karudedios
Copy link
Author

Hi @michelengelen, just wanted to ping on this and check if the PR I submitted for this looks OK and/or if it could get a reviewer assigned, it's linked to this issue but I'm also happy to re-post it for easier visibility #13411.

Thanks for your time, and no rush- it's a pretty small issue, albeit a little noisy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: data grid This is the name of the generic UI component, not the React module! customization: extend Logic customizability feature: Filtering Related to the data grid Filtering feature good first issue Great for first contributions. Enable to learn the contribution process. typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[core] update type and propType enum for GridFilterInputMultipleValue karudedios/mui-x
2 participants
@karudedios @michelengelen