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] Final polish on Base docs - formatting and style consistency #33156

Merged
merged 58 commits into from Jul 22, 2022

Conversation

samuelsycamore
Copy link
Member

@samuelsycamore samuelsycamore commented Jun 15, 2022

https://deploy-preview-33156--material-ui.netlify.app/base/react-badge/

This is the final (?) follow up to #32072 to polish the existing Base documentation and implement consistent formatting and style.

This PR covers all component pages in the Base docs, as well as one tiny tweak to the Overview page to fix the title. Changes include:

  • removing HTML from <p> descriptions for SEO
  • reverting ClickAwayListener's page title for SEO
  • removing (redundant) bundle size info where present (it's already in the ComponentLinkHeader)
  • adding lead-in descriptions for demos, explaining what's going on and pointing out important bits to notice
  • improving and standardizing headers for better TOC navigation
  • formalizing Introduction and Anatomy sections
  • implementing universal formatting
  • adding new callouts
  • cleaning up random typos and style and grammar errors
  • expanding and revising content that was still unclear after the last pass

The primary purpose of this final round of polishing is to establish a universal template for component pages that we could use when writing and revising pages going forward. It looks like this:

# Name of component

<p>description</p>

## Introduction

### Features (optional)

- 🥉 only included when there are at least 3 interesting things to point out

{{ComponentLinkHeader}}

## Component(s)

### Usage

### Basics

### Anatomy

### Slot props

## Hook(s) (where applicable)

### Noteworthy features or examples

## Customization

### Props and features that apply to both components and hooks

## Accessibility

## API

Once we're satisfied with this template, we can move on to apply it to all component pages across MUI's products. I believe this will significantly improve the overall experience of our docs.

This final pass turned out to be a much bigger undertaking than I anticipated so I'm sure there are tiny details I've missed! The big-picture formatting is what I'm most concerned with here.

@mui-bot
Copy link

mui-bot commented Jun 15, 2022

No bundle size changes

Generated by 🚫 dangerJS against 8f485ec

@samuelsycamore samuelsycamore added docs Improvements or additions to the documentation package: base-ui Specific to @mui/base labels Jun 15, 2022
Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Looks amazing to me-scanned it through a couple of times and nothing stood out.
Awesome work 🙌

@samuelsycamore
Copy link
Member Author

Thanks @danilo-leal! Maybe @mnajdova and/or @michaldudak could give this one last review to make sure it's all good before merging? It's a big one so I want to make sure we have many sets of eyes on it.

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I could just find two issues around missing slots in the slots section. The rest look splendid :)

Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

Great work, @samuelsycamore, I really like it! I do have just two comments.

Hooks _do not_ support [slot props](#slot-props), but they do support [customization props](#customization).

:::info
Hooks give you the most room for customization, but require more work to implement.
Copy link
Member

Choose a reason for hiding this comment

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

I recently realized another use case for hooks - if you're building a component library that can be customized further by developers (e.g. using the componentsProps prop), use hooks. Unstyled components are better suited to build "closed" libraries (that's why we use hooks to build Joy components, for example).

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think this is worth mentioning on all pages? Or maybe it would be better suited for the Usage page?

Copy link
Member

Choose a reason for hiding this comment

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

I suppose having it just on the Usage page should be enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. I'll add it to #33272

docs/data/base/components/badge/badge.md Outdated Show resolved Hide resolved
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

👍

@samuelsycamore
Copy link
Member Author

Just applied the suggestions from @mnajdova's last review. I think this is ready to go finally! 🤞

@samuelsycamore
Copy link
Member Author

I think it's time to merge this beast! There will always be more room for improvement, but I think we've established a very strong template here that I'd like to start applying across all product documentation.

@samuelsycamore samuelsycamore merged commit d207a3b into mui:master Jul 22, 2022
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.

A great step forward 👍

Hooks give you the most room for customization, but require more work to implement.
With hooks, you can take full control over how your component is rendered, and define all the custom props and CSS classes you need.

You may not need to use hooks unless you find that you're limited by the customization options of their component counterparts—for instance, if your component requires significantly different [structure](#component-slots).
Copy link
Member

Choose a reason for hiding this comment

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

Would this be better?

Suggested change
You may not need to use hooks unless you find that you're limited by the customization options of their component counterparts—for instance, if your component requires significantly different [structure](#component-slots).
You may not need to use hooks unless you find that you're limited by the customization options of their component counterparts—for instance, if your component requires a significantly different [structure](#component-slots).


```js
`ButtonUnstyled` replaces the native HTML `<button>` element.
Copy link
Member

Choose a reason for hiding this comment

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

It could be interesting to explain why (The value proposition for a developer to replace a native button) Or maybe it's better to say "extends" it.


`ClickAwayListener` is a utility component that listens for click events outside of its child.
## Introduction
Copy link
Member

Choose a reason for hiding this comment

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

Is the plan to add this ## Introduction in the rest of the docs? It seems that in Material UI we have systematically skipped it (the introduction content was left to be described under the h1), it's not consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think we should apply this format across the Material UI docs as well.


{{"demo": "AccessibleBadges.js", "defaultCodeOpen": false}}
{{"demo": "AccessibleBadges.js"}}
Copy link
Member

Choose a reason for hiding this comment

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

I think that we could keep the preview disabled. It doesn't explain how the feature works, so I feel noise for developers

Suggested change
{{"demo": "AccessibleBadges.js"}}
{{"demo": "AccessibleBadges.js", "defaultCodeOpen": false}}

Comment on lines -53 to -55
## Component size

- 📦 [2.0 kB gzipped](https://bundlephobia.com/package/@mui/base@latest)
Copy link
Member

Choose a reason for hiding this comment

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

In the future, we could use Bundlephobia, https://bundlephobia.com/api/exports-sizes?package=@mui/base@5.0.0-alpha.90 and show the value associated with what we export,

Screenshot 2022-07-23 at 01 35 44

pastelsky/bundlephobia#389

```

{{"component": "modules/components/ComponentLinkHeader.js", "design": false}}
### Basics
Copy link
Member

Choose a reason for hiding this comment

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

When landing on https://deploy-preview-33156--material-ui.netlify.app/base/react-badge you need to scroll a lot before you can find an actual demo of the component or one that you can copy and paste into your project. It contrasts with what we had before https://mui.com/base/react-badge/ or https://www.radix-ui.com/docs/primitives/components/tooltip. So I conclude that we miss an introduction demo, maybe under ### Basics?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a good point. We could definitely use a basic intro demo here, and probably for some other Base components as well.

Copy link
Member

Choose a reason for hiding this comment

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

I will raise this discussion at the next team meeting, seeing

it feels nice to see a full demo first before everything else. It can even then turn into an SEO hack https://www.notion.so/mui-org/Page-image-generator-75bc177fefe64c0b899a42a6f919a907.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 23, 2022

Regarding #33156 (comment), Radix seems to do it:

Screenshot 2022-11-23 at 23 08 43

cc @michaldudak, I think it's great to communicate to the developers that bundle size is a key constraint in how components are designed.

daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
…ui#33156)

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
@samuelsycamore samuelsycamore deleted the base-small-fixes branch April 3, 2024 17:23
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: base-ui Specific to @mui/base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants