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
[Joy] Add Link
component
#31175
[Joy] Add Link
component
#31175
Conversation
@mui/joy: parsed: +2.47% , gzip: +1.74% |
@hbjORbj Please reexport the component from |
packages/mui-joy/src/Link/Link.tsx
Outdated
margin: 0, // Remove the margin in Safari | ||
borderRadius: 0, | ||
padding: 0, // Remove the padding in Firefox |
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.
margin: 0, // Remove the margin in Safari | |
borderRadius: 0, | |
padding: 0, // Remove the padding in Firefox | |
margin: 0, // Remove the margin in Safari | |
borderRadius: 0, | |
padding: 0, // Remove the padding in Firefox | |
...(!!ownerState.variant && { | |
paddingInline: '0.25em', // better than left, right because it also works with writing mode. | |
marginInline: '-0.25em', | |
}), |
I think this is worth trying because the current variant looks so tight which I think anyone will customize it anyway.
Adding paddingInline
gives this result (hover on text variant):
Negative marginInline
is for making the content align with others by default (hover on text variant):
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.
From what I tested, 0.25em
works best.
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.
Agree. Resolved.
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.
👍 Other than my suggestion, the rest looks really good.
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.
@hbjORbj The focus style & cursor pointer in the preview does not work, can you check?
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.
Not sure why but the color
prop doesn't seem to be working?! At least in the preview demo.
@danilo-leal You are right. I just fixed it! |
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.
Please check the button
condition again.
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.
👍 Looks great!
I have followed (at least) the PR section of the contributing guide.
variant
:'text' | 'outlined' | 'light' | 'contained'
(undefined
by default)color
:'danger' | 'info' | 'neutral' | 'primary' | 'success' | 'warning'
(primary
by default)level
:'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' | 'body1' | 'body2' | 'body3'
(body1
by default)underline
:'none' | 'hover' | 'always'
(always
by default)Preview: https://deploy-preview-31175--material-ui.netlify.app/experiments/joy/link/