-
-
Notifications
You must be signed in to change notification settings - Fork 355
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] Polish to match standard #4344
[docs] Polish to match standard #4344
Conversation
@@ -94,14 +94,14 @@ Now you can start your Toolpad Studio application using one of the commands: | |||
npm run toolpad-studio:dev | |||
``` | |||
|
|||
```bash yarn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iterate on mui/mui-public#181.
|
||
<p class="description">Learn how to install Toolpad Core in your local environment.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong position
</codeblock> | ||
|
||
The Toolpad Core package has a peer dependency on `@mui/material` and `@mui/icons-material`. If you aren't using these already in your project, you can install them with: | ||
|
||
<codeblock storageKey="package-manager"> | ||
|
||
```bash npm | ||
npm install -S @mui/material @mui/icons-material @emotion/react @emotion/styled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the -S
. Isn't this the default https://docs.npmjs.com/cli/v10/commands/npm-install#save?
## Manual Installation | ||
<p class="description">Learn how to install Toolpad Core in your local environment.</p> | ||
|
||
## Manual installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentence case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make vale catch these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use https://vale.sh/docs/topics/styles/#capitalization. But two challenges:
- There are valid uses of title cases when we reference components, e.g. https://mui.com/toolpad/core/react-app-provider/#next-js-app-router seems right, it's how Next.js labels it. It can be annoying to disable Vale rules
<!-- vale Style.Rule = NO -->
. - Vale CI is broken, visible from [core] Fix Vale errors #4347.
## Dynamic Routes | ||
## Dynamic routes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bharatkashyap Similar to #4306, I fixed a couple more to match https://www.notion.so/mui-org/Writing-style-guide-2a957a4168a54d47b14bae026d06a24b.
cc @samuelsycamore we are progressively getting to match the style guide everywhere 😄
@@ -4,6 +4,6 @@ title: Toolpad Studio docs - Home | |||
|
|||
# Toolpad Studio | |||
|
|||
<p class="description">Toolpad Studio docs</p> | |||
<p class="description">Toolpad Studio docs.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can enforce this in docs-infra: mui/material-ui#44292.
I noticed this from #4306 that fix the broken image reported in https://app.ahrefs.com/site-audit/3992793/data-explorer?columns=pageRating%2Curl%2ChttpCode%2CcontentType%2CincomingImage¤t=01-11-2024T025516&filterId=c7bd3d737800c382ec57c0adace75595&issueId=c0e87084-feaf-11e8-8c05-001e67ed4657&sorting=-pageRating (great to see this fixed beforehand!)