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] Remove GridFormatterParams completely #12660

Merged
merged 5 commits into from
Apr 4, 2024

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented Apr 3, 2024

Closes #12658

We didn't complete this breaking change. This is now a bug at runtime for users, so we need to remove it even if we didn't do it at v7 release.

@romgrk romgrk added the component: data grid This is the name of the generic UI component, not the React module! label Apr 3, 2024
@mui-bot
Copy link

mui-bot commented Apr 3, 2024

Deploy preview: https://deploy-preview-12660--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against a7b0423

Copy link
Member

@MBilalShafi MBilalShafi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting use case for the inevitable BCs during the stable phase.

P.S. Could deprecation be used instead?

@@ -641,7 +641,7 @@ describe('<DataGridPremium /> - Aggregation', () => {
it('should use the aggregation function valueFormatter if defined', () => {
const customAggregationFunction: GridAggregationFunction = {
apply: () => 'Agg value',
valueFormatter: (params) => `+ ${params.value}`,
valueFormatter: (value) => `+ ${value}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably document this in the migration guide and release notes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already in. This was missed during the refactor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failing argos SS could be fixed by:

diff --git a/docs/data/data-grid/aggregation/AggregationValueFormatter.tsx b/docs/data/data-grid/aggregation/AggregationValueFormatter.tsx
index 1a3f4c5a1..e5be77639 100644
--- a/docs/data/data-grid/aggregation/AggregationValueFormatter.tsx
+++ b/docs/data/data-grid/aggregation/AggregationValueFormatter.tsx
@@ -48,7 +48,7 @@ const firstAlphabeticalAggregation: GridAggregationFunction<string, string | nul
       return sortedValue[0];
     },
     label: 'first alphabetical',
-    valueFormatter: (params) => `Agg: ${params.value}`,
+    valueFormatter: (value) => `Agg: ${value}`,
     columnTypes: ['string'],
   };

@romgrk romgrk enabled auto-merge (squash) April 4, 2024 02:27
@romgrk romgrk merged commit ac02582 into mui:master Apr 4, 2024
15 checks passed
@cherniavskii
Copy link
Member

Thanks! I overlooked this in #11573

@romgrk romgrk deleted the fix-grid-formatter-params branch April 4, 2024 17:05
joakimtveter pushed a commit to joakimtveter/mui-x that referenced this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The GridValueFormatterParams interface has NOT been removed in v7
4 participants