Open oliviertassinari opened 2 months ago
Thanks for the feedback! We opted to merge the PR sooner, while not being super polished, to be able to create docs and demos in the new format and avoid merge conflicts. As we're not publishing them yet, it should not affect their reception.
Code in light mode, are we sure about this?
We're going to add dark mode to the whole site at some point. I don't see why should we present code in dark mode while the rest of the site is light. If someone prefers dark mode, they'll be able to switch. See https://www.radix-ui.com/primitives/docs/components/dialog, https://react-spectrum.adobe.com/react-aria/Button.html, https://ariakit.org/components/button, https://mantine.dev/core/checkbox/ and others.
No cursor: pointer on buttons?
This is controversial :) Native buttons don't have cursor: pointer.
I have my doubts about the table view, when the prop name and description and default value gets long, it usually become unreadable.
Yes, I second that.
I imagine we want those code demos to be collapsed by default?
Yes, I lean towards this as well.
Yarn lowercase?
The official name is "Yarn".
A number of hydration errors https://master--base-ui.netlify.app/components/react-tabs/
These will be gone when CSS demos are split into multiple files
The code demos is no longer editable
This is by design. We found out many of the demos of components that rely on Floating UI were broken when edited directly, so we decided to omit it. We may look into it in the future.
The initial HTML docs size output seems broken:
This is probably the biggest issue with RSC I found. The problem is that Next.js inserts the serialized state of server components to the page payload (even though it's static). See https://github.com/vercel/next.js/discussions/42170#discussioncomment-8137079 and https://github.com/vercel/next.js/discussions/42170#discussioncomment-8880248. I'll try to work around this problem by making the demos async components (they usually have the most impact on download size). It's worth noting, though, that this additional content is placed at the bottom of the page and does not affect the initial paint. Even with it, we're having a perfect Lighthouse score (it's not the case currently, but it's an issue with another component).
The official name is "Yarn".
I second @oliviertassinari here, for me seeing "Yarn" next to "npm" and "pmpm" is weird even if the official name is "Yarn". One could say that "yarn" is the name of the command line so it make sense as well.
I took a look at how other projects are writting it. Most of them just show "npm" instructions or just "yarn" instructions so no tab UI. For the few that also support this kind of UI, I always saw "yarn" instead of "Yarn":
Thanks for the feedback, I agree on many points. We're not quite ready for low-level feedback yet, as we just merged the PR to make progress (as Michal mentioned). We still have lots of work to do, but we'll open it up for feedback in the next few weeks.
I don't see why should we present code in dark mode while the rest of the site is light.
I agree. If someone wants light mode (like me), just set light mode. If you want dark mode, set the site to dark mode. But having some parts in dark mode while the rest is light mode is the worst situation, it causes the greatest eye-strain because your eyes constantly need to adapt depending on where you're looking. Imagine if your VSC UI chrome was light mode, while the code panel was dark mode, it would be awful.
Thanks @oliviertassinari, I'll stash this feedback for the next iterations.
Just want to second that I'll be taking over how everything in the docs is designed and built soon. We are likely to look into a lot of design tweaks and go over everything with a fine-tooth comb.
I don't see why should we present code in dark mode while the rest of the site is light
@michaldudak The majority of the developers on mui.com are in light mode: proof: https://www.notion.so/mui-org/Product-Analytics-f2e2576203564e6c8dc3429cf305a36f?pvs=4#2ef8caa05f434e56b20c9537171ff2c7. But what about IDEs? I would go back to the hypothesis that I formulated: "I would expect that 90% of the developer use their IDE in dark mode and 10% in light mode". I don't know if this is true, if it's the case, then it looks like a logical move, if it's false, then likely best to keep it consistent.
Personally, I'm terminal & IDE dark mode everything else is light mode (unless I work outside and I use dark mode). I haven't experienced eye-strain issues to date. I dream of a world where GitHub would show the code in dark mode and the UI in light mode 😁.
Examples: of projects doing docs light mode, code dark mode while theme is light mode: https://ui.shadcn.com/docs/components/checkbox, https://tailwindcss.com/docs/flex-wrap, https://garden.zendesk.com/components/tiles, https://evergreen.segment.com/components/file-picker, https://gestalt.pinterest.systems/web/avatargroup, https://v2.chakra-ui.com/, https://orbit.kiwi/components/primitives/buttonprimitive/react/.
Not a strong perspective on this, but more of a user feedback: I have a better experience on shadcn, tailwind, mui docs.
This is controversial :)
@michaldudak I could find two rationales for using cursor: default
that could maybe make sense for us: 1. marketing, to signal to developers that Base UI is more raw, and to be customized. 2. to signal that it's more meant for desktop-heavy applications. But is this really what we want to communicate?
Otherwise, I can't think of one website that I use on a day-to-day basis that doesn't have cursor: pointer. I think it breaks the UX flow most end-users have be trained to expect, regardless of what's right or wrong. Linear is the only one actually, but they allow in their settings to configure this and they don't do it for their marketing page, so doesn't truly count 😁.
(my setting as I was annoyed not knowing what I can click on)
I actually have the same pain points on all macOS apps, anytime the UI doesn't have a hover visual feedback, I get confused on whether I can click or not.
I'm not sure that Figma counts, if it's mostly used through a desktop app wrapper.
This is by design. We found out many of the demos of components that rely on Floating UI were broken when edited directly, so we decided to omit it. We may look into it in the future.
@michaldudak Do we have this issue in Material UI docs? If not, I would conclude it's a regression with the positioning logic that will surface at one point or another with GitHub issues.
This is probably the biggest issue with RSC I found. The problem is that Next.js inserts the serialized state of server components to the page payload (even though it's static). See https://github.com/vercel/next.js/discussions/42170#discussioncomment-8137079 and https://github.com/vercel/next.js/discussions/42170#discussioncomment-8880248. I'll try to work around this problem by making the demos async components (they usually have the most impact on download size). It's worth noting, though, that this additional content is placed at the bottom of the page and does not affect the initial paint. Even with it, we're having a perfect Lighthouse score (it's not the case currently, but it's an issue with another component).
@michaldudak Looking at the HTML output, the problems that I see:
self.__next_f.push(
with all the RSC what you flaggedhttps://github.com/user-attachments/assets/27922a9e-62d6-4c0d-8e46-2e68f42b6fcf
https://github.com/user-attachments/assets/10820690-6c62-424f-971f-773dbcfba4d9
I think we have to turn the links to be lazy loaded, on hover, not on in viewport. https://github.com/vercel/next.js/issues/50412. I don't think that it's possible to deploy this new docs-infra structure to Material UI until solved. I can't see us going from 8.4TB/month to x2 this.
Before: 13.7K gzipped, 66k payload
After: 11.4K gzipped, 245K payload
I suspect that fixing 1. would solve this. So I guess it's fine. It's also not JS but JSON, faster to handle for the browser.
This is the biggest problem of all that I can see. 5. would block Material UI deployment of this new docs-infra, but I think this one should block Base UI deployment too. Developers load all the demos of all the pages in the shared bundle. For example, go to https://master--base-ui.netlify.app/components/react-field/, all the demos of https://master--base-ui.netlify.app/components/react-progress/ are loaded in the JavaScript asset.
The solution might be as simple as creating one entry point per page, as before. Today, we don't really feel the pain in Base UI because we are very early in its journey (we have few demos and components), compare Argos CI records 73. Material UI 1345, MUI 709 demos we take screenshots against (we have more).
Proof:
https://github.com/user-attachments/assets/0d1d2edd-93b3-4b5e-90b0-4be7df48f705
This is feedback on my experience browsing https://master--base-ui.netlify.app/components/react-field/ and comparing the experience with https://deploy-preview-477--base-ui.netlify.app/base-ui/react-field/#labeling-and-descriptive-help-text. I imagine it's still a work in progress, so I won't go too much in-depth, but simply stop on the things that I noticed:
https://github.com/user-attachments/assets/47cbfa63-eab8-41a0-8d12-26db110b83a2
e.g.
https://github.com/user-attachments/assets/7fb9a4e7-1fcf-4bd3-878f-d4237407838c
I mean, I would expect that 90% of the developer use their IDE in dark mode and 10% in light mode. If this is true, aren't we making the code harder to read? from v3 to v4 https://v3.mui.com/demos/buttons/#contained-buttons we made this change.
[ ] 6. The code demos is no longer editable. We have to open CodeSandbox / StackBlitz and wait 10s+ to quickly try changes. I believe it was better with the edition support. We can always play this: https://www.youtube.com/watch?si=sRhbVRHVLRR83pMX&t=574&v=tRsxLLghL1k&feature=youtu.be but 🤷♂️
[ ] 7. No
cursor: pointer
on buttons?https://github.com/mui/material-ui/issues/17871. I mean, I get that it used to be the standard to only use pointer for links, and on desktop it's more intuitive, but how many popular website doesn't use cursor pointer today? I would be afraid that end-users will be confused. The spec made it more open in: https://github.com/w3c/csswg-drafts/issues/1936.
e.g. https://react.email/docs/components/link
[ ] 9. I imagine we want those code demos to be collapsed by default? https://master--base-ui.netlify.app/components/react-field/#labeling-and-descriptive-help-text. I find it makes it harder for me to scan the content of the docs.
[ ] 10. On the API table, we likely miss the
.algolia-lvl3
class name so Algolia can correctly index it.[ ] 11. I have my doubts about the table view, when the prop name and description and default value gets long, it usually become unreadable. Maybe we won't have this issue on Base UI, if we only have simple few components.
[ ] 12. Overflow issue https://master--base-ui.netlify.app/components/react-number-field/
https://github.com/mui/material-ui/pull/41148
[ ] 17. We will need to add the scroll restoration logic, e.g. https://master--base-ui.netlify.app/components/react-tooltip/ doesn't put the tooltip left side nav into the viewport
[ ] 18. I think we should add back the scrollbar size reservation so there is no layout shift when the page height is not large enough to show a scrollbar:
https://github.com/user-attachments/assets/3e5412fe-5236-4b2d-9e42-467e8ce6867e
.Mui-fixed
class name on the fixed element so there is no layout shift on Windows:https://github.com/user-attachments/assets/4bdea446-5bbd-4dba-a199-a81366280e3c
https://master--base-ui.netlify.app/components/react-dialog/
Before: 200KB https://deploy-preview-477--base-ui.netlify.app/base-ui/react-tabs/
After: 919KB https://master--base-ui.netlify.app/components/react-tabs/
[ ] 21. For the URLs, we use https://master--base-ui.netlify.app/components/react-field/. I'm not sure that we need "components". I kind of feel like it makes the URL longer and doesn't bring value in exchange.
[ ] 22. We need ways to crash the docs when the content written by Engineers, Designers, DevEx, PMs is incorrect. We have a bunch of those checks in the "classic" docs-infra, e.g. https://github.com/mui/material-ui/pull/43562.
There is more, e.g. "skip to content", docs feedback, etc., but I think that it's because it's in a WIP mode, so I stopped there.
It's pretty cool that we can have a custom look for the docs infra, I look forward to Material UI using the library default look & feel and not MUI company branding. And the rest of the docs, like for Toolpad to have a more neutral theme as well.