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] New docs design, early feedback #588

Open
22 tasks
oliviertassinari opened this issue Sep 5, 2024 · 5 comments
Open
22 tasks

[docs] New docs design, early feedback #588

oliviertassinari opened this issue Sep 5, 2024 · 5 comments
Labels
docs Improvements or additions to the documentation scope: docs-infra Specific to the docs-infra product

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 5, 2024

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:

SCR-20240905-tuhk
  • 2. The h2 h3 text selection doesn't work, it looks like we should restore the previous UX
Screen.Recording.2024-09-05.at.22.57.10.mov
  • 3. Code blocks are tabbable, it looks like they shouldn't be.
SCR-20240905-tvib
  • 4. When using the reset focus, it would be great to be able to visualize where the focus is
SCR-20240905-uavc

e.g.

Screen.Recording.2024-09-05.at.23.14.06.mov
  • 5. Code in light mode, are we sure about this?
SCR-20240905-ubdw

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.

SCR-20240905-ucuq

mui/material-ui#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: w3c/csswg-drafts#1936.

  • 8. Yarn lowercase? This is used in the Terminal.
SCR-20240905-ufmg

e.g. https://react.email/docs/components/link

SCR-20240905-ufog SCR-20240905-uibc SCR-20240905-ujfi
  • 13. until we have a custom scroll bar, a small scroll bar here could make sense:
SCR-20240905-ujij

mui/material-ui#41148

SCR-20240905-ukuh SCR-20240905-ulcr
  • 16. Important, we are missing the canonical links to avoid duplication of page on Google
SCR-20240905-ulxz
  • 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:

Screen.Recording.2024-09-05.at.23.48.07.mov
  • 19. We should add the equivalent of the .Mui-fixed class name on the fixed element so there is no layout shift on Windows:
Screen.Recording.2024-09-05.at.23.50.11.mov

https://master--base-ui.netlify.app/components/react-dialog/

  • 20. The initial HTML docs size output seems broken:

Before: 200KB https://deploy-preview-477--base-ui.netlify.app/base-ui/react-tabs/

SCR-20240906-baan

After: 919KB https://master--base-ui.netlify.app/components/react-tabs/

SCR-20240906-babf

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.

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation scope: docs-infra Specific to the docs-infra product status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 5, 2024
@michaldudak
Copy link
Member

michaldudak commented Sep 6, 2024

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 vercel/next.js#42170 (comment) and vercel/next.js#42170 (comment).
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 michaldudak removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 6, 2024
@flaviendelangle
Copy link
Member

flaviendelangle commented Sep 6, 2024

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

@colmtuite
Copy link
Contributor

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.

@vladmoroz
Copy link
Contributor

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.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 11, 2024

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

SCR-20240911-oqud

(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 vercel/next.js#42170 (comment) and vercel/next.js#42170 (comment).
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:

  1. Code formatting is done with inline style. It looks like it should be done with class names. This could save a lot of space.
  2. We render the code of demos that we don't need. It's actually annoying as I scroll down the page, my scroll gets absorbed by those scrollable code demos. So it both slows down the initial load experience and the scroll experience.
  3. self.__next_f.push( with all the RSC what you flagged
  4. The fetch of links in viewport is not going to work for a sidenav. It doubles the bandwidth pressure on Netlify which is already borderline:
Screen.Recording.2024-09-11.at.17.12.50.mov
Screen.Recording.2024-09-11.at.17.13.29.mov

I think we have to turn the links to be lazy loaded, on hover, not on in viewport. vercel/next.js#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.

  1. On the page transitions, we also load a lot more:

Before: 13.7K gzipped, 66k payload
SCR-20240911-pdxh

After: 11.4K gzipped, 245K payload
SCR-20240911-peqh

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.

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

Screen.Recording.2024-09-11.at.18.11.21.mov

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 scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

No branches or pull requests

5 participants