-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[docs] Improve "Example projects" page design #37007
Conversation
|
||
'&:hover': { | ||
borderColor: | ||
theme.palette.mode === 'dark' ? '#173A5E' : 'grey.300', |
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.
For some reason, using primaryDark
, or even just primary
was not working. Had to fall back to using plain hex codes 😭
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.
That's because the ExampleCollection.js
is considered a demo so it could not access the branding theme.
To be able to use branding theme, I switch from demo
to component
in the markdown file:
- {{"demo": "ExampleCollection.js", "hideToolbar": true, "bg": "inline"}}
+ {{"component": "docs/src/modules/components/ExampleCollection.js"}}
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.
Oh, that makes a ton of sense, thank you! Is that documented somewhere? 😅
Netlify deploy previewBundle size report |
Love this! Definitely a huge upgrade. One thing to consider: we could have examples that are more complex than the ones listed here—see this comment—for example, Material + Vite + Tailwind. How might we fit examples like that into this format? |
@samuelsycamore The heading can always wrap but, following the example, to make a shorter one, we could just cut "Base" from it, as it'd be sitting in an equivalent "Example projects" page within the Base docs, I think? We could also play with the icon design a bit so, say, Vite + Tailwind logos are overlapped. All in all, it might be good to expose this component I created⎯ |
<Box | ||
sx={{ | ||
display: 'flex', | ||
flexWrap: 'wrap', | ||
alignItems: 'baseline', | ||
}} |
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.
@siriwatknp Hey! Just for the sake of learning, I'm curious why the preference for using the Box
component rather than the Stack
?
@siriwatknp heya! 👋 Mind giving this one a review? |
Noted @danilo-leal. Sorry for the late response. I was busy with the Reactathon conference the whole week. Will definitely take the last look after landed! 🛫 |
@siriwatknp Woo, sweet, thanks for answering ⎯ no rush, too, though! |
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.
👍 Great improvement!
Co-authored-by: siriwatknp <[email protected]>
- [Vite.js](https://github.com/mui/material-ui/tree/master/examples/material-vite) ([TypeScript version](https://github.com/mui/material-ui/tree/master/examples/material-vite-ts)) | ||
- [Use styled-components as style engine](https://github.com/mui/material-ui/tree/master/examples/material-cra-styled-components) ([TypeScript version](https://github.com/mui/material-ui/tree/master/examples/material-cra-styled-components-ts)) | ||
- [Next.js + @mui/styles (v4 migration helper)](https://github.com/mui/material-ui/tree/master/examples/material-next-ts-v4-v5-migration) | ||
{{"component": "docs/src/modules/components/ExampleCollection.js"}} |
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.
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.
You tell me! Is it better, code-wise?
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.
I would imagine that
{{"component": "docs/src/modules/components/ExampleCollection.js"}} | |
{{"component": "modules/components/ExampleCollection.js"}} |
in the docs page would have worked without the need to change the docs-infra: https://github.com/mui/material-ui/pull/37007/files#diff-8896d42292e63fa8530b9808a333b4770fee0e9fbe33d57e0b4ab5b3b723ed3dR223.
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.
Oh actually, the location of the demo. This is now with the rest of the global component: docs/src/modules/components/
. This folder is meant for global components.
If this component is only used on this page, I think we should move it, we can host it in the same folder as the docs docs/data/material/getting-started/example-projects/
so that if we ever refactor the page, we would remove this dead demo at the same time.
Shall we move the file?
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.
I'll leave it up to @siriwatknp to decide on this one, then, as he pushed that commit on!
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.
Looking back at this, I think one of the reasons to have it moved over to that global folder was so it could pick up the theme. Is there a way so that it sits locally with the markdown files but picking up the branding theme?
It looks great 👍🔥 |
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.
- One design feedback trying to use the new example. The hover style got me confused, I thought it was a precursor indicator so I could click, but it's not the case:
Screen.Recording.2023-05-25.at.14.01.44.mov
- On the content structure, the Tailwind CSS example feels strange. I think that it should either clearly mention CRA or be removed and added under CRA.
const moduleID = path | ||
.join(this.rootContext, 'src', componentName.replace(/^docs\/src/, '')) | ||
.replace(/\\/g, '/'); |
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.
Is this change needed? https://github.com/mui/material-ui/pull/37007/files#r1205292380
const moduleID = path | |
.join(this.rootContext, 'src', componentName.replace(/^docs\/src/, '')) | |
.replace(/\\/g, '/'); | |
const moduleID = path.join(this.rootContext, 'src', componentName).replace(/\\/g, '/'); | |
`` |
- [Vite.js](https://github.com/mui/material-ui/tree/master/examples/material-vite) ([TypeScript version](https://github.com/mui/material-ui/tree/master/examples/material-vite-ts)) | ||
- [Use styled-components as style engine](https://github.com/mui/material-ui/tree/master/examples/material-cra-styled-components) ([TypeScript version](https://github.com/mui/material-ui/tree/master/examples/material-cra-styled-components-ts)) | ||
- [Next.js + @mui/styles (v4 migration helper)](https://github.com/mui/material-ui/tree/master/examples/material-next-ts-v4-v5-migration) | ||
{{"component": "docs/src/modules/components/ExampleCollection.js"}} |
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.
Oh actually, the location of the demo. This is now with the rest of the global component: docs/src/modules/components/
. This folder is meant for global components.
If this component is only used on this page, I think we should move it, we can host it in the same folder as the docs docs/data/material/getting-started/example-projects/
so that if we ever refactor the page, we would remove this dead demo at the same time.
Shall we move the file?
Just giving it a bump so it's one less bullet list throughout the docs!
https://deploy-preview-37007--material-ui.netlify.app/material-ui/getting-started/example-projects/