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

[data grid] Missing translation for key headerFilterOperatorBetween #13217

Closed
ankurheble opened this issue May 23, 2024 · 7 comments · Fixed by #13255
Closed

[data grid] Missing translation for key headerFilterOperatorBetween #13217

ankurheble opened this issue May 23, 2024 · 7 comments · Fixed by #13255
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Filtering on header Related to the header filtering (Pro) feature

Comments

@ankurheble
Copy link

ankurheble commented May 23, 2024

Steps to reproduce

Link to live example: (required)
https://codesandbox.io/p/sandbox/nostalgic-pascal-9twstk?file=%2Fsrc%2FDemo.tsx%3A141%2C22
Steps:

  1. Try to implement Multiple values filter as mentioned in the docs here https://mui.com/x/react-data-grid/filtering/customization/ by clicking on the codesandbox example
  2. Inside codesandbox install @mui/x-data-grid-pro latest version
  3. Enable Header Filters for the DataGridPro which has already got the custom quantity filter mentioned in above example.

Missing translation for key headerFilterOperatorBetween. is shown
image

Current behavior

Throws an error : Missing translation for key headerFilterOperatorBetween.
image

Expected behavior

Should use the same translation added for the custom operator.

Context

Trying to implement a multiple values operator for column type number in DataGridPro with headerFilters enabled.

Your environment

npx @mui/envinfo System: OS: Windows 11 10.0.22631 Binaries: Node: 20.11.1 - C:\Program Files\nodejs\node.EXE npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD pnpm: Not Found Browsers: Brave Browser: Chromium (125.0.6422.60) Edge: Chromium (123.0.2420.97) npmPackages: @emotion/react: ^11.11.4 => 11.11.4 @emotion/styled: ^11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.40 @mui/core-downloads-tracker: 5.15.18 @mui/icons-material: ^5.15.13 => 5.15.13 @mui/material: ^5.15.12 => 5.15.18 @mui/private-theming: 5.15.14 @mui/styled-engine: 5.15.14 @mui/styled-engine-sc: ^6.0.0-alpha.17 => 6.0.0-alpha.17 @mui/system: 5.15.15 @mui/types: 7.2.14 @mui/utils: 5.15.14 @mui/x-data-grid: ^7.5.0 => 7.5.0 @mui/x-data-grid-pro: ^7.5.0 => 7.5.0 @mui/x-license: ^7.2.0 => 7.2.0 @types/react: ^18 => 18.2.62 react: ^18 => 18.2.0 react-dom: ^18 => 18.2.0 styled-components: ^6.1.8 => 6.1.8 typescript: ^5 => 5.3.3

Search keywords: data grid pro, header filters, translation error
Order ID: 90421

@ankurheble ankurheble added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 23, 2024
@MBilalShafi MBilalShafi added component: data grid This is the name of the generic UI component, not the React module! feature: Filtering on header Related to the header filtering (Pro) feature labels May 23, 2024
@MBilalShafi
Copy link
Member

@ankurheble Just like you provide label for regular filter in a custom GridFilterOperator, you can add a headerLabel for header filters.

Something like:

const quantityOnlyOperators: GridFilterOperator<any, number>[] = [
  {
    label: "Between",
    headerLabel: "Between", // newly added property
    ...otherProperties,
  },
];

I've tried to update it in your demo, here's the forked version: https://codesandbox.io/p/sandbox/async-snow-nqcjz3

Let me know if it solves your issue.

Thanks

@MBilalShafi MBilalShafi 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 May 23, 2024
@michelengelen michelengelen changed the title DataGridPro: Missing translation for key headerFilterOperatorBetween. [data grid] Missing translation for key headerFilterOperatorBetween May 23, 2024
@flaviendelangle
Copy link
Member

flaviendelangle commented May 23, 2024

I had the same problem in my personal app

IMHO this is a bad practice:

const label =
  op?.headerLabel ??
  apiRef.current.getLocaleText(
    `headerFilterOperator${capitalize(op.value)}` as 'headerFilterOperatorContains',
  );

It crashes even though the headerLabel is supposed to be optional.
Not sure how we could improve it (wrap in a try to avoid crashing and render an empty label? check if the translation key exists before calling getLocaleText? Create a getOptionalLocaleText method?), but right now it's a bad DX.

To be clear, this code have been here for ages, it's not a new problem at all, it was already here when I was working on the grid.

@flaviendelangle
Copy link
Member

Similar issues:

#11029
#10270

@ankurheble
Copy link
Author

Thanks, this somehow didn't work in my local even though headerLabel was provided... but it might be something i'm doing wrong! I see the demo link you provided is working.. I'll try this again.. thanks @MBilalShafi

@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 May 24, 2024
@ankurheble
Copy link
Author

ankurheble commented May 24, 2024

@MBilalShafi I tried your solution but it appears to work only for 1 custom filter.. I tried to include the default numeric filters along with the custom one and it still fails with the same translation key error.
Code snippet changed for operator declaration as below:

const quantityOnlyOperators: GridFilterOperator<any, number>[] = [
  ...getGridNumericOperators(),
  {
    label: "Between",
    value: "between",
    headerLabel: "Between",
    getApplyFilterFn: (filterItem) => {
      if (!Array.isArray(filterItem.value) || filterItem.value.length !== 2) {
        return null;
      }
      if (filterItem.value[0] == null || filterItem.value[1] == null) {
        return null;
      }
      return (value) => {
        return (
          value !== null &&
          filterItem.value[0] <= value &&
          value <= filterItem.value[1]
        );
      };
    },
    InputComponent: InputNumberInterval,
  },
];

Updated my codesandbox to reflect this https://codesandbox.io/p/sandbox/nostalgic-pascal-9twstk?file=%2Fsrc%2FDemo.tsx%3A91%2C1-114%2C3

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 24, 2024
@MBilalShafi
Copy link
Member

MBilalShafi commented May 26, 2024

Thank you @ankurheble for providing the updated demo, this indeed is something we could fix internally.
I've raised a PR for the fix to this particular problem, you can test it in this live csb example too. Let me know if it looks good to you now.

For the broader problem, I agree the dx is not great and it should be improved, thank you @flaviendelangle for the suggestion, I remember we did discuss the problem during the implementation of header filters but settled on the same thing which was in normal filters back then.

I've created a separate issue to track and solve this problem.

Copy link

⚠️ 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.

@ankurheble: 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.

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! feature: Filtering on header Related to the header filtering (Pro) feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants