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] Add more content to the Material Icons page #40445

Closed

Conversation

danilo-leal
Copy link
Contributor

@danilo-leal danilo-leal commented Jan 5, 2024

This PR closes #40441 — it adds three additional sections to the Material Icons page to give the foundational context for folks that are primarily landing on this page instead of the Icons page. The clear trade-off here is that the "Browse library" component stays below the fold, so it is maybe not as discoverable (not sure). But, it seems worth it as, even though we have basically the same information in the SvgIcon section of the Icons page, not many people were necessarily going there to get informed about the basics of the Material Icons usage.

https://deploy-preview-40445--material-ui.netlify.app/material-ui/material-icons/

@danilo-leal danilo-leal added docs Improvements or additions to the documentation package: material-ui Specific to @mui/material labels Jan 5, 2024
@danilo-leal danilo-leal self-assigned this Jan 5, 2024
@mui-bot
Copy link

mui-bot commented Jan 5, 2024

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against d951240

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.

This page is the most used page of the whole MUI site, and by a very large margin: https://analytics.google.com/analytics/web/#/p353089763/reports/explorer?params=_u..nav%3Dmaui&r=4770517765&ruid=all-pages-and-screens,life-cycle,engagement&collectionId=4770480590

People want the search bar, so I'm convinced that it must be at the top of this page.

Comment on lines +49 to +61
### Size

Use the `fontSize` prop to toggle between small, medium (default, 24x24px), or large icon sizes.
You can also use the `sx` prop to pick arbitrary values that are outside of this built-in scale.

{{"demo": "SizeMaterialIcon.js"}}

### Color

Use the `color` prop to choose a palette key from the theme.
It defaults to the `main` value of its respective palette.

{{"demo": "ColorMaterialIcon.js"}}
Copy link
Member

@oliviertassinari oliviertassinari Jan 6, 2024

Choose a reason for hiding this comment

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

I don't think we should document this, It feels like it doesn't teach people that it's https://deploy-preview-40445--material-ui.netlify.app/material-ui/icons/#svgicon. I think it's better to have them struggle a bit, so that once they figure it out, they can have more autonomy with the product.

I feel that the root issue is that we don't correctly link to https://mui.com/material-ui/icons/#material-svg-icons from the relevant places.

@@ -2,4 +2,5 @@ Broken links found by `pnpm docs:link-check` that exist:

- https://mui.com/blog/material-ui-v4-is-out/#premium-themes-store-✨
- https://mui.com/material-ui/discover-more/roadmap/#priorities
- https://mui.com/material-ui/icons/#svgicon/
Copy link
Member

Choose a reason for hiding this comment

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

This error is correct, we shouldn't ignore it.

Suggested change
- https://mui.com/material-ui/icons/#svgicon/

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 11, 2024
@danilo-leal danilo-leal deleted the material-icons-page-enhancement branch March 19, 2024 12:20
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 PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][docs] Basic information about how to use the Material Icons
4 participants