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] Add installation component #36766

Closed
wants to merge 7 commits into from

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Apr 3, 2023

After seeing the installation section (with pnpm added`) in #36650 and https://deploy-preview-36717--material-ui.netlify.app/base/getting-started/quickstart/#installation, I cannot tolerate anymore 🤣.

This PR adds Installation component that renders Tabs. Here is what it looks like:

image

  • It store the latest value that you select in the local storage, so that the next time you open the page it focuses on that tab. (however, the UX is not great because the content swap after mounted. Feedbacks welcome)

  • Let's you specify dep managers for each page

    {{"component": "components/markdown/Installation", "dep": "@mui/base", "managers": ["yarn", "npm", "pnpm"]}}

Note: At first, I wanted to extend the markdown to enable us to write some syntax that will generate Tabs but I could not find a good reference because GMF (Github markdown format) does not support tabs. The closest one is Rmarkdown but it looks really complicated to implement.

Todos


Preview: https://deploy-preview-36766--material-ui.netlify.app/experiments/docs/installation/

@mui-bot
Copy link

mui-bot commented Apr 3, 2023

Netlify deploy preview

Bundle size report

No bundle size changes

Generated by 🚫 dangerJS against 098fd4a

@siriwatknp siriwatknp added docs Improvements or additions to the documentation scope: docs-infra Specific to the docs-infra product labels Apr 3, 2023
@danilo-leal
Copy link
Contributor

Very excited that we're working on this! At first, I'd suggest something within the markdown, but it's cool that you've explored that already. As far as the design goes, though, I imagined the tabs to be integrated into the code block, not an individual tab element on the page body (especially now that we're adding tabs to the high-level page navigation). I'd also like to see if there's a way to generalize this component more instead of something targeted at installation commands. We may want to use tabs within the code block for other stuff.

Screen Shot 2023-04-03 at 09 58 29

Additionally, I'm curious if we could also experiment with ways to display the file's name within the code block either ⎯ like we did on Joy's theme builder page. It would be visually very similar to the tabs, and it could even have a tab (exactly like Joy's example).

Screen Shot 2023-04-03 at 09 57 16

@siriwatknp
Copy link
Member Author

siriwatknp commented Apr 4, 2023

@flaviendelangle I thinking about this syntax to make it works with DatePickers:

{{"component": "components/markdown/Installation", "installation": "%Plan %DateLibrary", "managers": ["yarn", "npm"], "Plan": [["Community", "@mui/x-date-pickers"], ["Pro", "@mui/x-date-pickers-pro"]], "DateLibrary": ["dayjs", "date-fns", "luxon", "moment"]}}

Do you get it at first glance 🤔?

Note: at first I'd like to use {{Plan}} {{DateLibary}} as a template but the markdown parser does not work with this so I have to go with %Plan %DateLibrary.

@siriwatknp
Copy link
Member Author

@danilo-leal updated to

image

@flaviendelangle
Copy link
Member

@siriwatknp I'm fine with this synthax
I'd be curious to see the resulting UI 👍

@siriwatknp
Copy link
Member Author

@flaviendelangle How about this?

image

cc @danilo-leal

@flaviendelangle
Copy link
Member

cc @alexfauquette @LukasTy

For me we also need to be able to clean the label ("DateLibrary" is not great without a space).

@LukasTy
Copy link
Member

LukasTy commented Apr 7, 2023

For me we also need to be able to clean the label ("DateLibrary" is not great without a space).

Agreed, IMHO, a bit of spacing between the label and toggle button group wouldn't hurt, but other than that it looks great. 👍

Comment on lines +216 to +221
<HighlightedCode
language="bash"
code={`${
{ yarn: 'yarn add', npm: 'npm install', pnpm: 'pnpm add' }[item]
} ${finalInstallation}`}
/>
Copy link
Member

Choose a reason for hiding this comment

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

Probably too complicated, but would it be possible to split this into two distinct components:

  • HighlightedCodeWithSelect similar to HighlightedCode but accepts a callback for code with the selected state
  • Installation That reuses the previous one to do what you're currently doing.

The example I've in mind is the pickers setups. It is not at all about npm or yarn but has 2 selects to adapt the code. In such a case, we could reuse the HighlightedCodeWithSelect to get an up-to-date UI

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 I will take a look at it.

name: 'tabs',
level: 'block',
start(src) {
const match = src.match(/===/);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure to understand. You plan to add the possibility to write the following markdown, but I do not see how it should be used

===
Some content
===

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, this should be reverted.


## Basic

{{"component": "components/markdown/Installation", "installation": "@mui/material @emotion/react @emotion/styled"}}
Copy link
Member

@oliviertassinari oliviertassinari Apr 9, 2023

Choose a reason for hiding this comment

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

This PR is interesting, and much better than 3 headers for npm, yarn, pnpm. Now, I would like to bring some light on the level of abstraction, figuring out where is the right balance:

Update

Should this cover npm updates: https://mui.com/material-ui/migration/migration-v4/#update-react too? Maybe it needs to be a bit higher level?

More generic?

We could consider a higher-level API, this would allow to support any type of bash actions:

Suggested change
{{"component": "components/markdown/Installation", "installation": "@mui/material @emotion/react @emotion/styled"}}
{{"component": "components/markdown/TabsBash", "tabs": ["npm", "yarn", "pnpm"], "npm": "npm install @mui/material @emotion/react @emotion/styled", "yarn add @mui/material @emotion/react @emotion/styled", pnpm: "pnpm add @mui/material @emotion/react @emotion/styled" }}

CodeGroup?

But then maybe it's the notion of code group that we need, tabs between any time of code blocks.

Screenshot 2023-04-09 at 18 19 57

Screenshot 2023-04-09 at 18 18 03

Tabs?

I have seen even higher-level APIs, aTabs API:

Screenshot 2023-04-09 at 18 01 49

Then people add a code block inside each tab. This feels a bit cheap, two examples that don't feel so great: https://docusaurus.io/docs/installation#build, https://stripe.com/docs/stripe-js/react#setup

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for sharing. The ideal result looks like the code group from Mintlify.

@samuelsycamore
Copy link
Contributor

Can we move forward with this? I like the idea of a higher-level API for this component but in the meantime I'd love to see this implemented in the docs sooner rather than later.

@alexfauquette
Copy link
Member

I'd love to see this implemented in the docs sooner rather than later.

If something is not well done at the first shot in the docs, it ends up there for a long time before someone finds the motivation/time to fix it.

@siriwatknp would you need some help on this PR? This week I might be busy but after the alpha release, I should get a bit more bandwidth

@siriwatknp
Copy link
Member Author

I'd love to see this implemented in the docs sooner rather than later.

If something is not well done at the first shot in the docs, it ends up there for a long time before someone finds the motivation/time to fix it.

@siriwatknp would you need some help on this PR? This week I might be busy but after the alpha release, I should get a bit more bandwidth

Definitely @alexfauquette, thanks for the help!.

@alexfauquette
Copy link
Member

I'm closing it since the codeblock is now available on docs and the remaining interaction will be added in the X repo:

mui/mui-x#9644

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 PR: out-of-date The pull request has merge conflicts and can't be merged scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants