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(notice): add component tokens #11042

Merged
merged 12 commits into from
Jan 4, 2025
55 changes: 54 additions & 1 deletion packages/calcite-components/src/components/notice/notice.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { newE2EPage } from "@arcgis/lumina-compiler/puppeteerTesting";
import { describe, expect, it } from "vitest";
import { accessible, focusable, renders, slots, hidden, t9n } from "../../tests/commonTests";
import { accessible, focusable, renders, slots, hidden, themed, t9n } from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { openClose } from "../../tests/commonTests";
import { CSS, SLOTS } from "./resources";
Expand Down Expand Up @@ -129,4 +129,57 @@ describe("calcite-notice", () => {
describe("translation support", () => {
t9n("calcite-notice");
});

describe("theme", () => {
describe("default", () => {
themed(
html`
<calcite-notice kind="danger" open closable>
<div slot="title">Title</div>
<div slot="message">Message</div>
<calcite-link slot="link" title="my action">Retry</calcite-link>
</calcite-notice>
`,
{
"--calcite-notice-background-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "backgroundColor",
},
"--calcite-notice-close-text-color": {
shadowSelector: `.${CSS.close}`,
targetProp: "color",
},
"--calcite-notice-close-text-color-hover": [
{
shadowSelector: `.${CSS.close}`,
targetProp: "color",
state: "focus",
},
{
shadowSelector: `.${CSS.close}`,
targetProp: "color",
state: "hover",
},
],
"--calcite-notice-close-background-color-focus": [
{
shadowSelector: `.${CSS.close}`,
targetProp: "backgroundColor",
state: "focus",
},
{
shadowSelector: `.${CSS.close}`,
targetProp: "backgroundColor",
state: "hover",
},
],
"--calcite-notice-close-background-color-press": {
shadowSelector: `.${CSS.close}`,
targetProp: "backgroundColor",
state: { press: { attribute: "class", value: CSS.close } },
},
},
);
});
});
});
30 changes: 22 additions & 8 deletions packages/calcite-components/src/components/notice/notice.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@
*
* These properties can be overridden using the component's tag as selector.
*
* @prop --calcite-notice-width: The width of the component.
* @prop --calcite-notice-background-color: Specifies the background color of the component.
* @prop --calcite-notice-close-background-color-focus: Specifies the background color of the component when focused.
* @prop --calcite-notice-close-background-color-press: Specifies the background color of the component when active.
* @prop --calcite-notice-close-text-color-hover: Specifies the background color of component's close button when hovered.
* @prop --calcite-notice-close-text-color: Specifies the text color of component's close button.
* @prop --calcite-notice-content-text-color: Specifies the content text color of the component.
* @prop --calcite-notice-width: Specifies the width of the component.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can evaluate this for removal. Not sure why we'd need this, vs. the user being able to set this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah WDYT @alisonailea?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree we could set this for removal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DitwanP @geospatialem is there any specific process/term we need for deprecation and removal for this or just [Deprecated] note for now is enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the [Deprecated] note is enough.

Copy link
Member

Choose a reason for hiding this comment

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

@Elijbet We have a new deprecate conventional commit, so would recommend we use that approach for the deprecation so users are aware of the deprecation via our changelog.

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 will create a follow-up PR for this.

*/

// scale variables
Expand Down Expand Up @@ -71,20 +77,21 @@
}

.container {
@apply bg-foreground-1
pointer-events-none
@apply pointer-events-none
my-0
box-border
flex
w-full
opacity-0;

overflow: hidden;
max-block-size: 0;
transition-property: opacity, max-block-size;
transition-duration: var(--calcite-animation-timing);
text-align: start;
border-inline-start: var(--calcite-border-width-md) solid;
box-shadow: 0 0 0 0 transparent;
background-color: var(--calcite-notice-background-color, var(--calcite-color-foreground-1));
}

// focus styles
Expand All @@ -109,12 +116,16 @@
}

@include slotted("title", "*", ".container") {
@apply text-color-1 m-0 font-medium;
@apply m-0 font-medium;

color: var(--calcite-notice-close-text-color-hover, var(--calcite-color-text-1));
}

@include slotted("message", "*", ".container") {
@apply text-color-2 m-0 inline font-normal;
@apply m-0 inline font-normal;

margin-inline-end: var(--calcite-notice-spacing-token-small);
color: var(--calcite-notice-content-text-color, var(--calcite-color-text-2));
}

@mixin notice-element-base() {
Expand Down Expand Up @@ -146,17 +157,20 @@
}

.notice-close {
@apply text-color-3 flex cursor-pointer items-center self-stretch border-none bg-transparent outline-none;
@apply flex cursor-pointer items-center self-stretch border-none bg-transparent outline-none;
@include notice-element-base;
-webkit-appearance: none;

color: var(--calcite-notice-close-text-color, var(--calcite-color-text-3));

&:hover,
&:focus {
@apply bg-foreground-2 text-color-1;
background-color: var(--calcite-notice-close-background-color-focus, var(--calcite-color-foreground-2));
color: var(--calcite-notice-close-text-color-hover, var(--calcite-color-text-1));
}

&:active {
@apply bg-foreground-3;
background-color: var(--calcite-notice-close-background-color-press, var(--calcite-color-foreground-3));
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/calcite-components/src/custom-theme.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { label, labelTokens } from "./custom-theme/label";
import { link, linkTokens } from "./custom-theme/link";
import { list, listTokens } from "./custom-theme/list";
import { loader, loaderTokens } from "./custom-theme/loader";
import { notices } from "./custom-theme/notice";
import { notice, noticeTokens } from "./custom-theme/notice";
import { pagination } from "./custom-theme/pagination";
import { popover, popoverTokens } from "./custom-theme/popover";
import { progress, progressTokens } from "./custom-theme/progress";
Expand Down Expand Up @@ -113,7 +113,7 @@ const kitchenSink = (args: Record<string, string>, useTestValues = false) =>
</style>
<div class="demo">
<div class="demo-column">
${accordion} ${actionBar} ${autocomplete} ${notices} ${segmentedControl}
${accordion} ${actionBar} ${autocomplete} ${notice} ${segmentedControl}
<div style="display: flex">
${actionPad}
<div style="width: 40px; height: 40px;">${actionMenu}</div>
Expand Down Expand Up @@ -164,6 +164,7 @@ const componentTokens = {
...navigationLogoTokens,
...navigationTokens,
...navigationUserTokens,
...noticeTokens,
...popoverTokens,
...progressTokens,
...sliderTokens,
Expand Down
28 changes: 12 additions & 16 deletions packages/calcite-components/src/custom-theme/notice.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import { html } from "../../support/formatting";

export const notice = ({ kind, message, title }: { kind: string; message: string; title: string }): string =>
html`<calcite-notice kind=${kind} scale="s" open closable>
<div slot="title">${title}</div>
<div slot="message">${message}</div>
${kind === "danger" ? html`<calcite-link slot="link" title="my action">Retry</calcite-link>` : null}
</calcite-notice>`;
export const noticeTokens = {
calciteNoticeBackgroundColor: "",
calciteNoticeCloseBackgroundColorFocus: "",
calciteNoticeCloseBackgroundColorPress: "",
calciteNoticeCloseTextColorHover: "",
calciteNoticeCloseTextColor: "",
calciteNoticeContentTextColor: "",
};

export const notices = html`${notice({
kind: "danger",
message: "There was an error while performing the task.",
title: "Something failed",
})}
${notice({
kind: "success",
message: "That thing you wanted to do worked as expected",
title: "Something worked",
})}`;
export const notice = html`<calcite-notice kind="success" scale="s" open closable>
<div slot="title">Something worked</div>
<div slot="message">That thing you wanted to do worked as expected</div>
</calcite-notice>`;
3 changes: 2 additions & 1 deletion packages/calcite-components/src/tests/commonTests/themed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { RequireExactlyOne } from "type-fest";
import { E2EPage, E2EElement } from "@arcgis/lumina-compiler/puppeteerTesting";
import { expect, it } from "vitest";
import { getTokenValue } from "../utils/cssTokenValues";
import { toElementHandle } from "../utils";
import { skipAnimations, toElementHandle } from "../utils";
import type { ComponentTestSetup } from "./interfaces";
import { getTagAndPage } from "./utils";

Expand Down Expand Up @@ -75,6 +75,7 @@ export type ComponentTestTokens = Record<CalciteCSSCustomProp, TestSelectToken |
export function themed(componentTestSetup: ComponentTestSetup, tokens: ComponentTestTokens): void {
it("is themeable", async () => {
const { page, tag } = await getTagAndPage(componentTestSetup);
await skipAnimations(page);
await page.evaluate(() => {
// we block all clicks to prevent triggering behavior as mouse states are activated between assertions
document.addEventListener(
Expand Down
Loading