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

Merged

Conversation

aPreciado88
Copy link
Contributor

Related Issue: #10005

Summary

  • Update chip's interactivity states for chip-group's with selection modes.

  • appearance=“solid” kind=“neutral”:

    • Update hover background-color to --calcite-color-foreground-3.
    • Update active background-color to --calcite-color-border-3.
  • appearance=“solid” kind=“inverse”:

    • Update hover background-color to --calcite-color-inverse-hover.
    • Update active background-color to --calcite-color-inverse-press.
  • appearance=“solid” kind=“brand”:

    • Update hover background-color to --calcite-color-brand-hover.
    • Update active background-color to --calcite-color-brand-press.
  • appearance=“outline-fill” kind=“neutral”:

    • Update hover background-color to --calcite-color-foreground-2.
    • Update hover border-color to --calcite-color-border-input.
    • Update active background-color to --calcite-color-foreground-3.
    • Update active border-color to --calcite-color-text-3.
  • appearance=“outline-fill” kind=“inverse | brand”:

    • Update hover background-color to --calcite-color-foreground-2.
    • Update active background-color to --calcite-color-foreground-3.
  • appearance=“outline” kind=“neutral”:

    • Update hover background-color to --calcite-color-transparent-hover.
    • Update hover border-color to --calcite-color-border-input.
    • Update active background-color to --calcite-color-transparent-press.
    • Update active border-color to --calcite-color-text-3.
  • appearance=“outline” kind=“inverse | brand”:

    • Update hover background-color to --calcite-color-transparent-hover.
    • Update active background-color to --calcite-color-transparent-press.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Feb 12, 2025
@eriklharper
Copy link
Contributor

Didn't see any existing stories that would capture these new visual changes. Could you create a new story that shows all of these chip combinations so we can visually capture them with Chromatic? If you make a new one, you can add the _TestOnly suffix to the story name since it would just be for visual testing.

@eriklharper eriklharper self-requested a review February 13, 2025 22:36
@aPreciado88
Copy link
Contributor Author

@eriklharper I added stories to cover the new visual changes.

@eriklharper eriklharper added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Feb 14, 2025
@eriklharper
Copy link
Contributor

Hey @aPreciado88 ! Just saw this come in: #9085 Could you drop the _TestOnly suffix from the new story name? Thanks!

@aPreciado88 aPreciado88 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 14, 2025
@aPreciado88
Copy link
Contributor Author

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

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.

🚀

@@ -108,6 +108,95 @@
opacity: 1;
}

:host([appearance="solid"]) {
Copy link
Member

Choose a reason for hiding this comment

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

Would internal CSS props help simplify this?

.container {
  &.selectable {
    &:active {
      background-color: var(--calcite-internal-chip-selectable-active-background-color);
      border-color: var(--calcite-internal-chip-selectable-active-border-color);
    }
    &:hover {
      background-color: var(--calcite-internal-chip-selectable-hover-background-color);
      border-color: var(--calcite-internal-chip-selectable-hover-border-color);
    }
}

:host([appearance="solid"]) {
  &:host([kind="neutral"]) {
    // using illustrative names
    --calcite-internal-chip-selectable-active-background-color: var(--calcite-color-a);
    --calcite-internal-chip-selectable-active-border-color: transparent;
    --calcite-internal-chip-selectable-hover-background-color: var(--calcite-color-c);
    --calcite-internal-chip-selectable-hover-border-color: transparent;    
  }
}

// ...

This can be done in a follow-up if feasible.

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 can take a look at this and create a follow-up issue if it's feasible.

@@ -105,3 +105,79 @@ export const darkThemeRTL_TestOnly = (): string => html`
`;

darkThemeRTL_TestOnly.parameters = { themes: modesDarkDefault };

export const interactivityStates_TestOnly = (): string => html`
<div style="width:500px; max-width:100%; background-color:white; padding:100px">
Copy link
Member

Choose a reason for hiding this comment

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

This might not be the case, but let’s remove any non-essential inline styles for the test to keep it as focused and simple as possible.

@aPreciado88 aPreciado88 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 14, 2025
@ashetland
Copy link
Contributor

A thing of beauty! 🍾 🚀

@aPreciado88 aPreciado88 merged commit fcc129a into dev Feb 15, 2025
14 checks passed
@aPreciado88 aPreciado88 deleted the aPreciado88/10005-chip-enhance-component-interactivity-states branch February 15, 2025 00:43
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