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] Migrate button demos to base #31395

Merged
merged 18 commits into from
Mar 12, 2022

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Mar 10, 2022

Summary

  • moves the unstyled button demos from Material UI space to MUI Base.
  • fix the build API scripts to support incremental migration
  • auto-gen "Unstyled" section if unstyled: $url is detected in the markdown

Questions?

Should the ButtonUnstyled API link be removed from Material UI?YES
https://deploy-preview-31395--material-ui.netlify.app/material-ui/react-button/

Should the base has -unstyled at the end?No

From: https://deploy-preview-31395--material-ui.netlify.app/base/react-button/
To: https://deploy-preview-31395--material-ui.netlify.app/base/react-button-unstyled/

Should the title & meta has unstyled at the end? Unstyled at the beginning (take a look at Sam's comment)
The title is what appears on the search. I think having unstyled will teach people that base is the unstyled version. But the navigation does not need to have "unstyled".

Should the word "hook" be removed?No (Take a look at Sam's comment)

From: useButton hook
To: useButton

I think use* is already explicit that it is a react hook.

Screen Shot 2565-03-10 at 13 47 29

cc @mui/core


@siriwatknp siriwatknp added the docs Improvements or additions to the documentation label Mar 10, 2022
@mui-bot
Copy link

mui-bot commented Mar 10, 2022

No bundle size changes

Generated by 🚫 dangerJS against 28862db

@siriwatknp siriwatknp requested a review from michaldudak March 10, 2022 06:47
@michaldudak
Copy link
Member

Should the ButtonUnstyled API link be removed from Material UI?

I believe so, yes.

Additionally, I'd add a link to the unstyled component in the Material UI docs, similar to https://mui.com/components/menus/#unstyled

Should the base has -unstyled at the end?

The page is not only about the ButtonUnstyled component, but also the hook. I'm not 100% convinced what's the better option, but I'm leaning more towards not having the -unstyled suffix.

Should the title & meta has unstyled at the end?

As in https://mui.com/material/react-menu/ - the title can describe the page is about unstyled component and a hook.

Should the word "hook" be removed?

I think it'll be easier to search for if we have the full name (useButton)

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 10, 2022
Copy link
Contributor

@samuelsycamore samuelsycamore left a comment

Choose a reason for hiding this comment

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

I went ahead and did a close reading here to implement the style guide.

As for the question of whether the word "hook" should be included, I vote yes. When the docs are talking about components, elements, hooks, etc. all right next to each other, I think it's a good idea to explicitly spell it out, at least the first time it's mentioned.

docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
docs/data/base/components/button/button.md Outdated Show resolved Hide resolved

{{"demo": "UnstyledButtonCustom.js"}}

## useButton hook
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## useButton hook
## The useButton hook

Copy link
Member Author

Choose a reason for hiding this comment

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

Does "The" really need is this case?

docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
docs/data/base/components/button/button.md Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 11, 2022
@siriwatknp siriwatknp marked this pull request as ready for review March 11, 2022 08:10
@siriwatknp
Copy link
Member Author

@michaldudak @mnajdova The build-API script is fixed! Here is the checklist for the future components to migrate from material-ui to base.

  • Remove unstyled component files (.js & .json) from /pages/api-docs/...

  • Remove unstyled component from material-ui component markdown components and add unstyled: $url

    // data/material/components/button/buttons.md
    ---
    product: material-ui
    title: React Button component
    - components: Button, IconButton, ButtonBase, LoadingButton, ButtonUnstyled
    + components: Button, IconButton, ButtonBase, LoadingButton 
    materialDesign: https://material.io/components/buttons
    githubLabel: 'component: button'
    waiAria: https://www.w3.org/TR/wai-aria-practices/#button
    + unstyled: /base/react-button/
    ---
  • Remove unstyled section from the markdown file (the section is auto-generated if it sees unstyled: /base/react-button/ in the markdown header.

  • Add the migrated component to the line: 105 in docs/scripts/buildApiUtils.ts

    const migratedBaseComponents = [...] // this will prevent the API page generation to the old docs

After complete the checklist, run yarn docs:api as usual and you should see the changes in

  • the "Demos" & "API" comment in unstyled component .tsx should link to /base/react-...
  • the API .json in docs/pages/base/api/...json should have the correct demo url

.eslintrc.js Outdated Show resolved Hide resolved
@siriwatknp siriwatknp requested a review from mnajdova March 11, 2022 15:16
@siriwatknp
Copy link
Member Author

@michaldudak @samuelsycamore Since this PR is blocking other MUI Base pages, I think we should continue the content refinement in a separate PR.

@@ -291,6 +291,14 @@ function prepareMarkdown(config) {
const description = headers.description || getDescription(markdown);
const contents = getContents(markdown);

if (headers.unstyled) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@siriwatknp siriwatknp merged commit 40be8b4 into mui:master Mar 12, 2022
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants