feat(accordion, accordion-item): add component tokens #8878
Conversation
| } | ||
|
|
||
| .content { | ||
| padding: var(--calcite-accordion-item-padding); | ||
| padding: var(--calcite-internal-accordion-item-space-inset); |
There was a problem hiding this comment.
Nitpick since this is internal, but shouldn't this be --calcite-internal-accordion-item-content-space?
Applies to other inset space CSS props.
| margin-inline-start: var(--calcite-accordion-item-icon-spacing-start); | ||
| margin-inline-end: var(--calcite-accordion-item-icon-spacing-end); | ||
| transform: rotate(var(--calcite-accordion-item-icon-rotation)); | ||
| margin-inline-start: var(--calcite-internal-accordion-item-icon-spacing-start); |
There was a problem hiding this comment.
Nitpick since this is internal, but for consistency, these should use space vs spacing.
Also, since these are used for inline spacing, would --calcite-internal-accordion-item-icon-space-x-start/end be more appropriate?
| & .heading { | ||
| @apply text-color-1; | ||
| color: var( | ||
| --calcite-accordion-item-heading-text-color, |
There was a problem hiding this comment.
Per our latest guidelines, should these be stateful CSS props?
component tokens should be added for the appropriate states as well. I.E. if background-color changes on hover then you should also add a
background-color-hovertoken.
Applies to other CSS props affected by state.
There was a problem hiding this comment.
because all parts of the component respond to the state of the host, we don't need state tokens here.
| * | ||
| * @prop --calcite-accordion-item-background-color: defines the background color for the component. | ||
| * @prop --calcite-accordion-item-border-color: defines the border color for the component. | ||
| * @prop --calcite-accordion-item-description-text-color: defines the text color of the description in the component. Falls back to `--calcite-accordion-item-text-color`. |
There was a problem hiding this comment.
For the time being, can we omit specifying which CSS prop it will fall back to? I think we could do so since it's an implementation detail.
| border-block-end-width: var(--calcite-border-width-none); | ||
| border-style: solid; | ||
|
|
||
| ::slotted(accordion-item) { |
There was a problem hiding this comment.
Do we need to specify this if accordion items have these CSS props defined with the same fallback values?
There was a problem hiding this comment.
nope! Thanks for the catch.
| * These properties can be overridden using the component's tag as selector. | ||
| * | ||
| * @prop --calcite-accordion-item-background-color: defines the background color for the component. | ||
| * @prop --calcite-accordion-item-border-color: defines the border color for the component. |
There was a problem hiding this comment.
--calcite-accordion-item-border-color doesn't seem to be used in accordion-item's styling.
There was a problem hiding this comment.
thanks for the catch!
|
|
||
| .accordion--transparent { | ||
| ::slotted(accordion-item) { | ||
| --calcite-accordion-item-border-color: var(--calcite-accordion-border-color, transparent); |
There was a problem hiding this comment.
We should add a note to our guidelines about composite component overrides to slotted components.
…components into astump/7180-accordion
Related Issue: #7180
Summary
Accordion
--calcite-accordion-background-color: defines the background color for the component and it's slotted accordion-items.
--calcite-accordion-border-color: defines the border color for the component and it's slotted accordion-items.
Accordion Item
--calcite-accordion-item-background-color: defines the background color for the component.
--calcite-accordion-item-border-color: defines the border color for the component.
--calcite-accordion-item-description-text-color: defines the text color of the description in the component. Falls back to
--calcite-accordion-item-text-color.--calcite-accordion-item-heading-text-color: defines the text color of the heading in the component. Falls back to
--calcite-accordion-item-text-color.--calcite-accordion-item-icon-color: defines the icon color for the component.
--calcite-accordion-item-expand-icon-color: defines the icon color for the component.
--calcite-accordion-item-text-color: defines the text color for the component.