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

[base][docs] Export Base UI theme in stylesheet #39694

Merged
merged 15 commits into from
Nov 15, 2023

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Nov 1, 2023

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

Notes:

  • some of the components used the exported classes from the Base UI components, while some of the demos used custom classes on all slots. In this scenario I believe custom classes for all slots should be used, so that we can isolate these components from the rest of the docs. When exporting the theme so that it can be used externally we can use the default Base UI classes. We should decide if the same should apply for state classes, like Mui-disabled etc.
  • regarding implementation, the dark theme is at this point only re-defining some CSS variables

Maybe try in this PR:

  • make it render in iframe and avoid using global CSS
  • use Mui prefix for the class names

Next steps:

  • rename cyan to primary
  • make the theme exportable

@mnajdova mnajdova added docs Improvements or additions to the documentation package: base-ui Specific to @mui/base labels Nov 1, 2023
@mui-bot
Copy link

mui-bot commented Nov 1, 2023

Netlify deploy preview

https://deploy-preview-39694--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 1207bc9

@zanivan
Copy link
Contributor

zanivan commented Nov 1, 2023

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 😅

Do we use the Base UI class names? I used custom class names for now to avoid conflicts with the theme as the stylesheet is imported globally (we can likely easily find + replace this later anyways)

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.

How do we want to define the dark mode styles? So far in the CSS demos we have conditions based on isDarkMode. I used the prefers-color-scheme: dark media query in the stylesheet for setting up some dark mode variables.

Sincerely, I don't know. Using the prefers-color-scheme seems to be the safest way for now, since it doesn't use any third party logic, I guess.

If the objective is just showcasing the theme, and customizing it, I don't see any issues on using System logics, be it the theme or the useTheme, like we currently do.

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 System package to use the useTheme? And would the user need to do anything else if he wants to use the stylesheet with isDarkMode?

@siriwatknp
Copy link
Member

@mnajdova I might have missed some context here. Before I can provide any suggestions, may I ask what's the Base UI theme for?

@mnajdova
Copy link
Member Author

mnajdova commented Nov 2, 2023

@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:

  1. Extract the currently used styles for the plain CSS demos of Base UI in a standalone stylesheet
  2. Define CSS tokens (via CSS variables) for altering the styles of the components.
  3. Create a page showcasing all components and some playground for customizing the theme and exporter for the theme

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.


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.

It should be easily replaceable, and we won't be blocked by #39467. Once that issue is resolved, we can replace the class names.

If the objective is just showcasing the theme, and customizing it, I don't see any issues on using System logics, be it the theme or the useTheme, like we currently do.

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 System package to use the useTheme? And would the user need to do anything else if he wants to use the stylesheet with isDarkMode?

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.

@zanivan
Copy link
Contributor

zanivan commented Nov 2, 2023

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 agree 100%!

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.

Just a clarifying question then: How can we make the theme switching feature exportable without relying on MUI System?

@mnajdova
Copy link
Member Author

mnajdova commented Nov 3, 2023

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.

@michaldudak
Copy link
Member

Do we use the Base UI class names?

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.

How do we want to define the dark mode styles?

prefers-dark-mode is the least opinionated and the most standards-compliant, so I'd stick to it for these demos.

I would avoid showcasing any MUI System related feature on this page

100% agree - this should be just Base UI + plain CSS.

@mnajdova
Copy link
Member Author

mnajdova commented Nov 3, 2023

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.

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.

Signed-off-by: Marija Najdova <mnajdova@gmail.com>
@mnajdova mnajdova marked this pull request as ready for review November 7, 2023 12:45
@michaldudak
Copy link
Member

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.

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
@zanivan
Copy link
Contributor

zanivan commented Nov 10, 2023

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:

50: hsl(189, 76, 95)
100: hsl(189, 70, 86)
200: hsl(189, 59, 75)
300: hsl(189, 49, 60)
400: hsl(189, 68, 38)
500: hsl(189, 76, 21)
600: hsl(189, 80, 18)
700: hsl(189, 84, 15)
800: hsl(189, 86, 12)
900: hsl(189, 90, 8)

@mnajdova
Copy link
Member Author

mnajdova commented Nov 14, 2023

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.

I see two potential ways of how to do this

  1. we can use CSS modules and add a classname on the root div and bump the specificity of each class to be a child of the div (we can remove this when exporting the stylesheet).
  2. same as ☝️ but still depend on the global stylesheet as now

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.

@mnajdova mnajdova merged commit cec367a into mui:master Nov 15, 2023
20 checks passed
Copy link
Member

@oliviertassinari oliviertassinari left a 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';
Copy link
Member

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

Suggested change
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;
Copy link
Member

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

Suggested change
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 {
Copy link
Member

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

Suggested change
.GalleryButton:disabled {
.GalleryButton.Mui-disabled {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation package: base-ui Specific to @mui/base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants