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] Missing icon and installation instructions in new landing page template #41043

Closed
samuelsycamore opened this issue Feb 10, 2024 · 3 comments · Fixed by #41080
Assignees
Labels
docs Improvements or additions to the documentation package: material-ui Specific to @mui/material

Comments

@samuelsycamore
Copy link
Member

samuelsycamore commented Feb 10, 2024

Steps to reproduce

No response

Current behavior

I'm tinkering around with https://github.com/mui/material-ui/tree/master/docs/data/material/getting-started/templates/landing-page, and as I'm getting set up I'm finding it breaks in two ways:

  1. README doesn't include instructions to install @mui/lab (needed for the Masonry component)
  2. LandingPage.js and LandingPage.tsx try to import SvgMaterialDesign from 'docs/src/icons/SvgMaterialDesign' which doesn't exist outside of the material-ui repo

Expected behavior

I should be able to follow the instructions in the README and install the template and all of its dependencies with no errors. Step 2 in the README should include @mui/lab, and the template folder should either include the Material Design icon or else be revised to remove it.

Context

No response

Your environment

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: landing page

@samuelsycamore samuelsycamore added docs Improvements or additions to the documentation status: waiting for maintainer These issues haven't been looked at yet by a maintainer nextjs labels Feb 10, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 10, 2024

This issue makes me think of #14604.

I could see this as a single file, copy & paste, boom, it works. https://mui.com/material-ui/getting-started/templates/landing-page/ is heavy.

instructions to install @mui/lab (needed for the Masonry component)

Wait what. We can't use the lab Masonry component for landing pages, it wasn't built to be compatible with it.

Why it doesn't work? Because it's layout shift guarantee. See #36673 (comment). That Masonry component we built is for the kind you use for https://react-grid-layout.github.io/react-grid-layout/examples/0-showcase.html use cases: dashboards.

In the design of this Masonry component, we might have shot ourselves in the foot by placing too much value in a11y and not enough having something that works well for non-a11y use cases. I think we could give up on having a correct DOM order. It's what I did in the end in #37975 but who cares?
So I wouldn't be against dropping correct DOM order. In the future, we will get it from https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_grid_layout/Masonry_layout.

@oliviertassinari oliviertassinari added package: material-ui Specific to @mui/material and removed nextjs labels Feb 10, 2024
@danilo-leal danilo-leal removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 12, 2024
@danilo-leal
Copy link
Contributor

@zanivan I guess the simplest course of action for now is removing the Material Design icon and swapping out the Masonry component for a Grid. 👍

@zanivan
Copy link
Contributor

zanivan commented Feb 13, 2024

I guess the simplest course of action for now is removing the Material Design icon and swapping out the Masonry component for a Grid. 👍

Yep, I think it'll do the trick. I'l open a PR :)


Wait what. We can't use the lab Masonry component for landing pages, it wasn't built to be compatible with it.

Why it doesn't work? Because it's layout shift guarantee. See #36673 (comment). That Masonry component we built is for the kind you use for https://react-grid-layout.github.io/react-grid-layout/examples/0-showcase.html use cases: dashboards.

It's important to have this mentioned on the docs then. Especially since the demos aren't exactly related to dashboards.

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
4 participants