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] Add support for default props from theme #1019
[DataGrid] Add support for default props from theme #1019
Conversation
packages/grid/_modules_/grid/components/toolbar/DensitySelector.tsx
Outdated
Show resolved
Hide resolved
packages/grid/_modules_/grid/components/toolbar/DensitySelector.tsx
Outdated
Show resolved
Hide resolved
it('should pass translations using the ThemeProvider', () => { | ||
const theme = createMuiTheme({}, bgBG); | ||
|
||
const { getByText } = render( | ||
<div style={{ width: 300, height: 300 }}> | ||
<ThemeProvider theme={theme}> | ||
<DataGrid | ||
{...baselineProps} | ||
showToolbar | ||
components={{ | ||
Toolbar: GridToolbar, | ||
}} | ||
/> | ||
</ThemeProvider> | ||
</div>, | ||
); | ||
|
||
expect(getByText('Гъстота')).not.to.equal(null); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this test is too high-level. We don't need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider this approach instead: https://github.com/mui-org/material-ui/blob/b5bce5007b6d025360fefa887eaf5b0206fab5ae/test/utils/describeConformanceV5.js#L40. We can copy the helper until we upgrade to v5 to be able to simply replace the import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the test for now. If we don't need it it's better to avoid copying the helper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, no preferences for the testing approach in this case
I think that long term, we should have all the public components running inside describeConformance
. Marija had opened mui/material-ui#22371 related to this localization problem.
…e/DataGrid-984-theme-support-default-props
Fixes #984
This PR allows us to do the folowing:
It turns out we don't need the entire hook to achieve what was desired.
I didn't add the
isRtl
since the grid doesn't really support it.Added 1 test.
No docs - I didn't add any since this should have work in the first place. If you have suggestions share and I will also add docs 😉