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

feat(navigation-logo): add component tokens #10582

Merged

anveshmekala merged 21 commits into dev from anveshmekala/7180-navigation-logo-add-tokens

Nov 22, 2024

Contributor

anveshmekala commented Oct 22, 2024 •

edited

Loading

Related Issue: #7180

Summary

Add the following component-scoped CSS Variables to calcite-navigation-logo:

--calcite-navigation-accent-color: Specifies the component's border color when active.
--calcite-navigation-background-color: Specifies the component's background color.
--calcite-navigation-logo-heading-text-color: Specifies the component's default color for heading text.
--calcite-navigation-logo-text-color: Specifies the component's default color for icon and description text.


          feat(navigation-logo): add component tokens

2129b15

github-actions bot assigned anveshmekala

github-actions bot added the enhancement label

anveshmekala commented

View reviewed changes

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated

    
              }

              .anchor:hover,

              .anchor:focus {

                @apply bg-foreground-2;

                background-color: var(--calcite-navigation-logo-background-color-hover, var(--calcite-color-foreground-2));

Contributor Author

anveshmekala Oct 22, 2024 •

edited

Loading

Should this be --calcite-navigation-logo-background-color-focus ? since #10002 removes background-color on :hover

Contributor

alisonailea Oct 25, 2024

same as above. I don't think state tokens are necessary here but if they are, first check if the navigation-logo background color is different than navigation? If not, use the same token for both --calcite-navigation-background-color-hover

anveshmekala and others added 4 commits

October 22, 2024 16:36


          replace active with press in tokens

64befdb


          stories

ea2f797


          Merge branch 'dev' into anveshmekala/7180-navigation-logo-add-tokens

28cbce7


          fix icon-color for active attribute

ce040f8

anveshmekala marked this pull request as ready for review

October 24, 2024 20:09

anveshmekala added the pr ready for visual snapshots label

anveshmekala and others added 2 commits

October 25, 2024 09:49


          Merge branch 'dev' into anveshmekala/7180-navigation-logo-add-tokens

04296df


          fix boolean attribute in stories

22f2ffa

anveshmekala added pr ready for visual snapshots and removed pr ready for visual snapshots labels

anveshmekala commented

View reviewed changes

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated

+              * @prop --calcite-navigation-logo-border-color: Specifies the border color of the component.
+              * @prop --calcite-navigation-logo-description-text-color: Specifies text color of description of component.
+              * @prop --calcite-navigation-logo-heading-text-color: Specifies the text color of heading of component.
+              * @prop --calcite-navigation-logo-icon-color-press: Specifies the icon color of the component when active.

Contributor Author

anveshmekala Oct 25, 2024

should this be --calcite-navigation-logo-color-press? The reason for using --calcite-navigation-logo-icon-color-press is to indicate the color only affects the icon since there is no text at host level other than heading or description which has it own tokens.

Contributor

alisonailea Oct 25, 2024

the heading - description pattern should be

--calcite-[component]-text-color: Specifies the component's default color for description and icons

--calcite-[component]-heading-text-color: Specifies the component's heading text color.

alisonailea requested changes

View reviewed changes

Contributor

alisonailea left a comment

I think we can simplify tokens here and leave the extra tailwind changes. We want to keep using the rem styles for now.

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated Show resolved Hide resolved

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated

+              * @prop --calcite-navigation-logo-border-color: Specifies the border color of the component.
+              * @prop --calcite-navigation-logo-description-text-color: Specifies text color of description of component.
+              * @prop --calcite-navigation-logo-heading-text-color: Specifies the text color of heading of component.
+              * @prop --calcite-navigation-logo-icon-color-press: Specifies the icon color of the component when active.

Contributor

alisonailea Oct 25, 2024

the heading - description pattern should be

--calcite-[component]-text-color: Specifies the component's default color for description and icons

--calcite-[component]-heading-text-color: Specifies the component's heading text color.

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated

+                font-size: var(--calcite-font-size-0);
+                line-height: var(--calcite-font-line-height-fixed-lg);
+                border-block-end: var(--calcite-spacing-base) solid var(--calcite-navigation-logo-border-color, transparent);
+                background-color: var(--calcite-navigation-logo-background-color, var(--calcite-color-foreground-1));

Contributor

alisonailea Oct 25, 2024

if the background color of the navigation-logo different than the background color of navigation? If not, use the same token for both --calcite-navigation-background-color

Contributor Author

anveshmekala Oct 25, 2024

as discussed in teams, navigation-logo is not expected to use outside of navigation. will fallback this to navigation-background-color

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated

    
              }

              .anchor:hover,

              .anchor:focus {

                @apply bg-foreground-2;

                background-color: var(--calcite-navigation-logo-background-color-hover, var(--calcite-color-foreground-2));

Contributor

alisonailea Oct 25, 2024

same as above. I don't think state tokens are necessary here but if they are, first check if the navigation-logo background color is different than navigation? If not, use the same token for both --calcite-navigation-background-color-hover

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated Show resolved Hide resolved

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated

-                @apply text-color-1;
-                border-color: var(--calcite-color-brand);
-                --calcite-icon-color: var(--calcite-color-brand);
+                --calcite-icon-color: var(--calcite-navigation-logo-icon-color-press, var(--calcite-color-brand));

Contributor

alisonailea Oct 25, 2024

this should be

.icon {
color: var(--calcite-navigation-logo-icon-color-press, var(--calcite-icon-color, var(--calcite-color-brand)));
}

Contributor Author

anveshmekala Oct 25, 2024

does the state tokens apply here?

we can use one token

.icon {
  color: var(--calcite-navigation-logo-text-color, var(--calcite-internal-navigation-logo-text-color, var(--calcite-icon-color, inherit)));
}

:host([active]),
:host(:active) {
  .icon {
 --calcite-internal-navigation-logo-text-color: var(--calcite-icon-color, var(--calcite-color-brand)));
  }
}

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated Show resolved Hide resolved

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated Show resolved Hide resolved

packages/calcite-components/src/components/navigation-logo/navigation-logo.scss Outdated Show resolved Hide resolved

anveshmekala added 2 commits

October 25, 2024 15:28


          remove state tokens

e9cb59f


          refactor stories

f160022

anveshmekala requested a review from alisonailea

October 25, 2024 20:43

anveshmekala and others added 2 commits

October 25, 2024 15:50


          editorial cleanup

80f094b


          Merge branch 'dev' into anveshmekala/7180-navigation-logo-add-tokens

43c8efe

anveshmekala added pr ready for visual snapshots and removed pr ready for visual snapshots labels

anveshmekala and others added 2 commits

October 30, 2024 15:57


          Merge branch 'dev' into anveshmekala/7180-navigation-logo-add-tokens

516041c


          tidy up

5aab492

alisonailea approved these changes

View reviewed changes


          refactor

8f2e81d

anveshmekala requested a review from benelan as a code owner

November 14, 2024 21:50

anveshmekala requested a review from jcfranco as a code owner

November 14, 2024 21:50


          revert unwanted changed

27530eb

anveshmekala added pr ready for visual snapshots and removed pr ready for visual snapshots labels

anveshmekala and others added 2 commits

November 20, 2024 17:22


          Merge branch 'dev' into anveshmekala/7180-navigation-logo-add-tokens

0d3f2f5


          rename selection-border-color to accent-color

6ba5740

anveshmekala added pr ready for visual snapshots and removed pr ready for visual snapshots labels

anveshmekala and others added 3 commits

November 21, 2024 17:38


          resolve merge conflicts

4da000e


          Merge branch 'dev' into anveshmekala/7180-navigation-logo-add-tokens

a83ee45


          fix stylelintrc config

anveshmekala added pr ready for visual snapshots and removed pr ready for visual snapshots labels

anveshmekala mentioned this pull request

Design token phase II - Integration across components #7180

Open


          Merge branch 'dev' into anveshmekala/7180-navigation-logo-add-tokens

00a307f

anveshmekala added pr ready for visual snapshots and removed pr ready for visual snapshots labels

anveshmekala merged commit 0638891 into dev

15 checks passed

anveshmekala deleted the anveshmekala/7180-navigation-logo-add-tokens branch

November 22, 2024 04:36

github-actions bot added this to the 2024-11-26 - 3.0.0 Nov Breaking Change Milestone milestone

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

Labels

enhancement pr ready for visual snapshots