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] Detailed type definitions #818

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

vladmoroz
Copy link
Contributor

@vladmoroz vladmoroz commented Nov 13, 2024

Link
https://deploy-preview-818--base-ui.netlify.app/new/components/dialog

What's up

  • Fully-fledged, customisable type definitions in the API reference
  • Data attribute tables
  • Improved inline code highlighting
  • Much cleaner prop definitions JSON

How it works

  • Temporary prop definitions and translations are created using @mui-internal/api-docs-builder
  • Type data and descriptions are pieced together into a single file per component. Unnecessary fields aren't included.
  • Type overrides are looked up in docs/reference/overrides
    • Overrides common to many components are stored in docs/reference/overrides/common.json
    • Per-component overrides are stored in the corresponding files in that folder.
  • Temporary data is deleted, new JSON is written into docs/reference/generated

docs/data folder is still in place and is used by the existing setup, but will be removed once we are finished.

Things that we will need to add to overrides pretty much all the time:

  • Custom function types, because the original types are no good
  • Data attributes documentation

Screenshot

image

If you are looking at the screenshot and wondering about nicer formatting for multiline union types, that's coming in a future PR.

@vladmoroz vladmoroz added the docs Improvements or additions to the documentation label Nov 13, 2024
@@ -0,0 +1,14 @@
{
"name": "AccordionHeader",
"description": "",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Empty descriptions throw warnings during the API generation

@mui-bot
Copy link

mui-bot commented Nov 13, 2024

Netlify deploy preview

https://deploy-preview-818--base-ui.netlify.app/

Generated by 🚫 dangerJS against 3fa72d3

@@ -24,6 +24,7 @@ const customStyleHookMapping: CustomStyleHookMapping<DialogBackdrop.OwnerState>
};

/**
* An overlay displayed beneath the popup. Renders a `<div>` element.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we like this and the other descriptions I added in the PR @colmtuite @samuelsycamore?

I plan to add/rework the descriptions for all the components in this style. (Most are missing).

Copy link
Contributor

Choose a reason for hiding this comment

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

I left a note on one but in general I'm happy with the descriptions and their style. 🤝

Comment on lines +1 to +42
{
"props": {
"anchor": {
"type": "React.Ref | Element | VirtualElement | (() => Element | VirtualElement | null) | null"
},
"className": {
"type": "string | (state) => string"
},
"collisionBoundary": {
"type": "'clippingAncestors' | Element | Element[] | Rect"
},
"collisionPadding": {
"type": "number | Rect"
},
"container": {
"type": "React.Ref | HTMLElement | null"
},
"initialFocus": {
"type": "React.Ref | (interactionType => HTMLElement | null)"
},
"inputRef": {
"type": "React.Ref"
},
"onClick": {
"type": "(event) => void"
},
"render": {
"type": "React.ReactElement | (props, state) => React.ReactElement"
}
},
"types": {
"bool": "boolean",
"func": "function",
"node": "React.ReactNode",
"ref": "React.Ref",
"Array<number>": "number[]",
"Array<string>": "string[]",
"'bottom' | 'left' | 'right' | 'top'": "'top' | 'bottom' | 'left' | 'right'",
"'center' | 'end' | 'start'": "'start' | 'center' | 'end'",
"'both' | 'none' | 'x' | 'y'": "'none' | 'x' | 'y' | 'both'"
}
}
Copy link
Contributor Author

@vladmoroz vladmoroz Nov 13, 2024

Choose a reason for hiding this comment

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

Point of interest: these are type definition overrides across all components. E.g. all render prop definitions get this one; all bool types are transformed to boolean, etc.

Comment on lines 1 to 17
{
"name": "DialogPopup",
"attributes": {
"[data-open]": {
"description": "Present when the dialog is open."
},
"[data-closed]": {
"description": "Present when the dialog is closed."
},
"[data-entering]": {
"description": "Present when the dialog is animating in."
},
"[data-exiting]": {
"description": "Present when the dialog is animating out."
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Point of interest: this is how data attributes are documented

Comment on lines +1 to +11
{
"name": "NumberFieldRoot",
"props": {
"format": {
"type": "Intl.NumberFormatOptions"
},
"onValueChange": {
"type": "(value, event) => void"
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Point of interest: this is how generated type definitions are overridden when they are no good

@@ -28,6 +28,7 @@ const customStyleHookMapping: CustomStyleHookMapping<DialogPopup.OwnerState> = {
};

/**
* A component that groups the dialog contents. Renders a `<div>` element.
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this a little confusing when comparing with the description in DialogRoot.tsx below—I think I know the difference given that I have insider context, but for a newer user it might be hard to figure out. Is there a way to rewrite one or the other for clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right, I didn't notice that both are saying that they group things.

Does this feel good for the Popup part?

Suggested change
* A component that groups the dialog contents. Renders a `<div>` element.
* A container for the dialog contents. Renders a `<div>` element.

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