-
Notifications
You must be signed in to change notification settings - Fork 47
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
🚀 feat(stepper): add custom color for stepper #672
Conversation
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
return ( | ||
<Wrapper> | ||
{labels.map((label, index) => ( | ||
<DotWrapper key={label}> |
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 know it's rare, but assuming labels are always unique may be a problem, so i would review the component key here.
<DotWrapper key={label}> | |
<DotWrapper key={`${index}_$({label}`}> |
/** Must be a color from yoga colors. */ | ||
secondary: bool, |
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.
Exactly as described in the prop comment, i think this should be a yoga color string, so the component is more flexible :)
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 was talking with @matheushdsbr about this option, the initial approach was to receive a string that could be any yoga color, but after we talk about we decided to keep the component more unflexible. What do you think about it @matheushdsbr ?
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
b0a37a3
to
389e445
Compare
`, | ||
return css` | ||
width: 95px; | ||
margin-top: 10px; |
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.
Codacy has a fix for the issue: Delete ··
margin-top: 10px; | |
margin-top: 10px; |
font-size: ${stepper.label.font.size}px; | ||
text-align: center; | ||
`, | ||
return css` |
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.
Codacy has a fix for the issue: Delete ··
return css` | |
return css` |
return css` | ||
width: 95px; | ||
margin-top: 10px; | ||
margin-left: -40px; |
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.
Codacy has a fix for the issue: Delete ··
margin-left: -40px; | |
margin-left: -40px; |
color: ${active | ||
? stepper.label.color[state] | ||
: stepper.label.color.inactive}; | ||
font-size: ${stepper.label.font.size}px; |
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.
Codacy has a fix for the issue: Delete ··
font-size: ${stepper.label.font.size}px; | |
font-size: ${stepper.label.font.size}px; |
width: 95px; | ||
margin-top: 10px; | ||
margin-left: -40px; | ||
color: ${active |
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.
Codacy has a fix for the issue: Delete ··
color: ${active | |
color: ${active |
margin-left: -40px; | ||
color: ${active | ||
? stepper.label.color[state] | ||
: stepper.label.color.inactive}; |
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.
Codacy has a fix for the issue: Replace ········
with ······
: stepper.label.color.inactive}; | |
: stepper.label.color.inactive}; |
); | ||
|
||
const Label = styled(Text.Bold)( | ||
({ | ||
active, | ||
secondary, |
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.
Codacy has a fix for the issue: Insert ··
secondary, | |
secondary, |
width: 95px; | ||
margin-top: 10px; | ||
margin-left: -40px; | ||
}) => { |
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.
Codacy has a fix for the issue: Insert ··
}) => { | |
}) => { |
margin-top: 10px; | ||
margin-left: -40px; | ||
color: ${active | ||
? stepper.label.color[state] |
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.
Codacy has a fix for the issue: Replace ······
with ········
? stepper.label.color[state] | |
? stepper.label.color[state] |
margin-top: 10px; | ||
margin-left: -40px; | ||
color: ${active | ||
? stepper.label.color[state] |
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.
Codacy has a fix for the issue: Delete ··
? stepper.label.color[state] | |
? stepper.label.color[state] |
margin-left: -40px; | ||
color: ${active | ||
? stepper.label.color[state] | ||
: stepper.label.color.inactive}; |
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.
Codacy has a fix for the issue: Insert ··
: stepper.label.color.inactive}; | |
: stepper.label.color.inactive}; |
? stepper.label.color[state] | ||
: stepper.label.color.inactive}; | ||
font-size: ${stepper.label.font.size}px; | ||
text-align: center; |
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.
Codacy has a fix for the issue: Delete ··
text-align: center; | |
text-align: center; |
width: 95px; | ||
margin-top: 10px; | ||
margin-left: -40px; | ||
}) => { |
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.
Codacy has a fix for the issue: Delete ··
}) => { | |
}) => { |
text-align: center; | ||
`, | ||
return css` | ||
width: 95px; |
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.
Codacy has a fix for the issue: Delete ··
width: 95px; | |
width: 95px; |
: stepper.label.color.inactive}; | ||
font-size: ${stepper.label.font.size}px; | ||
text-align: center; | ||
`; |
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.
Codacy has a fix for the issue: Replace ····
with ··
`; | |
`; |
Description 📄
We add a prop for Stepper receive a custom color.
Platforms 📲
Type of change 🔍
How Has This Been Tested? 🧪
use a stepper component with color prop.
Unit Test
Snapshot Test
Checklist: 🔍
Screenshots 📸