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(rating): enhance component's interactivity states #11469

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

aPreciado88
Copy link
Contributor

Related Issue: #10015

Summary

  • Update star icon to fill on :hover.
  • Update star icon color to --calcite-color-brand-hover on :hover.
  • Update star icon color to --calcite-color-brand-press on :active.
  • Add --calcite-rating-color-press css token.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Feb 6, 2025
@aPreciado88 aPreciado88 added this to the 2025-02-25 - Feb Milestone milestone Feb 6, 2025
…ciado88/10015-rating-enhance-component-interactivity-states
Copy link
Contributor

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 github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Feb 14, 2025
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Aside from a few comments, code changes LGTM!

"--calcite-rating-color-press": {
shadowSelector: `.${CSS.star}`,
targetProp: "color",
state: { press: { attribute: "class", value: CSS.star } },
Copy link
Member

Choose a reason for hiding this comment

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

The object argument to state.press is deprecated. Use a selector instead (see example).

@@ -5,7 +5,8 @@
*
* @prop --calcite-rating-spacing-unit: [Deprecated] Use `--calcite-rating-spacing`. Specifies the amount of left and right margin spacing between each item.
* @prop --calcite-rating-spacing: Specifies the amount of left and right margin spacing between each item.
* @prop --calcite-rating-color-hover: Specifies the component's item color when hovered or selected.
* @prop --calcite-rating-color-hover: Specifies the component's item color when hovered.
* @prop --calcite-rating-color-press: Specifies the component's item color when active.
Copy link
Member

Choose a reason for hiding this comment

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

+@geospatialem @DitwanP @matgalla for CSS prop doc review.

@@ -70,7 +77,7 @@

.hovered,
Copy link
Member

Choose a reason for hiding this comment

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

Is .hovered still applicable here? It seems like it might be overridden above.

@aPreciado88 aPreciado88 added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed Stale Issues or pull requests that have not had recent activity. labels Feb 15, 2025
@aPreciado88
Copy link
Contributor Author

@ashetland @SkyeSeitz Can you please take a look? 👀

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.

2 participants