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] Inject default Col Def values before updating the column state #15366

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

k-rajat19
Copy link
Contributor

@k-rajat19 k-rajat19 commented Nov 10, 2024

Closes #15317
Before: https://stackblitz.com/edit/github-3rk5go-nd8osk?file=src%2Fdemo.tsx
After: https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-m49n6j

Fixes #14446
Before: https://codesandbox.io/p/sandbox/wispy-paper-mfjzqs?file=%2Fsrc%2FDemo.tsx%3A1%2C1-23%2C1
After: https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-4lk7v4

Change done in #14456 is a bit misleading, existingState does not contains default col def values rather it contains the default values which are overridden by the values passed in a column prop.

Closes #15409

@k-rajat19 k-rajat19 changed the title [DataGrid] Inject default col Def values before updating the column state [DataGrid] Inject default Col Def values before updating the column state Nov 10, 2024
@mui-bot
Copy link

mui-bot commented Nov 10, 2024

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

Generated by 🚫 dangerJS against f3b4e5a

@k-rajat19 k-rajat19 marked this pull request as ready for review November 10, 2024 16:55
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Nov 10, 2024
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work regression A bug, but worse labels Nov 10, 2024
@@ -380,7 +380,13 @@ export const createColumnsState = ({
}
});

columnsState.lookup[field] = resolveProps(existingState, { ...newColumn, hasBeenResized });
const newColumnWithDefaultValues = resolveProps(defaultColTypeDef, newColumn);
Copy link
Contributor Author

@k-rajat19 k-rajat19 Nov 14, 2024

Choose a reason for hiding this comment

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

do we really need to use resolveProps here to get rid of undefined values? It is breaking other things #15409 where we need to pass undefined values, and to fix #14446 I think we should handle properly undefined values for filterOperators before performing any operations with them?

@sai6855
Copy link
Contributor

sai6855 commented Nov 22, 2024

@romgrk moving #15317 (comment) here, looking at this PR changes, I'm not sure we can extract logic to helper function as changes seems to be context dependent, wdyt?

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! regression A bug, but worse
Projects
None yet
4 participants