Migrate Kyper AttentionFilled Icon to MXUI - #356
Conversation
| fill={true} | ||
| name="error" | ||
| size={16} | ||
| sx={{ color: '#E32727', marginRight: tokens.Spacing.Tiny }} |
There was a problem hiding this comment.
Let's not hardcode #E32727 here. We should be using tokens.
| size={16} | ||
| sx={{ color: '#E32727', marginRight: tokens.Spacing.Tiny }} | ||
| /> | ||
| <Text color="#E32727" sx={{ fontSize: '12px' }} variant="XSmall"> |
There was a problem hiding this comment.
Same here. let's use tokens instead of raw values.
| fill={true} | ||
| name="error" | ||
| size={32} | ||
| sx={{ color: '#4D4D4D', marginBottom: tokens.Spacing.Large }} |
There was a problem hiding this comment.
same issue here. we should be using color tokens rather than raw color values.
| fill={true} | ||
| name="error" | ||
| size={32} | ||
| sx={{ color: '#4D4D4D', marginBottom: tokens.Spacing.Large }} |
There was a problem hiding this comment.
Most of the time I think using Stack spacing={?} is better than adding margins to create space between things.
| fill={true} | ||
| name="error" | ||
| size={16} | ||
| sx={{ color: '#E32727', marginRight: tokens.Spacing.Tiny }} |
There was a problem hiding this comment.
Same spacing comment here
| {isSubmitted && !selectedReason && ( | ||
| <section role="alert" style={styles.errorContent}> | ||
| <AttentionFilled color={tokens.Color.Error300} /> | ||
| <Icon fill={true} name="error" size={16} sx={{ color: tokens.Color.Error300 }} /> |
There was a problem hiding this comment.
color prop here as well
There was a problem hiding this comment.
@platypus801 The token value here would be error.300 right?
| fill={true} | ||
| name="error" | ||
| size={48} | ||
| sx={{ color: tokens.TextColor.Default, marginBottom: tokens.Spacing.XLarge }} |
There was a problem hiding this comment.
Color prop and stack
| fill={true} | ||
| name="error" | ||
| size={24} | ||
| sx={{ color: tokens.BackgroundColor.MessageBoxError }} |
There was a problem hiding this comment.
Given that we're trying to get rid of kyper we shouldn't use kyper tokens
There was a problem hiding this comment.
@platypus801 tokens.BackgroundColor.MessageBoxError is #DF320C. What would that translate to in MUI?
| {isSubmitted && _isEmpty(selectedOption) && ( | ||
| <section role="alert" style={styles.errorContent}> | ||
| <AttentionFilled color={tokens.Color.Error300} /> | ||
| <Icon fill={true} name="error" size={16} sx={{ color: tokens.Color.Error300 }} /> |
| {isSubmitted && _isEmpty(selectedOption) && ( | ||
| <section role="alert" style={styles.errorContent}> | ||
| <AttentionFilled color={tokens.Color.Error300} /> | ||
| <Icon fill={true} name="error" size={16} sx={{ color: tokens.Color.Error300 }} /> |
| <div style={styles.container}> | ||
| <div style={styles.iconContainer}> | ||
| <AttentionFilled color={tokens.Color.NeutralWhite} size={24} /> | ||
| <Icon fill={true} name="error" size={24} sx={{ color: tokens.Color.NeutralWhite }} /> |
There was a problem hiding this comment.
Because we are moving towards css modules we should use our css variables. Unfortuntely we don't get these until we migrate to MXUI v2.
@ash-wright123 and I came up with a strategy to define these global styles somewhere, and then delete that file when we move to MXUI v2. We are only planning to define the variables that we actually use for now.
You can find the list of variables here: https://mxui.kube.qa.internal.mx/?path=/story/mui-components-toggle-button--group
There was a problem hiding this comment.
The link you shared is taking me to the Toggle button group component. Where are those variables defined? In MXUI or somewhere else?
There was a problem hiding this comment.
Also, I'm curious whether those variables match what MXUI v2 has.
| fill={true} | ||
| name="error" | ||
| size={48} | ||
| sx={{ color: tokens.TextColor.Default, marginBottom: tokens.Spacing.XLarge }} |
There was a problem hiding this comment.
i believe icons have a color prop so we don't need to use sx. https://mxui.kube.qa.internal.mx/?path=/story/mxui-components-icon--icons. i believe it uses inherit by default to match the text it is with.
3 participants
Before(Kyper)
After(MXUI)