-
-
Notifications
You must be signed in to change notification settings - Fork 50
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-infra] Tweak buildReference
script
#1062
Conversation
Netlify deploy preview |
98dd7d5
to
b7a0518
Compare
6b009a3
to
d0e455c
Compare
buildReference
script and fix select docs
@@ -70,7 +70,7 @@ export async function buildReference() { | |||
const descriptionData: PropsTranslations = JSON.parse(descriptionJsonContents); | |||
|
|||
for (const prop in props) { | |||
props[prop].description = descriptionData.propDescriptions[prop]?.description; | |||
props[prop].description ??= descriptionData.propDescriptions[prop]?.description; |
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.
Fixes when a description
is specified in overrides/*.json
but not reflected in the generated one
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.
(Probably only happens to something that is not literally an "override" like children
)
@@ -93,7 +93,7 @@ export async function buildReference() { | |||
const json: ComponentDef = { | |||
name: componentData.name, | |||
description: descriptionData.componentDescription, | |||
props, | |||
props: Object.fromEntries(Object.entries(props).sort()), |
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.
If there are "overrides" the props would not be sorted alphabetically, e.g. https://master--base-ui.netlify.app/react/components/radio#root
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.
Would be good to leave this as a comment in code
buildReference
script and fix select docsbuildReference
script and fix select docs
buildReference
script and fix select docsbuildReference
script
"props": { | ||
"children": { | ||
"type": "React.ReactElement | (value) => React.ReactElement", | ||
"description": "Allows you to customize the rendering of the text.\n\nAccepts a `ReactElement` or a function that returns the element to render." |
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.
@vladmoroz If you're ok with this description I'll use the same for Slider.Value
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.
I’d say:
Override children if you need to customize how the value is displayed.
By default, renders the current value as is.
Accepts a `ReactElement` or a function that returns the element to render.
Also, shouldn't the children allow React.ReactNode
instead of React.ReactElement
? So then we'd say
Accepts a `ReactNode` or a function that returns the node to render.
fc4f504
to
938d581
Compare
Fixes #1061
Also fixes #990 (comment)