[docs] Add CodeSandbox/Stackblitz to the rest of the templates #43708

siriwatknp · 2024-09-11T10:51:39Z

Preview:

Follow up PR from #43604

Mostly deleting duplicated theme files in each template (switch to use the shared theme)
🫣
Fixed minor CSS after switching to shared TemplateFrame
Refactor theme.palette.* to use theme.vars.palette.*

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2024-09-11T10:58:34Z

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against d75ead4

zanivan · 2024-09-11T12:39:37Z

@siriwatknp, these are the issues I found during my initial review. I won't dive too deep into visual tweaks for the templates, like the box-shadow on the marketing page appbar, since that's not the focus of this PR. I've kept the feedback primarily focused on layout and interaction issues based on TemplateFrame:

There's an issue on the mobile layout of the Marketing-page, Sign-in, and Checkout templates:
The AppBar on Blog template is not centralized:
The drawer in the Blog/Marketing page template is placed behind the TemplateFrame:
The same issue happens in the mobile drawer of Checkout template:
There's a vertical scroll in the Checkout template:

Screen.Recording.2024-09-11.at.08.57.18.mov

There's an arrow icon behind the color mode select (same as in this comment):

siriwatknp · 2024-09-12T03:49:40Z

Updates:

Make the theme selector smaller in mobile viewport

Addressed all of the comments

…late/new-structure

DiegoAndai · 2024-09-12T12:51:26Z

Question

Mostly deleting duplicated theme files in each template (switch to use the shared theme)

I thought the idea was to have duplicate theme files so each template would have all the code. This way users can clone one template without having to navigate to the shared theme. Is this no longer the case? @zanivan

Some issues found

1. Checkout mobile dark mode

2. Blog images not showing up

3. Stackblitx only: Sessions graph width not working as expected

Screen.Recording.2024-09-12.at.09.42.37.mov

zanivan · 2024-09-12T13:29:26Z

I thought the idea was to have duplicate theme files so each template would have all the code. This way users can clone one template without having to navigate to the shared theme. Is this no longer the case? @zanivan

This was the idea, as the only way users could get the templates before was by copying the folder from the project. However, integrating with CodeSandbox/StackBlitz solves this issue, since @siriwatknp packaged the shared theme within the template files when opened on either platform

zanivan · 2024-09-12T13:57:56Z

Some bugs/issues I noticed while reviewing it again:

The TemplateFrame toolbar sometimes gets stuck when scrolling the dashboard template:
The AppBar from Blog and Marketing-page templates is not centralized:
There's an issue in all templates in both StackBlitz and CodeSandbox regarding the dark/light theme:

Just a disclaimer, there are other small UI improvements since we changed the way the TemplateFrame works, and some other adjustments, that can be addressed in a future PR.

DiegoAndai · 2024-09-12T14:12:07Z

This was the idea, as the only way users could get the templates before was by copying the folder from the project. However, integrating with CodeSandbox/StackBlitz solves this issue, since @siriwatknp packaged the shared theme within the template files when opened on either platform

This makes sense. I'll leave it up to you to decide. I think we could be a bit more cautious: We have this new and improved way to use the templates with the sandboxes, and we should promote it, but it might not be best to "remove" the old way right away. There's no rush; we can wait and see how users react to this new system, and if it sticks, we can remove the old one (duplicated themes).

If you decide to remove the duplicated files, then let's also remove the copying mechanism and CI check, which were added here: #43220

zanivan · 2024-09-12T16:07:07Z

We have this new and improved way to use the templates with the sandboxes, and we should promote it, but it might not be best to "remove" the old way right away. There's no rush; we can wait and see how users react to this new system, and if it sticks, we can remove the old one (duplicated themes).

No strong opinion on this. Maybe we can wait until the feature to copy the theme to the clipboard is ready before removing the theme folders from the templates

siriwatknp · 2024-09-13T02:43:07Z

I think we could be a bit more cautious: We have this new and improved way to use the templates with the sandboxes, and we should promote it, but it might not be best to "remove" the old way right away

Thanks for pointing out. Let me clarify the removal of the duplicate theme. Using the shared-theme import has more benefits in several ways:

Better DX, updating the shared theme immediately update all the templates, no longer need to run the script every time you change the code. Also, when new template is added, you don't need to remember to update the script.
Reduced complexity, no script, no CI check. Faster feedback loop, if you forget to run the script, you need to wait for more 15-20 minutes until the CIs are green.
Almost 20k lines are removed, the repo size is already huge (> 120MB). If we could optimize it, we should do it.
The duplicated theme is not required to make the CodeSandbox/Stackblitz works.

With all the above reasons, I don't see why we should keep the duplicated theme.

siriwatknp · 2024-09-13T03:07:05Z

There's an issue in all templates in both StackBlitz and CodeSandbox regarding the dark/light theme:

This is a regression from #43632 due to <StyledEngineProvider injectFirst>. I'm investigating.

siriwatknp · 2024-10-10T06:17:08Z

There's a bug with Dashboard background color:

Found the root cause, will open a separate PR but it's not related to this PR.

There's a bug with the theme Select where an icon appears behind the light/dark color mode toggle:

I could not see it in Chrome (in any templates), what's your browser and which template?

In almost all templates, when in mobile breakpoint, we end up losing the color mode toggle. Since users will likely see this in a split view, we need to have it there somehow—maybe we could draw some inspiration from the previous layouts that we had before adding the TemplateFrame, and bring it to CodeSandbox/Stackblitz UIs

Added the toggle to Dashboard, Marketing, and Blog mobile viewport (only show when open with Sandboxes)

Images aren't loading, probably because we use static images from the docs. Do you have any suggestions about how we might solve this?

I fixed it by using a new process.env.TEMPLATE_IMAGE_URL. In dev, it will use the / path but when deployed it will use the deployed url.

Sign-in, Sign-up and Sign-in-side color mode toggle could be improved, there are some bugs when clicking on it, as well as layout shift and overlapping.

Improved, can you check it again. For "there are some bugs when clicking on it", I think it comes from the custom styles from the theme, can you try to fix it?

…p/material-ui into pr/43708

zanivan · 2024-10-10T10:35:41Z

@siriwatknp I tried to update the branch from upstream to get the changes from the light/dark background color PR #44059 to review the PR again, but noticed that doing this it also committed 170a9c9. Is this right?

zanivan · 2024-10-10T10:44:29Z

I could not see it in Chrome (in any templates), what's your browser and which template?

It's happening on Sign-in, Sign-up and Sign-in-side. I've tested on Arc (chrome), Firefox and Safari, and it happens in all of these browsers.

For "there are some bugs when clicking on it", I think it comes from the custom styles from the theme, can you try to fix it?

Will work on it 👍

…late/new-structure

siriwatknp · 2024-10-14T09:51:33Z

There's a bug with the theme Select where an icon appears behind the light/dark color mode toggle

@zanivan found the issue, it's fixed in the latest change.

zanivan · 2024-10-14T12:02:59Z

Awesome—I think we're almost there! I just have some small remarks:

On the custom theme, by default, we're only using the icons rounded, so we need to replace these in the StackBlitz/CodeSandbox (let me know if you want me to make this one):

import DarkModeIcon from '@mui/icons-material/DarkModeOutlined';
import LightModeIcon from '@mui/icons-material/LightModeOutlined';

I don't know if it's expected, so correct me if I'm wrong, but images aren't loading on both StackBlitz/CodeSandbox;
On the checkout template, we could add a little extra space atop, so we wouldn't have this overlapping components:

siriwatknp · 2024-10-15T01:50:22Z

@zanivan All fixed. There is one caveat for the images, when you open with CodeSandbox, the image will use https://mui.com as a host.

zanivan

I think everything design-wise for adding StackBlitz and CodeSandbox is covered—nice work, @siriwatknp! It might be good to get an engineering review before merging, though @DiegoAndai @aarongarciah

Side note: There are some design issues with the templates, but they aren’t directly related to the CodeSandbox/StackBlitz addition. I think it makes sense to address these in follow-up PRs.

oliviertassinari

Very cool, I love to see this.

oliviertassinari · 2024-10-16T17:56:42Z

docs/data/material/getting-started/templates/blog/README.md

@@ -4,7 +4,7 @@

 <!-- #default-branch-switch -->

-1. Copy the files into your project, or one of the [example projects](https://github.com/mui/material-ui/tree/master/examples).
+1. Copy these folders (`blog` and `shared-theme`) into your project, or one of the [example projects](https://github.com/mui/material-ui/tree/master/examples).


Off-topic

We could provide a CLI script that does this. Related to #39588

I really dig this! I created a shaping page kind of related to this https://www.notion.so/mui-org/material-ui-Installation-CLI-e32438c1880c466c8b7075d289e825c4 I think it'd be really nice to have the templates as starting point for new projects.

oliviertassinari · 2024-10-16T18:02:07Z

docs/pages/material-ui/getting-started/templates/blog.js

@@ -1,11 +1,14 @@
 import * as React from 'react';
 import AppTheme from 'docs/src/modules/components/AppTheme';


Could we rename this? We have two AppTheme in the React tree, and this component has nothing to do with theming.

Actually, seeing:

material-ui/docs/pages/premium-themes/onepirate/index.js

Line 7 in 064fa67

<AppTheme title="Onepirate theme - MUI" description="A onepirate theme">

Could we also add a title to those template pages? This could be a better DX.

I believe we should also remove those description. What's the point, the page is <meta name="robots" content="noindex,nofollow" />, it's absurd.

Could we rename this? We have two AppTheme in the React tree, and this component has nothing to do with theming.

Thanks for the feedback but I would do it in a separate PR.

It makes sense, it's already off in HEAD, not specific to this PR, so best to split the effort in smaller chunks 👍

DiegoAndai

LGTM, one small nit.

DiegoAndai · 2024-10-17T15:18:05Z

docs/data/material/getting-started/templates/shared-theme/themePrimitives.ts

@@ -395,5 +395,9 @@ export const shape = {
 };

 // @ts-ignore


Nit: is this still required?

Yes, it's still required.

DiegoAndai · 2024-10-17T15:19:05Z

@zanivan lets remember to tweet about this when it releases 🙏🏼

siriwatknp added 6 commits September 11, 2024 17:11

update checkout template

6a69cf5

update marketing page

273cb02

update sign-in-side template

9a709c2

update sign-up

43b36e7

update blog

4c28a7f

use CSS theme variables

29907fa

siriwatknp requested a review from zanivan September 11, 2024 10:51

siriwatknp added the scope: docs-infra Specific to the docs-infra product label Sep 11, 2024

siriwatknp changed the title ~~[docs-infra] Apply new structure to the rest of the templates~~ [docs-infra] Add CodeSandbox/Stackblitz to the rest of the templates Sep 11, 2024

siriwatknp marked this pull request as draft September 11, 2024 10:58

siriwatknp removed the request for review from zanivan September 11, 2024 10:58

siriwatknp added 2 commits September 12, 2024 10:47

fix all issues

0b1cf8f

update screenshots

e33ef1d

siriwatknp marked this pull request as ready for review September 12, 2024 03:49

siriwatknp requested review from zanivan and DiegoAndai September 12, 2024 04:16

siriwatknp added 2 commits September 12, 2024 11:16

Merge branch 'master' of https://github.com/mui/material-ui into temp…

e710571

…late/new-structure

fallback to theme

790fcc2

siriwatknp assigned siriwatknp and zanivan Sep 12, 2024

siriwatknp added 2 commits October 10, 2024 13:45

improve sign in-* templates

c1865d6

run dedupe

c2f5e54

siriwatknp mentioned this pull request Oct 10, 2024

[material-ui] Make the palette always return new light and dark object #44059

Merged

1 task

zanivan added 2 commits October 10, 2024 07:32

Merge remote-tracking branch 'upstream/master' into pr/43708

88a6a6e

Merge branch 'template/new-structure' of https://github.com/siriwatkn…

170a9c9

…p/material-ui into pr/43708

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 10, 2024

siriwatknp added 3 commits October 14, 2024 16:18

Merge branch 'master' of https://github.com/mui/material-ui into temp…

04a0b60

…late/new-structure

revert

883e35a

hide select root

5cc855c

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 14, 2024

siriwatknp added 3 commits October 14, 2024 20:21

add TEMPLATE_IMAGE_URL to images in the templates

97d344c

docs ts format

55d7dc4

fallback to mui.com

d75ead4

siriwatknp requested a review from zanivan October 15, 2024 09:05

zanivan approved these changes Oct 16, 2024

View reviewed changes

oliviertassinari reviewed Oct 16, 2024

View reviewed changes

oliviertassinari added docs Improvements or additions to the documentation package: material-ui Specific to @mui/material enhancement This is not a bug, nor a new feature and removed scope: docs-infra Specific to the docs-infra product labels Oct 16, 2024

oliviertassinari changed the title ~~[docs-infra] Add CodeSandbox/Stackblitz to the rest of the templates~~ [docs] Add CodeSandbox/Stackblitz to the rest of the templates Oct 16, 2024

DiegoAndai approved these changes Oct 17, 2024

View reviewed changes

siriwatknp merged commit 91d0d91 into mui:master Oct 21, 2024
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Add CodeSandbox/Stackblitz to the rest of the templates #43708

[docs] Add CodeSandbox/Stackblitz to the rest of the templates #43708

siriwatknp commented Sep 11, 2024 •

edited

Loading

mui-bot commented Sep 11, 2024 •

edited

Loading

zanivan commented Sep 11, 2024

siriwatknp commented Sep 12, 2024

DiegoAndai commented Sep 12, 2024

zanivan commented Sep 12, 2024

zanivan commented Sep 12, 2024

DiegoAndai commented Sep 12, 2024 •

edited

Loading

zanivan commented Sep 12, 2024

siriwatknp commented Sep 13, 2024 •

edited

Loading

siriwatknp commented Sep 13, 2024 •

edited

Loading

siriwatknp commented Oct 10, 2024 •

edited

Loading

zanivan commented Oct 10, 2024

zanivan commented Oct 10, 2024

siriwatknp commented Oct 14, 2024

zanivan commented Oct 14, 2024

siriwatknp commented Oct 15, 2024

zanivan left a comment •

edited

Loading

oliviertassinari left a comment

oliviertassinari Oct 16, 2024

zanivan Oct 16, 2024

oliviertassinari Oct 16, 2024

oliviertassinari Oct 16, 2024 •

edited

Loading

siriwatknp Oct 17, 2024

oliviertassinari Oct 17, 2024 •

edited

Loading

DiegoAndai left a comment

DiegoAndai Oct 17, 2024

siriwatknp Oct 21, 2024

DiegoAndai commented Oct 17, 2024

		@@ -1,11 +1,14 @@
		import * as React from 'react';
		import AppTheme from 'docs/src/modules/components/AppTheme';

[docs] Add CodeSandbox/Stackblitz to the rest of the templates #43708

[docs] Add CodeSandbox/Stackblitz to the rest of the templates #43708

Conversation

siriwatknp commented Sep 11, 2024 • edited Loading

mui-bot commented Sep 11, 2024 • edited Loading

Netlify deploy preview

Bundle size report

zanivan commented Sep 11, 2024

siriwatknp commented Sep 12, 2024

DiegoAndai commented Sep 12, 2024

Question

Some issues found

zanivan commented Sep 12, 2024

zanivan commented Sep 12, 2024

DiegoAndai commented Sep 12, 2024 • edited Loading

zanivan commented Sep 12, 2024

siriwatknp commented Sep 13, 2024 • edited Loading

siriwatknp commented Sep 13, 2024 • edited Loading

siriwatknp commented Oct 10, 2024 • edited Loading

zanivan commented Oct 10, 2024

zanivan commented Oct 10, 2024

siriwatknp commented Oct 14, 2024

zanivan commented Oct 14, 2024

siriwatknp commented Oct 15, 2024

zanivan left a comment • edited Loading

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Oct 16, 2024

Choose a reason for hiding this comment

zanivan Oct 16, 2024

Choose a reason for hiding this comment

oliviertassinari Oct 16, 2024

Choose a reason for hiding this comment

oliviertassinari Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

siriwatknp Oct 17, 2024

Choose a reason for hiding this comment

oliviertassinari Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

DiegoAndai left a comment

Choose a reason for hiding this comment

DiegoAndai Oct 17, 2024

Choose a reason for hiding this comment

siriwatknp Oct 21, 2024

Choose a reason for hiding this comment

DiegoAndai commented Oct 17, 2024

siriwatknp commented Sep 11, 2024 •

edited

Loading

mui-bot commented Sep 11, 2024 •

edited

Loading

DiegoAndai commented Sep 12, 2024 •

edited

Loading

siriwatknp commented Sep 13, 2024 •

edited

Loading

siriwatknp commented Sep 13, 2024 •

edited

Loading

siriwatknp commented Oct 10, 2024 •

edited

Loading

zanivan left a comment •

edited

Loading

oliviertassinari Oct 16, 2024 •

edited

Loading

oliviertassinari Oct 17, 2024 •

edited

Loading