[docs] Move more prop docs into IntelliSense #21002
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Details of bundle changes.Comparing: 4e12b95...07f7327 Details of page changes
|
eps1lon
commented
May 12, 2020
| @@ -470,10 +474,9 @@ ButtonBase.propTypes = { | |||
| */ | |||
| TouchRippleProps: PropTypes.object, | |||
| /** | |||
| * Used to control the button's purpose. | |||
| * This prop passes the value to the `type` attribute of the native button component. | |||
| * @ignore | |||
There was a problem hiding this comment.
With our types it would require more work to get JSDOC working. type is only considered when component="button". It has different types depending on the component so we can't just slap it into props with type: React.ButtonHTMLAttributes['type'].
It was also too narrow previously. I'd say this is fine since the documentation value is pretty low. I doubt this is only used after reading about it here.
eps1lon
commented
May 13, 2020
| | <span class="prop-name">classes</span> | <span class="prop-type">object</span> | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. | | ||
| | <span class="prop-name">component</span> | <span class="prop-type">elementType</span> | <span class="prop-default">'nav'</span> | The component used for the root node. Either a string to use a HTML element or a component. By default, it maps the variant to a good default headline component. | |
There was a problem hiding this comment.
Would require a lot of type gymnastics to get this back as far as I can tell. I don't think this documentation adds anything because it's implicit: Obviously we want to choose a "good default". The question is what this default is and we have that already answered in the default column. Then you ask: "But why this default value?" and "Because it's good" is not a meaningful answer.
eps1lon
commented
May 13, 2020
| /** | ||
| * The icon element. | ||
| */ | ||
| icon?: React.ReactNode; |
There was a problem hiding this comment.
To sync with propTypes. I don't see a reason to disallow arrays or booleans from conditional rendering.
eps1lon
commented
May 13, 2020
| /** | ||
| * Element placed before the children. | ||
| */ | ||
| startIcon: PropTypes.node, | ||
| /** | ||
| * @ignore | ||
| */ | ||
| type: PropTypes.string, | ||
| type: PropTypes.oneOfType([PropTypes.oneOf(['button', 'reset', 'submit']), PropTypes.string]), |
There was a problem hiding this comment.
Some redundant checks due to intersecting button#type with a#type. But we don't document it explicitly anyway so this seems ok-ish.
There was a problem hiding this comment.
Also: This is actually a fix since it was too narrow when you used the component prop e.g. <Button component="a" type="some/mimetype" /> or <Button component={MyButtonHandlingArbitraryTypes} type="arbitrary" /> would issue false positive runtime warnings.
eps1lon
commented
May 13, 2020
| @@ -37,7 +37,7 @@ CardContent.propTypes = { | |||
| * The component used for the root node. | |||
| * Either a string to use a HTML element or a component. | |||
| */ | |||
| component: PropTypes.elementType, | |||
| component: PropTypes /* @typescript-to-proptypes-ignore */.elementType, | |||
There was a problem hiding this comment.
Proptypes arent't generated yet. It's just in preparation of future work and was easy to find&replace.
github-actions
bot
added
the
PR: out-of-date
eps1lon
removed
the
PR: out-of-date
2 tasks
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not doing all components in one go. We already had some regression in the previous PRs. Easy to make mistakes if you're mindlessly c&p JSDOC.
generateProptypesnow has a list of unfinished components.TODO:
[ ] per-component documentation forSee [docs] Move more prop docs into IntelliSense #21002 (comment)component