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

[material-ui][docs] Open Material UI template with CodeSandbox/StackBlitz #43604

Merged
merged 43 commits into from
Sep 11, 2024

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Sep 4, 2024

Iterate on #41469

Preview: https://deploy-preview-43604--material-ui.netlify.app/material-ui/getting-started/templates/sign-in/
Preview: https://deploy-preview-43604--material-ui.netlify.app/material-ui/getting-started/templates/dashboard/

Changes

All of the changes are done for SignIn and Dashboard templates only (once this PR is merged, I will open another PR to apply to the rest of the templates).

  • Fix the dark mode flicker by adopting the CSS variables and refactor the theme usage to theme.vars.
  • Move toggle color mode and theme switching to the docs TemplateFrame.js so that the template code is clean and ready to be used.

TemplateFrame Toolbar

  • Add CodeSandbox and StackBlitz buttons
  • Changed the ToggleColorMode to buttons and move it to center. (it's too tight to have everything on the right side)
  • Changed the theme switcher to icon button with a menu to optimize for small screen
image image

@siriwatknp siriwatknp added proof of concept Studying and/or experimenting with a to be validated approach docs Improvements or additions to the documentation labels Sep 4, 2024
@mui-bot
Copy link

mui-bot commented Sep 4, 2024

@siriwatknp siriwatknp changed the title [POC] Open Material UI template via CodeSandbox [material-ui][docs] Open Material UI template with CodeSandbox Sep 5, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 6, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 6, 2024
@siriwatknp siriwatknp changed the title [material-ui][docs] Open Material UI template with CodeSandbox [material-ui][docs] Open Material UI template with CodeSandbox/StackBlitz Sep 6, 2024
Comment on lines -302 to -307
- run:
name: Update the templates shared themes
command: pnpm template:update-theme
- run:
name: '`pnpm template:update-theme` changes committed?'
command: git add -A && git diff --exit-code --staged
Copy link
Member Author

Choose a reason for hiding this comment

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

No longer need to run the script. The shared theme is the source of truth and it will be bundled with the template when users open CodeSandbox or StackBlitz.

@siriwatknp siriwatknp added scope: docs-infra Specific to the docs-infra product and removed proof of concept Studying and/or experimenting with a to be validated approach labels Sep 6, 2024
@siriwatknp siriwatknp requested a review from zanivan September 6, 2024 04:30
@zanivan
Copy link
Contributor

zanivan commented Sep 9, 2024

Hey @siriwatknp I've added a couple tweaks to the Toolbar, and I think it's good to go!

One thing that I think we still need to fix is the template screenshot script. I reckon the toolbar shouldn't be included on those, is it possible? The script is pnpm template:screenshot material-ui

I really like the content of the contributing guide, but its placement on this page feels a bit off. The page is already packed with templates, a Store CTA for premium templates, and Toolpad Core, so adding the guide there might be a bit confusing. How about adding it as a FAQ item like, 'How can I contribute to the free templates?' with a link on the templates page?

@siriwatknp
Copy link
Member Author

Hey @siriwatknp I've added a couple tweaks to the Toolbar, and I think it's good to go!

One thing that I think we still need to fix is the template screenshot script. I reckon the toolbar shouldn't be included on those, is it possible? The script is pnpm template:screenshot material-ui

I really like the content of the contributing guide, but its placement on this page feels a bit off. The page is already packed with templates, a Store CTA for premium templates, and Toolpad Core, so adding the guide there might be a bit confusing. How about adding it as a FAQ item like, 'How can I contribute to the free templates?' with a link on the templates page?

Done!

Copy link
Contributor

@zanivan zanivan left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@siriwatknp siriwatknp mentioned this pull request Sep 11, 2024
@siriwatknp siriwatknp merged commit ff9406a into mui:master Sep 11, 2024
22 checks passed
@aarongarciah
Copy link
Member

aarongarciah commented Sep 11, 2024

some stuff I noticed:

  • The color scheme dropdown trigger is displaying up/down arrows, see screenshot below.
    Screenshot 2024-09-11 at 12 43 03

  • The text in the theme dropdown trigger looks a bit too large, the old one felt more balanced in comparison with the options' text size:

    old new
    Screenshot 2024-09-11 at 12 45 37 Screenshot 2024-09-11 at 12 45 50
  • I think the bar contents being all blue seems to attract more attention because we use blue as a color accent. Not sure it's intentional or not. I think the problem is the bar is too integrated with the rest of the template, we could try make it very obvious that the bar is not part of the template (another background color?), but that's something for the future.

    old
    old

    new
    new

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.

Cool banner, but it looks like we need to continue to iterate on this:

  1. Doesn't work on mobile:
Screen.Recording.2024-09-22.at.00.02.25.mov
  1. Can't really click on it:
Screen.Recording.2024-09-22.at.00.04.33.mov

@zanivan
Copy link
Contributor

zanivan commented Sep 26, 2024

@oliviertassinari sure thing! We're still iterating it on #43708

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 scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants