Skip to content

fix(tab-title): enable support for deprecated icon color variable, add "press" variables #13681

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.

Sign up for GitHub

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

Jump to bottom
Open
brendan-vincent-rice wants to merge 7 commits into
base: dev
Choose a base branch
from
Open

fix(tab-title): enable support for deprecated icon color variable, add "press" variables #13681

brendan-vincent-rice wants to merge 7 commits into from
+42 −30

Conversation

@brendan-vincent-rice
Copy link
Member

@brendan-vincent-rice brendan-vincent-rice commented Jan 13, 2026

Related Issue: #13056

Summary

This PR enables support in the Tab Title component for the deprecated --calcite-ui-icon-color variable and adds a related end-to-end test. This PR also adds "press" variables to the component. Customers can now control start/end icon colors for the default and hover/selected states.

@brendan-vincent-rice brendan-vincent-rice self-assigned this Jan 13, 2026
@brendan-vincent-rice brendan-vincent-rice added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jan 13, 2026
@brendan-vincent-rice brendan-vincent-rice marked this pull request as ready for review January 13, 2026 16:03
brendan-vincent-rice
brendan-vincent-rice commented Jan 13, 2026
View reviewed changes
ashetland
ashetland approved these changes Jan 13, 2026
View reviewed changes
Copy link
Contributor

@ashetland ashetland left a comment

Choose a reason for hiding this comment

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

LGTM!

@brendan-vincent-rice brendan-vincent-rice mentioned this pull request Jan 13, 2026
Open
19 tasks
@matgalla matgalla 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 Jan 13, 2026
matgalla
matgalla requested changes Jan 14, 2026
View reviewed changes
Copy link
Contributor

@matgalla matgalla left a comment

Choose a reason for hiding this comment

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

--calcite-tab-close-icon-background-color-press's description says it updates the color "when hovered, focused, and active" and none of those are showing for me on 5.0.0-next.61, just when pressed specifically. This was different in 3.3.3: https://codepen.io/matgalla/pen/QwEGXEy

However, before I suggest changes or updating this description - @ashetland / @isaacbraun would this be at all related to recent Action changes?

@isaacbraun
Copy link
Contributor

isaacbraun commented Jan 14, 2026

--calcite-tab-close-icon-background-color-press's description says it updates the color "when hovered, focused, and active" and none of those are showing for me on 5.0.0-next.61, just when pressed specifically. This was different in 3.3.3: codepen.io/matgalla/pen/QwEGXEy

However, before I suggest changes or updating this description - @ashetland / @isaacbraun would this be at all related to recent Action changes?

Good catch 🎣 Yes, I was able to pinpoint the change in behavior to PR https://github.com/Esri/calcite-design-system/pull/13444/changes#diff-d1141ec2b59630222fe415d9945ea5c65250a439dd512eb51c2fd305db6dcee7, where the close icon was switched to use an Action and the --calcite-tab-close-icon-background-color-press was only applied to Action's --calcite-action-background-color-press token instead of focus, hover, and active as it was previously.

This change is in line with the token name, so unless we have a reason I'm not seeing to revert the behavior, I think the token description needs to be updated. This could be done as part of #12833.

cc @ashetland @matgalla

@matgalla
Copy link
Contributor

matgalla commented Jan 15, 2026

Nice sleuthing @isaacbraun! Since the change was intended, that makes sense to me.

Since we don't have that same level of granularity here with Tabs, we're essentially just missing a --calcite-tab-close-icon-background-color-hover prop here to cover our bases, I believe. @brendan-vincent-rice think we should look to add that in this PR?

If so, we can probably just update the description here for a (hopefully) simpler verification process?

@brendan-vincent-rice
Copy link
Member Author

brendan-vincent-rice commented Jan 15, 2026 •
edited
Loading

Nice sleuthing @isaacbraun! Since the change was intended, that makes sense to me.

Since we don't have that same level of granularity here with Tabs, we're essentially just missing a --calcite-tab-close-icon-background-color-hover prop here to cover our bases, I believe. @brendan-vincent-rice think we should look to add that in this PR?

If so, we can probably just update the description here for a (hopefully) simpler verification process?

@matgalla @isaacbraun @ashetland Matt and I paired today on this. I added a "hover" variable, which can be tested in the index.html test page using the following <body> markup. I was overthinking things initially; the addition was pretty straight forward. add hover component variable

Next up: I'll add e2e test coverage for the new variable.

<body
    style="
      padding: 3rem;
      --calcite-ui-icon-color: green;

      --calcite-tab-text-color: red;
      --calcite-tab-text-color-press: blue;

      --calcite-tab-icon-color-start: violet;
      --calcite-tab-icon-color-start-press: yellow;

      --calcite-tab-icon-color-end: orange;
      --calcite-tab-icon-color-end-press: silver;
      --calcite-tab-close-icon-background-color: beige;
      --calcite-tab-close-icon-background-color-press: blue;
      --calcite-tab-close-icon-background-color-hover: red;
    "
  >
    <demo-dom-swapper>
      <p>tab-title</p>
      <calcite-tabs>
        <calcite-tab-nav slot="title-group">
          <calcite-tab-title closable selected icon-start="embark" icon-end="embark"> Watercraft </calcite-tab-title>
          <calcite-tab-title closable icon-start="embark" icon-end="embark">Automobiles</calcite-tab-title>
        </calcite-tab-nav>
        <calcite-tab selected>
          <calcite-notice icon="embark" open>
            <div slot="message">Recommended for coastal use</div>
          </calcite-notice>
        </calcite-tab>
        <calcite-tab>
          <calcite-notice icon="car" open>
            <div slot="message">A good choice for inland adventure</div>
          </calcite-notice>
        </calcite-tab>
      </calcite-tabs>

      <br /><br />

      <calcite-tabs>
        <calcite-tab-nav slot="title-group">
          <calcite-tab-title closable selected icon-start="embark" icon-end="embark"> Watercraft </calcite-tab-title>
          <calcite-tab-title closable icon-start="embark" icon-end="embark">Automobiles</calcite-tab-title>
        </calcite-tab-nav>
        <calcite-tab selected>
          <calcite-notice icon="embark" open>
            <div slot="message">Recommended for coastal use</div>
          </calcite-notice>
        </calcite-tab>
        <calcite-tab>
          <calcite-notice icon="car" open>
            <div slot="message">A good choice for inland adventure</div>
          </calcite-notice>
        </calcite-tab>
      </calcite-tabs>

      <br /><br />

      <calcite-tabs bordered position="bottom">
        <calcite-tab-nav slot="title-group">
          <calcite-tab-title closable selected icon-start="embark" icon-end="embark"> Watercraft </calcite-tab-title>
          <calcite-tab-title closable icon-start="embark" icon-end="embark">Automobiles</calcite-tab-title>
        </calcite-tab-nav>
        <calcite-tab selected>
          <calcite-notice icon="embark" open>
            <div slot="message">Recommended for coastal use</div>
          </calcite-notice>
        </calcite-tab>
        <calcite-tab>
          <calcite-notice icon="car" open>
            <div slot="message">A good choice for inland adventure</div>
          </calcite-notice>
        </calcite-tab>
      </calcite-tabs>
    </demo-dom-swapper>
    <demo-options></demo-options>
  </body>

matgalla
matgalla requested changes Jan 15, 2026
View reviewed changes
isaacbraun
isaacbraun reviewed Jan 15, 2026
View reviewed changes
Copy link
Contributor

@isaacbraun isaacbraun left a comment

Choose a reason for hiding this comment

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

A few more comments regarding variable name changes, descriptions and when vars are/should be applied.

cc @Esri/calcite-pes for another set of eyes in case I'm missing something.

@brendan-vincent-rice brendan-vincent-rice 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 Jan 16, 2026
geospatialem
geospatialem reviewed Jan 16, 2026
View reviewed changes
@jcfranco jcfranco changed the title fix(tab title): enable support for deprecated icon color variable, add "press" variables fix(tab-title): enable support for deprecated icon color variable, add "press" variables Jan 16, 2026
@matgalla matgalla removed the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jan 17, 2026
@matgalla matgalla added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jan 17, 2026
matgalla
matgalla approved these changes Jan 17, 2026
View reviewed changes
Copy link
Contributor

@matgalla matgalla left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@geospatialem geospatialem geospatialem left review comments

@isaacbraun isaacbraun isaacbraun left review comments

@matgalla matgalla matgalla approved these changes

@ashetland ashetland ashetland approved these changes

Assignees

@brendan-vincent-rice brendan-vincent-rice

Labels

bug Bug reports for broken functionality. Issues should include a reproduction of the bug. pr ready for visual snapshots Adding this label will run visual snapshot testing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@brendan-vincent-rice @isaacbraun @matgalla @geospatialem @ashetland