Skip to content
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(dropdown): add component tokens #11465

Merged
merged 6 commits into from
Feb 12, 2025
Merged

feat(dropdown): add component tokens #11465

merged 6 commits into from
Feb 12, 2025

Conversation

alisonailea
Copy link
Contributor

Related Issue: #7180

Summary

Add Component Tokens

Dropdown

--calcite-dropdown-background-color

Dropdown Group

--calcite-dropdown-group-border-color: Specifies the color of the dropdown's border.
--calcite-dropdown-group-title-text-color: Specifies the color of the dropdown's title.

Dropdown Item

--calcite-dropdown-item-background-color-hover: Specifies the background color of the dropdown item when hovered.
--calcite-dropdown-item-background-color-press: Specifies the background color of the dropdown item when selected or active.
--calcite-dropdown-item-icon-color-hover: Specifies the color of the dropdown item icon when hovered.
--calcite-dropdown-item-icon-color-press: Specifies the color of the dropdown item icons when selected or active.
--calcite-dropdown-item-text-color-press: Specifies the color of the dropdown item text when selected or active.
--calcite-dropdown-item-text-color: Specifies the color of the dropdown item text.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Feb 5, 2025
*
* @prop --calcite-dropdown-item-background-color-hover: Specifies the background color of the dropdown item when hovered.
* @prop --calcite-dropdown-item-background-color-press: Specifies the background color of the dropdown item when selected or active.
* @prop --calcite-dropdown-item-icon-color-hover: Specifies the color of the dropdown item icon when hovered.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: It does seem like having a "base" for text-color but not icon-color here is a bit unexpected if there is a hover state for icon-color. These would be the icon-start/end right?

There could be a need to have those be distinct and independent from other overrides like the selection icon color, but we can add as requested!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but I think we should wait for our customers to tell us.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While testing the CSS variables/tokens I'm also observing the same. This seems odd, and when I read the CSS variable I assumed it was related to the icon-start and icon-end props. We do this for other components, such as tab-title, maybe we need to also accommodate for dropdown-item for consistency.

Copy link
Contributor

@macandcheese macandcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some small comments!

@@ -135,13 +136,13 @@ export class DropdownGroup extends LitElement {

override render(): JsxNode {
const groupTitle = this.groupTitle ? (
<span ariaHidden="true" class="dropdown-title">
<span ariaHidden="true" class={CSS.title}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹

…mp/dropdown

# Conflicts:
#	packages/calcite-components/src/components/dropdown/dropdown.e2e.ts
#	packages/calcite-components/src/custom-theme.stories.ts
@alisonailea alisonailea added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Feb 10, 2025
@alisonailea alisonailea added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Feb 12, 2025
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 💯

@alisonailea alisonailea merged commit e6d405b into dev Feb 12, 2025
14 checks passed
@alisonailea alisonailea deleted the astump/7180-dropdown branch February 12, 2025 17:34
@github-actions github-actions bot added this to the 2025-02-25 - Feb Milestone milestone Feb 12, 2025
* @prop --calcite-dropdown-item-background-color-press: Specifies the background color of the dropdown item when selected or active.
* @prop --calcite-dropdown-item-icon-color-hover: Specifies the color of the dropdown item icon when hovered.
* @prop --calcite-dropdown-item-icon-color-press: Specifies the color of the dropdown item icons when selected or active.
* @prop --calcite-dropdown-item-text-color-press: Specifies the color of the dropdown item text when selected or active.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alisonailea It seems this also pertains to on hover, is that expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants