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

[DataGrid] Accept row argument in valueFormatter #8836

Closed
2 tasks done
TiagoPortfolio opened this issue May 2, 2023 · 4 comments
Closed
2 tasks done

[DataGrid] Accept row argument in valueFormatter #8836

TiagoPortfolio opened this issue May 2, 2023 · 4 comments
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! dx Related to developers' experience enhancement This is not a bug, nor a new feature performance plan: Premium Impact at least one Premium user v7.x

Comments

@TiagoPortfolio
Copy link
Contributor

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 💡

In the DataGrid component, this is how we can declare a column:

{
  field: 'time',
  headerName: 'Time',
  valueGetter: ({ row }) => row.time.seconds,
  renderCell: ({ row }) => (
    <span>row.time.formatted</span>
  )
}

In this example, I am using renderCell to display the formatted value on the table for the following time object:

type Time = {
  seconds: number
  formatted: string
}

I am parsing the data from the server to be already formatted on my components.
But now I want to export this data as CSV and I want to display the formatted value on the file. For this, I have to use valueFormatter to format the value.
This is how I can get the formatted value using valueFormatter:

{
  field: 'time',
  headerName: 'Time',
  valueGetter: ({ row }) => row.time.seconds,
  valueFormatter: ({ api, id }) => {
    if (id !== undefined) {
      const row = api.getRow<Data>(id)

      return row?.times.formatted
    }

    return ''
}

I think a better approach would be just to make row available on the valueFormatter params

{
  field: 'time',
  headerName: 'Time',
  valueGetter: ({ row }) => row.time.seconds,
  valueFormatter: ({ row }) => row.time.formatted,
}

I believe that many developers are already parsing and formatting the data from the server to be already formatted when the data is used on components. This has the obvious advantage of only needing to format once instead of on every single component where the data is needed. Another advantage is, if we add too much format logic to the valueFormatter function, it can be bad performance-wise because that logic will run on every render (I may be wrong).

In short, my suggestion is to make row argument available on the valueFormatter function :)

Examples 🌈

Expected behavior

Use row argument in valueFormatter

{
  field: 'time',
  headerName: 'Time',
  valueGetter: ({ row }) => row.time.seconds,
  valueFormatter: ({ row }) => row.time.formatted,
}

Motivation 🔦

I was displaying the formatted value on the DataGrid using renderCell but the requirement to be able to export the data from the DataGrid was raised.
With this requirement, I have to use valueFormatter instead to display the formatted value on the exported file.
After converting to valueFormatter, I faced the problem where the valueFormatter function doesn't make much sense if I am not able to access the row to get the formatted value that I have previously formatted.

Order ID 💳 (optional)

54728

@TiagoPortfolio TiagoPortfolio added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 2, 2023
@TiagoPortfolio TiagoPortfolio changed the title [DataGrid] Accept row argument in valueFormatter [DataGrid] Accept row argument in valueFormatter May 2, 2023
@zannager zannager added component: data grid This is the name of the generic UI component, not the React module! plan: Premium Impact at least one Premium user labels May 3, 2023
@cherniavskii
Copy link
Member

Thanks for your suggestion, @TiagoPortfolio!
We might want to make valueFormatter's params more consistent with the ones from valueGetter

@cherniavskii cherniavskii added enhancement This is not a bug, nor a new feature dx Related to developers' experience and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 3, 2023
@romgrk
Copy link
Contributor

romgrk commented Sep 15, 2023

We'll need to update the API for v7 by replacing the params object (params: {...}) => string by passing arguments directly (row, column, etc) => string.

@mhuggins
Copy link

I'm interested in this as well. Specifically, when rendering currencies, the numeric currency value is on one field of a row, while the currency (needed to use for formatting) is stored on a separate field of a row.

@cherniavskii
Copy link
Member

Closing in favor of #10741

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! dx Related to developers' experience enhancement This is not a bug, nor a new feature performance plan: Premium Impact at least one Premium user v7.x
Projects
Status: Done
Development

No branches or pull requests

5 participants