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(action menu): add component tokens #8837

Merged
merged 6 commits into from
Feb 28, 2024

Conversation

alisonailea
Copy link
Contributor

Related Issue: #7180

Summary

Adds Action-Menu component tokens and sub-component tokens to Calcite Components

Action Group

--calcite-action-group-action-menu-border-color: The border color of the sub-component.
--calcite-action-group-action-menu-text-color: The text color of the sub-component.

Action Menu

--calcite-action-menu-border-color: The border color of the component.
--calcite-action-menu-text-color: The text color of the component.

Block

--calcite-block-action-menu-border-color: The border color of the sub-component.
--calcite-block-action-menu-text-color: The text color of the sub-component.

Menu Item

--calcite-menu-item-action-menu-border-color: The border color of the sub-component.
--calcite-menu-item-action-menu-text-color: The text color of the sub-component.

Panel

--calcite-panel-action-menu-border-color: The border color of the sub-component.
--calcite-panel-action-menu-text-color: The text color of the sub-component.

@alisonailea alisonailea added the design-tokens Issues requiring design tokens. label Feb 27, 2024
@alisonailea alisonailea changed the base branch from main to epic/7180-component-tokens February 27, 2024 18:10
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.

This looks good besides the missing popover tokens.

Because the PR and commit reference just a feature for action-menu, i wouldn't expect tokens added to block, menu-item, panel, action-group. I think this is fine its just a little odd to be updating other components. I would expect the updates to the other components to handle the vars for action-menu once its merged.

@alisonailea
Copy link
Contributor Author

This looks good besides the missing popover tokens.

Because the PR and commit reference just a feature for action-menu, i wouldn't expect tokens added to block, menu-item, panel, action-group. I think this is fine its just a little odd to be updating other components. I would expect the updates to the other components to handle the vars for action-menu once its merged.

As stated in my suggested workflow proposal document. It should be up to the person with the most knowledge of those component tokens to update them in dependent components. This workflow means no component is blocked by any other.

@alisonailea alisonailea added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Feb 28, 2024
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.

This looks good. Some of the CSS files could use sorting of props but not a showstopper. 👍

@alisonailea alisonailea merged commit 82cbf88 into epic/7180-component-tokens Feb 28, 2024
4 checks passed
@alisonailea alisonailea deleted the astump/7180-action-menu branch February 28, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-tokens Issues requiring design tokens. 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.

2 participants