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] Remove ` around @default values #12158

Merged
merged 5 commits into from Apr 3, 2024

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Feb 21, 2024

We have the two way of defining default that exist:

@default `false`
@default false

The docs currently support the second one. Otherwise you get the following rendering in API pages

image

https://mui.com/x/api/date-pickers/multi-input-date-range-field/#multi-input-date-range-field-prop-shouldRespectLeadingZeros

Better to wait for #41069 to avoid issues with the MD file generation used by data grid interfaces

To post in slack when merging

Hi, Just a head up about docs API
The @default JSDocs already expect some code. So it's unnecessary to add ` around the default value. Otherwise the ` will be rendered in the API
image

@alexfauquette alexfauquette added the docs Improvements or additions to the documentation label Feb 21, 2024
@mui-bot
Copy link

mui-bot commented Feb 21, 2024

Deploy preview: https://deploy-preview-12158--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against d8b6ec1

@alexfauquette alexfauquette changed the base branch from next to master April 2, 2024 12:55
@alexfauquette alexfauquette marked this pull request as ready for review April 2, 2024 13:00
Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

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

Do you think we could even throw during the build step if a default starts and ends with the ` char?

@alexfauquette
Copy link
Member Author

Do you think we could even throw during the build step if a default starts and ends with the ` char?

The issue is that it can make sense, for example * @default `${adapter.formats.month} ${adapter.formats.year}`

@flaviendelangle
Copy link
Member

"start with , ends with and do not contain any ${} pattern" 😆

The idea is just that if we can catch most bad usages of `, even if it's not 100%, it will make is easier to keep consistency
But feel free to ignore my comment if you think it's not worth it / if it adds to much complexity

@alexfauquette
Copy link
Member Author

@flaviendelangle I gave it a try by logging warning in the docs:api but it's lost with all the other lines displayed in the console. And I do't feel motivated enough to start creating an eslint rule :)

@alexfauquette alexfauquette merged commit 3d86411 into mui:master Apr 3, 2024
17 checks passed
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.

None yet

3 participants