-
Notifications
You must be signed in to change notification settings - Fork 13.4k
feat(chip): add recipe and variables #30873
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
base: ionic-modular
Are you sure you want to change the base?
Changes from all commits
e729dea
0d6bd9d
ade5c05
0349ff7
ee81a8a
c6dfb10
e795c61
39bfce5
b67f4f5
9ecb103
7040091
098cba4
55b610a
918582b
3e5236c
b90fe18
2f71c0a
32c7471
da0a9a6
28d61aa
d616dee
2c9b1f1
d830d48
68a19fd
fdaafcd
c07ef36
b419a9c
3111d64
0db9f57
ef79c8d
626bace
92edc69
beaf349
4dd7e23
61dce8e
e36b9bb
94b9288
98bf385
401ef1f
683093d
ee18ef3
cf6a2fd
c680ee2
95aeea4
6ae28ab
fd0f67f
66fd3f3
ce86297
85989fc
f425a07
d927c57
a710962
83c948e
81ec32f
36301c8
67b6342
f2bf154
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should consider removing this test from avatar and having it in chip instead.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can make a note to do that in |
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These changes for
|
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,153 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export type IonChip = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we call this something else? Some ideas:
Suggested change
Suggested change
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| cursor?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This does not seem like it should be configurable. At the base level the component should behave the same for all themes. Any overrides to this value can be made at the app level. |
||||||||||||||||||||||||||||||||||||||||||||||||||
| margin: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| fontWeight?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| gap?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gap seems like a weird property to define here - shouldn't this be determined by spacing somehow? |
||||||||||||||||||||||||||||||||||||||||||||||||||
| lineHeight?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| letterSpacing?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| padding?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| vertical: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| horizontal: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+12
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+13
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the values like this - padding, margin, etc. need to be made in a shared component interface to make sure they're consistent among all components. We are going to run into issues with not using ionic-framework/core/src/components/button/button.scss Lines 33 to 36 in 442e3e9
If we keep these in a common interface then we can make sure components are using similar recipes. Something like: export type IonMargin = {
margin?: {
top: string | number;
end: string | number;
bottom: string | number;
start: string | number;
};
}
export type IonPadding = {
padding?: {
top: string | number;
end: string | number;
bottom: string | number;
start: string | number;
};
} |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Sizes | ||||||||||||||||||||||||||||||||||||||||||||||||||
| size: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| small: IonChipSizeDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| large: IonChipSizeDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+19
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // States | ||||||||||||||||||||||||||||||||||||||||||||||||||
| state?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| disabled?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| opacity: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| focus?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ring?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| color: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| width?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Shapes | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shape: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| soft: IonChipShapeDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| round: IonChipShapeDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| rectangular: IonChipShapeDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Hues | ||||||||||||||||||||||||||||||||||||||||||||||||||
| hue: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bold: IonChipHueDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subtle: IonChipHueDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+46
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Variants | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| variant?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| outline: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| border: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| color?: HueRef; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| width?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Any of the semantic colors like primary, secondary, etc. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| semantic?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: HueRef; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| border?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| color?: HueRef; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| state?: IonChipVariantState; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| icon: IonChipIconDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| avatar?: IonChipMediaDefinition; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipSizeDefinition = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| minHeight: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| font: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| size: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have
Comment on lines
+79
to
+80
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipShapeDefinition = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| border: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| radius: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to determine a standard for when to do this vs
Comment on lines
+85
to
+86
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipHueDefinition = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| color: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+91
to
+92
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| semantic?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| color?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| state?: IonChipInteractionStates; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| state?: IonChipInteractionStates; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipInteractionStates = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| focus?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| hover?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| activated?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipVariantState = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this when it matches |
||||||||||||||||||||||||||||||||||||||||||||||||||
| focus?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: HueRef; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| hover?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: HueRef; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| activated?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bg?: HueRef; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type HueRef = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bold?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subtle?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipMediaDefinition = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| size: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Styles for the media component only if it is the first element in the slot | ||||||||||||||||||||||||||||||||||||||||||||||||||
| firstChild?: IonChipMediaMargin; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about if we use export type IonMargin = {
margin?: {
top?: string | number;
end?: string | number;
bottom?: string | number;
start?: string | number;
};
};
export type IonChipMediaTheme = {
leading?: IonMargin;
trailing?: IonMargin;
}; |
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Styles for the media component only if it is the last element in the slot | ||||||||||||||||||||||||||||||||||||||||||||||||||
| lastChild?: IonChipMediaMargin; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipMediaMargin = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not be defined - margin should always have the same base interface as I mentioned previously. |
||||||||||||||||||||||||||||||||||||||||||||||||||
| margin?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| vertical?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| start?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| end?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type IonChipIconDefinition = IonChipMediaDefinition & { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| color?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
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.
These changes for
iosare happening becauseroundinstead ofsoft.subtleinstead ofbold.