mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.11k stars 1.27k forks source link

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

Closed karudedios closed 2 months ago

karudedios commented 3 months ago

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

michelengelen commented 3 months ago

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?

karudedios commented 3 months ago

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

michelengelen commented 3 months ago

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 ?

karudedios commented 3 months ago

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 https://github.com/mui/mui-x/pull/13411.

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

github-actions[bot] commented 2 months ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@karudedios: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.