feat(menu-item): add component tokens #10654
Conversation
github-actions
bot
added
the
enhancement
|
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
github-actions
bot
added
the
Stale
anveshmekala
removed
the
Stale
|
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
github-actions
bot
added
the
Stale
anveshmekala
removed
the
Stale
anveshmekala
changed the title
|
|
||
| &:hover, | ||
| &:focus { | ||
| background-color: var(--calcite-menu-background-color-hover, var(--calcite-color-foreground-2)); |
There was a problem hiding this comment.
because these are on :host they should not have a state. Remove -hover
There was a problem hiding this comment.
Good catch, i had state tokens earlier assuming menu-item & action can have two different background colors.
| &:hover { | ||
| @apply text-color-2 border-b-color-2; | ||
| border-block-end-color: var(--calcite-menu-item-accent-color-hover, var(--calcite-color-border-2)); |
There was a problem hiding this comment.
same here. Remove the state from all tokens on :host
There was a problem hiding this comment.
Q: Do we need to add a borderBlockEndColor when hovered ? Currently, navigation components do not add any border when hovered.
There was a problem hiding this comment.
I think that makes sense, I think calcite-menu-item:hover { --calcite-menu-item-accent-color: red } could be a way to set that if desired for some reason, though correct the pattern doesn't currently have that, so we don't need a standalone state token.
| &:active { | ||
| @apply bg-foreground-3 text-color-1; | ||
| background-color: var(--calcite-menu-background-color-press, var(--calcite-color-foreground-3)); |
There was a problem hiding this comment.
and here. Remove the state from all tokens on :host
| :host { | ||
| @apply flex; | ||
| } | ||
|
|
||
| ul { | ||
| @apply inline-flex items-center h-full m-0 p-0; | ||
| background-color: var(--calcite-menu-background-color, var(--calcite-color-transparent)); |
There was a problem hiding this comment.
I don't think this token is necessary here. The background color is covered by the menu item
anveshmekala
changed the title
anveshmekala
added
the
pr ready for visual snapshots
anveshmekala
added
pr ready for visual snapshots
anveshmekala
added
pr ready for visual snapshots
anveshmekala
added
pr ready for visual snapshots
anveshmekala
added
pr ready for visual snapshots
github-actions
bot
added this to the 2025-01-30 - 3.0.0 Jan Breaking Change Release milestone
Related Issue: #7180
Summary
Add the following component-scoped CSS Variables to calcite-menu-item:
--calcite-menu-item-accent-color: Specifies the border color of the component whenactive.--calcite-menu-item-accent-color-hover: Specifies the border color of the component when hovered.--calcite-menu-background-color: Specifies the background color of the component.--calcite-menu-item-sub-menu-border-color: Specifies the border color of sub-menu.--calcite-menu-item-sub-menu-corner-radius: Specifies the border radius of sub-menu.--calcite-menu-text-color: Specifies the text color of the component.