-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[base][docs] Export Base UI theme in stylesheet #39694
Conversation
Netlify deploy previewhttps://deploy-preview-39694--material-ui.netlify.app/ Bundle size report |
I'll leave my two cents, but feel free to ignore it, since these seem questions that are above my developer skills to make the final call 😅
If it's something that we can easily replace later, I'd go with a custom class name for now, at least until we decide how we are going to take the next steps.
Sincerely, I don't know. Using the If the objective is just showcasing the theme, and customizing it, I don't see any issues on using However, if we want to allow users to export and use this stylesheet in their projects, maybe we'll have to dig further to understand what's the best approach. Would it be a problem if the user had to install |
@mnajdova I might have missed some context here. Before I can provide any suggestions, may I ask what's the Base UI theme for? |
@siriwatknp for sure, sorry. The idea is that we would like to offer plain CSS themes for Base UI, that people can export and use in their apps. This will consist of three steps:
The aim of this PR is just to extract the theme in a separate file (step 1.). However, to do so we need to align on some expectations on how we are going to generate the styles for the components (these are the questions I have in the PR description). For now, I just added the styles for two components, but it should show the intention.
It should be easily replaceable, and we won't be blocked by #39467. Once that issue is resolved, we can replace the class names.
The objective here should be to make it available for people to export it, otherwise, I don't see why we would do this at all. If it is just for showcasing we don't need the hustle to export it to stylesheet and think of ways to make it customizable. I would avoid showcasing any MUI System related feature on this page, it is about showing how Base UI can be used fairly easily using plain CSS without any other dependencies. |
I agree 100%!
Just a clarifying question then: How can we make the theme switching feature exportable without relying on MUI System? |
I assume we would generate the CSS vars required and append them on the stylesheet before generating the file. We don't need a dependency on the system for this. |
Ideally, yes, so the components don't require any customization. I just started working on #39467, so we'll have class names that don't clash with Material UI.
100% agree - this should be just Base UI + plain CSS. |
Alright, until we have this feature, I will use the custom class name I have now, and we'll replace it later with the Base UI specific class name. |
docs/data/base/components/number-input/NumberInputIntroduction/css/index.js
Outdated
Show resolved
Hide resolved
docs/data/base/components/number-input/NumberInputIntroduction/css/index.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Marija Najdova <[email protected]>
Actually, if we use a built-in Base UI class names, all our demos will be styled using this stylesheet, which is not ideal. We should consider placing this gallery on a page with separate CSS, so the stylesheet doesn't affect everything else on the docs. |
- Fixed TextArea box-shadow and border radi - Fixed font-family import on TablePagination - Fixed Switch style and sizes
As we discussed earlier today, I've reviewed the cyan palette and converted them to HSL values, making it easy to create various palettes by adjusting the HUE. The CSS stylesheet is already updated with the HEX values. Here are the HSL values:
|
I see two potential ways of how to do this
I couldn't find a way to circumvent the Next.js error that global styles are imported outside of _app.js, so I am keeping the page for now as is. Anyways, I would like to merge this one as is, and iterate in follow up PRs. |
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.
What's the use case for base-theme.css
?
@@ -29,6 +29,7 @@ import findActivePage from 'docs/src/modules/utils/findActivePage'; | |||
import { pathnameToLanguage } from 'docs/src/modules/utils/helpers'; | |||
import getProductInfoFromUrl from 'docs/src/modules/utils/getProductInfoFromUrl'; | |||
import './global.css'; | |||
import './base-theme.css'; |
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.
https://nextjs.org/docs/app/building-your-application/styling/css-modules
![](https://private-user-images.githubusercontent.com/3165635/284356852-6875496d-0ea5-4f54-ac74-c64c7a89ba9e.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NTczNjYsIm5iZiI6MTczOTY1NzA2NiwicGF0aCI6Ii8zMTY1NjM1LzI4NDM1Njg1Mi02ODc1NDk2ZC0wZWE1LTRmNTQtYWM3NC1jNjRjN2E4OWJhOWUucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIxNSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMTVUMjIwNDI2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9ZTE3YzMzYjg2MzczYTEwZDFjN2JkNmRlYTUyMGY0ZDg3MTQwYWQzYjgzYzY0Y2ViMjkwNzQ5NzlhMjNlMmQzYyZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.XCES5a5ovG0Ysbo3nRkjq-Waqpjs3QXYRZR7H7n4G1M)
import './base-theme.css'; | |
// TODO app directory: move import to relevant page | |
import './base-theme.css'; |
background-color: var(--GalleryButton-disabled-background-color, var(--grey-200)); | ||
color: var(--GalleryButton-disabled-color, var(--grey-700)); | ||
border: 0; | ||
cursor: not-allowed; |
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.
Against the cursor policy we use so far
cursor: not-allowed; | |
cursor: default; |
x the other cases
box-shadow: 0 0 0 4px var(--GalleryButton-focusVisible-boxShadow-color, var(--cyan-200)); | ||
outline: none; | ||
} | ||
.GalleryButton:disabled { |
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.
Doesn't work with custom hosts
.GalleryButton:disabled { | |
.GalleryButton.Mui-disabled { |
Part of #40108
This PR exports the Base UI docs's theme in a separate styleshseet. Preview: https://deploy-preview-39694--material-ui.netlify.app/experiments/base/components-gallery/
For testing dark mode, select the "Enable automatic dark mode" option that you can find under More tools -> Rendering (on chrome):
![Screenshot 2023-11-01 at 12 11 05](https://private-user-images.githubusercontent.com/4512430/279657335-36681fd2-9bf9-4cf0-96a1-ad181d74b240.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NTczNjUsIm5iZiI6MTczOTY1NzA2NSwicGF0aCI6Ii80NTEyNDMwLzI3OTY1NzMzNS0zNjY4MWZkMi05YmY5LTRjZjAtOTZhMS1hZDE4MWQ3NGIyNDAucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIxNSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMTVUMjIwNDI1WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NTE5YjM2MzExNWU1YTVkMjRkODcxZWZiMTFkYzk2YzhlNTE3NmI3YTk5OTJmMDQ4Nzk4NzQwNjk4MWY2OWU2ZiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.E36fTFcSNC8vgBoGqzqelInqU4BnOG4KDNGwl3CwLsA)
Notes:
Maybe try in this PR:
Next steps: