-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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] Batch small changes #32170
[docs] Batch small changes #32170
Conversation
michaldudak
commented
Apr 7, 2022
- Updated the bundle size of the Popper (as per [Popper] Allow setting default props in a theme #30118 (review))
- Added a TODO comment to remove the Safari CSS transition bug workaround ([Grow] Add a workaround for Safari 15.4 CSS transition bug #31975 (comment))
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.
👌
@@ -15,7 +15,7 @@ Some important features of the `Popper` component: | |||
|
|||
- 🕷 Popper relies on the 3rd party library ([Popper.js](https://popper.js.org/)) for perfect positioning. | |||
- 💄 It's an alternative API to react-popper. It aims for simplicity. | |||
- 📦 [8 kB gzipped](/size-snapshot). | |||
- 📦 [17.3 kB gzipped](/size-snapshot). |
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.
? https://mui.com/size-snapshot
- 📦 [17.3 kB gzipped](/size-snapshot). | |
- 📦 [24.9 kB gzipped](/size-snapshot). |
I'm not sure if it even makes sense to have all these demos https://mui.com/components/popper/. Maybe simply linking the base page and saying it also has the sx
prop would be better, no duplication of content, less confusion.
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.
Why does bundlephobia show 17.3, then?
I'm not sure if it even makes sense to have all these demos https://mui.com/components/popper/. Maybe simply linking the base page and saying it also has the sx prop would be better, no duplication of content, less confusion.
It's a hard call - on the other hand devs using just Material UI could expect docs for everything exported from @mui/material within the Material UI space, so duplication is justified.
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 right, there is a 7.6 kB difference. Maybe emotion's related? AFAIK bundlephobia doesn't take peer dependencies into account, our bundle size scripts has externals: /^(date-fns|dayjs|luxon|moment|react|react-dom)(\/.*)?$/,
in its webpack config. So https://mui.com/size-snapshot takes emotion into account.
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.
It's a hard call - on the other hand devs using just Material UI could expect docs for everything exported from @mui/material within the Material UI space, so duplication is justified.
I would 👍 to keep the page but to strip down the content. Instead, we would teach developers the inheritance pattern. It would be similar to how Dialog extends Modal and we don't duplicate the content but cross link: https://mui.com/components/dialogs/#performance. I think it's much better as it makes having high-quality docs content possible (no duplication effort, which leads to more up-to-date, accurate, comprehensive, and consistent content). I would not work for all components, I think that it works well with Popper because it's mostly unstyled.
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.
Instead, we would teach developers the inheritance pattern. It would be similar to how Dialog extends Modal and we don't duplicate the content but cross link: https://mui.com/components/dialogs/#performance. I think it's much better as it makes having high-quality docs content possible
It may be much better for us as docs authors, but I personally hated this pattern as a developer trying to learn about Material UI in the past.
When I'm learning about a component, I'd like to have all the meaningful information on one page. Having to jump between pages is counterproductive and sometimes confusing. Developers usually don't care (nor they should) that a given component inherits or uses another one internally. They are concerned about the external API.
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 personally hated this pattern as a developer trying to learn about Material UI in the past.
I agree that it comes with its drawback. I would personally give more value to the pros of linking (e.g. teaching how things are inherited to developers) than the pros of duplication (I would fear leads to too much dilution of efforts; a lower quality doc overall for developers). I think that the best of the two would be this pattern: https://www.notion.so/help/synced-blocks, with a visible link to the source so we retain the "teaching of the inheritance structure" feature.
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 one correct too? #31813 (comment)
Good catch. Fixed. |
@oliviertassinari Could we merge this PR and discuss how we approach documenting components like Popper with the whole team? |
@michaldudak Sure, we can move iteratively. The changes here are already a clear step forward.
It might actually deserves a notion page too. I think that we have the exact same problem with the system, for example this duplication opens for questions cc @mui/core |
This is a great question, and particularly relevant to the work I've been doing on the Base docs. Pages like Popper and Portal are now in better shape IMO in the Base docs vs the older Material UI docs, and I think we should migrate the newer drafts over. I agree with @michaldudak that we should not make the developer jump between multiple pages to find all the info they need when dealing with a single component, as a general rule. That means we would err on the side of duplicating content. That's obviously not ideal for us as maintainers, but overall I think it's far preferable for the reader. |
I think that we will need to broaden the scope of the problem. I think that the implications go beyond the tradeoff that we have discussed so far (the pain of jumping docs vs. content quality), it's also about teaching developers our different products. For example, when you use Radix, you are forced to learn about Stitches https://www.radix-ui.com/docs/primitives/components/dialog#examples. When you use Tailwind UI, they force you to learn Headless UI. Right now, we bundle everything in |
I see what you're getting at, and I agree that it makes sense from the perspective of educating our users on how to better use our product suite as a whole. It's a complex UX problem to solve. |