feat(chip): add recipe and variables #30873
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Do we need Sass variables for this? I feel like it just adds another layer when the CSS variables would do
There was a problem hiding this comment.
I created it so we don't have such long variables cluttering the base file. Thoughts?
| }; | ||
|
|
||
| type Components = { | ||
| IonChip?: { |
There was a problem hiding this comment.
Do you think we should keep these interfaces in the component's folder to keep it better organized? Or is this something we might move later and combine between components?
core/src/themes/md/default.tokens.ts
Outdated
| paddingVertical: '6px', | ||
| paddingHorizontal: '12px', |
There was a problem hiding this comment.
| paddingVertical: '6px', | |
| paddingHorizontal: '12px', | |
| padding: { | |
| vertical: '6px', | |
| horizontal: '12px', | |
| } |
Would it not be cleaner to do it as objects?
There was a problem hiding this comment.
Looks good. We might want to move to top, end, start and bottom for all components for consistency but we can tackle that when we get there.
There was a problem hiding this comment.
A lot of these values are hardcoded - why aren't we using the design tokens like this:
--ion-scaling-xxs
--ion-scaling-xs
--ion-scaling-sm
etc
| }, | ||
|
|
||
| avatar: { | ||
| size: `${24 / fontSizes.chipBase}em`, |
There was a problem hiding this comment.
These numbers seem made up - maybe we need to define these calculations better?
| } | ||
|
|
||
| const fontSizes = { | ||
| chipBase: 14, |
There was a problem hiding this comment.
Why is the base 14 for Chip? Shouldn't all components have the same base size?
There was a problem hiding this comment.
My comments on the md file apply here as well
Co-authored-by: Brandy Smith <[email protected]>
Co-authored-by: Brandy Smith <[email protected]>
Co-authored-by: Brandy Smith <[email protected]>
Co-authored-by: Brandy Smith <[email protected]>
Issue number: resolves #
What is the current behavior?
What is the new behavior?
Does this introduce a breaking change?
Other information