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

[docs] Improve "Example projects" page design #37007

Merged
merged 6 commits into from
May 8, 2023

Conversation

danilo-leal
Copy link
Contributor

@danilo-leal danilo-leal commented Apr 25, 2023

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/

Screen Shot 2023-04-25 at 02 25 48

@danilo-leal danilo-leal added the docs Improvements or additions to the documentation label Apr 25, 2023

'&:hover': {
borderColor:
theme.palette.mode === 'dark' ? '#173A5E' : 'grey.300',
Copy link
Contributor Author

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 😭

Copy link
Member

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"}}

Copy link
Contributor Author

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? 😅

@mui-bot
Copy link

mui-bot commented Apr 25, 2023

Netlify deploy preview

Bundle size report

No bundle size changes

Generated by 🚫 dangerJS against b73c219

@samuelsycamore
Copy link
Contributor

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?

@danilo-leal
Copy link
Contributor Author

@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⎯ExampleCollection⎯ as a more general one, instead of having one for each docs, so we maintain consistency and adhere to these expansions use cases there as well (as variants, probably). What do you think?

Comment on lines 130 to 135
<Box
sx={{
display: 'flex',
flexWrap: 'wrap',
alignItems: 'baseline',
}}
Copy link
Contributor Author

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?

@danilo-leal
Copy link
Contributor Author

@siriwatknp heya! 👋 Mind giving this one a review?

@siriwatknp
Copy link
Member

@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! 🛫

@danilo-leal
Copy link
Contributor Author

@siriwatknp Woo, sweet, thanks for answering ⎯ no rush, too, though!

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Great improvement!

@danilo-leal danilo-leal merged commit e8d91ec into master May 8, 2023
@danilo-leal danilo-leal deleted the improve-example-projects-design branch May 8, 2023 12:59
binh1298 pushed a commit to binh1298/material-ui that referenced this pull request May 17, 2023
@oliviertassinari oliviertassinari added the package: material-ui Specific to @mui/material label May 25, 2023
- [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"}}
Copy link
Member

Choose a reason for hiding this comment

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

Should we update the rest of the docs to use the full import path?

Screenshot 2023-05-25 at 12 03 33

Copy link
Contributor Author

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?

Copy link
Member

@oliviertassinari oliviertassinari May 25, 2023

Choose a reason for hiding this comment

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

I would imagine that

Suggested change
{{"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.

Copy link
Member

@oliviertassinari oliviertassinari May 25, 2023

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?

Copy link
Contributor Author

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!

Copy link
Contributor Author

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?

@oliviertassinari
Copy link
Member

It looks great 👍🔥

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.

  1. 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
  1. 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.

Comment on lines +222 to +224
const moduleID = path
.join(this.rootContext, 'src', componentName.replace(/^docs\/src/, ''))
.replace(/\\/g, '/');
Copy link
Member

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

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

@oliviertassinari oliviertassinari May 25, 2023

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?

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: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants