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] refactor: theme to CSS variables #16588

Merged
merged 12 commits into from
Feb 18, 2025

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented Feb 14, 2025

Re-revert of #15704

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

mui-bot commented Feb 14, 2025

@romgrk romgrk marked this pull request as ready for review February 16, 2025 01:03
@romgrk romgrk requested a review from a team February 16, 2025 01:03
Comment on lines +8 to +11
const CSSVariablesContext = React.createContext({
className: 'unset',
tag: <style href="/unset" />,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to the mui-next prototype, this PR inserts an inline style tag to inject the classname for CSS variables.

const className = `${CLASSNAME_PREFIX}-${description.id}`;
const cssString = `.${className}{${variablesToString(description.variables)}}`;
const tag = <style href={`/${className}`}>{cssString}</style>;
return { className, tag };
Copy link
Member

Choose a reason for hiding this comment

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

Is this SSR-compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, inline <style> tags are SSR compatible. The description.id is a stable hash so there's also no hydration error.

Comment on lines +30 to +31
const cssString = `.${className}{${variablesToString(description.variables)}}`;
const tag = <style href={`/${className}`}>{cssString}</style>;
Copy link
Member

Choose a reason for hiding this comment

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

So the variables are scoped to a specific instance of the data grid 👍🏻
And then the className is applied to GridRootStyles.
Even though the GridRootStyles div cannot reach the CSS variables values (the style tag is a child of GridRootStyles), we can use CSS variables there because we only apply them to elements down the tree.
Do I get this correctly?

Copy link
Contributor Author

@romgrk romgrk Feb 18, 2025

Choose a reason for hiding this comment

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

It doesn't matter where the <style> tag is, it contains something like this:

<style href="/DataGridVariables-123">
.DataGridVariables-123 {
  --DataGrid-t-spacing-unit: 4px;
  /* ... */
}
</style>

The .DataGridVariables-123 classname is available globally even if the style tag is a child of the grid's root container. We need the global classname to target childs in portals, outside of the grid's root container.

@romgrk romgrk merged commit ff0a240 into mui:master Feb 18, 2025
18 checks passed
@romgrk romgrk deleted the refactor-agnostic-css-variables branch February 18, 2025 00:42
@romgrk romgrk restored the refactor-agnostic-css-variables branch February 21, 2025 06:27
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.

4 participants