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
[Link] Refinement #14162
[Link] Refinement #14162
Conversation
96916ad
to
33d4056
Compare
@@ -206,6 +206,7 @@ class AppFrame extends React.Component { | |||
color="inherit" | |||
aria-label="Edit docs colors" | |||
component={Link} | |||
naked |
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.
* The target of the link. You can read about this property on the | ||
* [MDN Web Docs](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#Attributes) | ||
*/ | ||
target: PropTypes.oneOf(['_self', '_blank', '_parent', '_top']), |
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.
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.
Didn't we decide anyway that we don't validate DOM attributes?
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.
What do you mean by we don't validate DOM attributes? Oh yeah. We don't need to document it. It's making the component lighter :).
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.
What I mean by this is that we don't validate props that we simply forward.
target
is valid DOM attribute for a
elements just not button
. So the validation was correct for the default case but not if component="button"
was used. The validator mentioned this explicitly "Attribute target
not allowed on element button
at this point.".
Oh and we need to remove the default value for target
: https://github.com/mui-org/material-ui/blob/dfff366df7b83b26b2ab6cc169e51184742d043b/packages/material-ui/src/Link/Link.js#L143
I think that we can announce the release of this component in the next v3.9.0 release.