-
Notifications
You must be signed in to change notification settings - Fork 77
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(rating): add component tokens #11150
Conversation
@@ -4,6 +4,11 @@ | |||
* These properties can be overridden using the component's tag as selector. | |||
* | |||
* @prop --calcite-rating-spacing-unit: The amount of left and right margin spacing between each rating star. | |||
* @prop --calcite-rating-hover-selected-color: Specifies the component's color when hovered and selected. |
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 think the order of this should be --calcite-rating-selected-color-hover
right?
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 updated this to --calcite-rating-selected-color-hover
@@ -4,6 +4,11 @@ | |||
* These properties can be overridden using the component's tag as selector. | |||
* | |||
* @prop --calcite-rating-spacing-unit: The amount of left and right margin spacing between each rating star. | |||
* @prop --calcite-rating-hover-selected-color: Specifies the component's color when hovered and selected. | |||
* @prop --calcite-rating-idle-unselected-color: Specifies the component's color when idle and unselected. |
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.
Can this just be --calcite-rating-color
? Or does it need to specify idle and unselected?
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 updated this token to --calcite-rating-color
.
targetProp: "color", | ||
}, | ||
}); | ||
}); |
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.
Can you add tests for the other vars?
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 added tests for the other tokens. I'm having issues testing --calcite-rating-spacing
, could you please take a look?
cc @driskull @alisonailea
@@ -3,7 +3,12 @@ | |||
* | |||
* These properties can be overridden using the component's tag as selector. | |||
* | |||
* @prop --calcite-rating-spacing-unit: The amount of left and right margin spacing between each rating star. | |||
* @prop --calcite-rating-spacing-unit: Specifies the amount of left and right margin spacing between each item. |
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.
deprecate in favor of --calcite-rating-spacing
@@ -3,7 +3,12 @@ | |||
* | |||
* These properties can be overridden using the component's tag as selector. | |||
* | |||
* @prop --calcite-rating-spacing-unit: The amount of left and right margin spacing between each rating star. | |||
* @prop --calcite-rating-spacing-unit: Specifies the amount of left and right margin spacing between each item. | |||
* @prop --calcite-rating-selected-color-hover: Specifies the component's item color when hovered or selected. |
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.
--calcite-rating-color-hover
@@ -84,13 +89,14 @@ calcite-chip { | |||
|
|||
.number--average { | |||
font-weight: bold; | |||
color: var(--calcite-rating-average-text-color); |
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.
what is the purpose of this token?
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.
@@ -15,17 +20,17 @@ | |||
|
|||
:host([scale="s"]) { | |||
@apply h-6; | |||
--calcite-rating-spacing-unit: theme("spacing.1"); | |||
--spacing: theme("spacing.1"); |
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.
tokens should not be assigned in the component unless they are tagged with -internal-
} | ||
|
||
:host([scale="m"]) { | ||
@apply h-8; | ||
--calcite-rating-spacing-unit: theme("spacing.2"); | ||
--spacing: theme("spacing.2"); |
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.
tokens should not be assigned in the component unless they are tagged with -internal-
} | ||
|
||
:host([scale="l"]) { | ||
@apply h-11; | ||
--calcite-rating-spacing-unit: theme("spacing.3"); | ||
--spacing: theme("spacing.3"); |
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.
tokens should not be assigned in the component unless they are tagged with -internal-
@ashetland @SkyeSeitz Can you please take a look at the snapshots? |
Lookin' good. Normal false positives. |
…ciado88/7180-add-rating-design-tokens
…ciado88/7180-add-rating-design-tokens
Related Issue: #7180
Summary
Add
rating
component tokens.--calcite-rating-spacing
: Specifies the amount of left and right margin spacing between each item.--calcite-rating-color-hover
: Specifies the component's item color when hovered or selected.--calcite-rating-color
: Specifies the component's item color.--calcite-rating-average-color
: Specifies the component's item color when average is set.--calcite-rating-average-text-color
: Specifies the component's average text color.--calcite-rating-count-text-color
: Specifies the component's count text color.