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(panel, flow-item): add alerts slot #9778

Merged
merged 8 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 24 additions & 24 deletions packages/calcite-components/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,10 @@ export namespace Components {
* Specifies the duration before the component automatically closes - only use with `autoClose`.
*/
"autoCloseDuration": AlertDuration;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"embedded": boolean;
/**
* When `true`, shows a default recommended icon. Alternatively, pass a Calcite UI Icon name to display a specific icon.
*/
Expand Down Expand Up @@ -556,10 +560,6 @@ export namespace Components {
* Sets focus on the component's "close" button, the first focusable item.
*/
"setFocus": () => Promise<void>;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"slottedInShell": boolean;
}
interface CalciteAvatar {
/**
Expand Down Expand Up @@ -3388,6 +3388,10 @@ export namespace Components {
* When `true`, prevents the component from expanding to the entire screen on mobile devices.
*/
"docked": boolean;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"embedded": boolean;
/**
* When `true`, disables the default close on escape behavior.
*/
Expand Down Expand Up @@ -3438,10 +3442,6 @@ export namespace Components {
* Sets focus on the component's "close" button (the first focusable item).
*/
"setFocus": () => Promise<void>;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"slottedInShell": boolean;
/**
* Updates the element(s) that are used within the focus-trap of the component.
*/
Expand Down Expand Up @@ -4331,6 +4331,10 @@ export namespace Components {
* Specifies the display mode - `"float"` (content is separated detached), or `"overlay"` (displays on top of center content).
*/
"displayMode": DisplayMode;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"embedded": boolean;
/**
* When `true`, disables the default close on escape behavior.
*/
Expand Down Expand Up @@ -4367,10 +4371,6 @@ export namespace Components {
* Sets focus on the component's "close" button - the first focusable item.
*/
"setFocus": () => Promise<void>;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"slottedInShell": boolean;
/**
* Updates the element(s) that are used within the focus-trap of the component.
*/
Expand Down Expand Up @@ -8273,6 +8273,10 @@ declare namespace LocalJSX {
* Specifies the duration before the component automatically closes - only use with `autoClose`.
*/
"autoCloseDuration"?: AlertDuration;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"embedded"?: boolean;
/**
* When `true`, shows a default recommended icon. Alternatively, pass a Calcite UI Icon name to display a specific icon.
*/
Expand Down Expand Up @@ -8340,10 +8344,6 @@ declare namespace LocalJSX {
* Specifies the size of the component.
*/
"scale"?: Scale;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"slottedInShell"?: boolean;
}
interface CalciteAvatar {
/**
Expand Down Expand Up @@ -11359,6 +11359,10 @@ declare namespace LocalJSX {
* When `true`, prevents the component from expanding to the entire screen on mobile devices.
*/
"docked"?: boolean;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"embedded"?: boolean;
/**
* When `true`, disables the default close on escape behavior.
*/
Expand Down Expand Up @@ -11415,10 +11419,6 @@ declare namespace LocalJSX {
* Specifies the size of the component.
*/
"scale"?: Scale;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"slottedInShell"?: boolean;
/**
* Specifies the width of the component.
*/
Expand Down Expand Up @@ -12330,6 +12330,10 @@ declare namespace LocalJSX {
* Specifies the display mode - `"float"` (content is separated detached), or `"overlay"` (displays on top of center content).
*/
"displayMode"?: DisplayMode;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"embedded"?: boolean;
/**
* When `true`, disables the default close on escape behavior.
*/
Expand Down Expand Up @@ -12378,10 +12382,6 @@ declare namespace LocalJSX {
* Determines where the component will be positioned.
*/
"position"?: LogicalFlowPosition;
/**
* This internal property, managed by a containing calcite-shell, is used to inform the component if special configuration or styles are needed
*/
"slottedInShell"?: boolean;
/**
* When `position` is `"inline-start"` or `"inline-end"`, specifies the width of the component.
*/
Expand Down
4 changes: 4 additions & 0 deletions packages/calcite-components/src/components/alert/alert.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ describe("defaults", () => {
propertyName: "autoCloseDuration",
defaultValue: "medium",
},
{
propertyName: "embedded",
Copy link
Member

Choose a reason for hiding this comment

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

We should avoid testing internal props unless there's a strong use case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know it's an internal prop, but we should still test it to expect it to be false by default IMO. This adds protection from someone removing it accidentally or its value being changed.

defaultValue: false,
},
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ $alertDurations:
/**
* Conditional styles for when Alert is slotted in Shell
*/
.container--slotted-in-shell {
.container--embedded {
@apply absolute;
}

Expand Down
20 changes: 10 additions & 10 deletions packages/calcite-components/src/components/alert/alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ export class Alert implements OpenCloseComponent, LoadableComponent, T9nComponen
/** Specifies the duration before the component automatically closes - only use with `autoClose`. */
@Prop({ reflect: true }) autoCloseDuration: AlertDuration = "medium";

/**
* This internal property, managed by a containing calcite-shell, is used
* to inform the component if special configuration or styles are needed
*
* @internal
*/
@Prop({ mutable: true }) embedded = false;

/** Specifies the kind of the component, which will apply to top border and icon. */
@Prop({ reflect: true }) kind: Extract<
"brand" | "danger" | "info" | "success" | "warning",
Expand Down Expand Up @@ -141,14 +149,6 @@ export class Alert implements OpenCloseComponent, LoadableComponent, T9nComponen
/* wired up by t9n util */
}

/**
* This internal property, managed by a containing calcite-shell, is used
* to inform the component if special configuration or styles are needed
*
* @internal
*/
@Prop({ mutable: true }) slottedInShell: boolean;

@Watch("autoCloseDuration")
updateDuration(): void {
if (this.autoClose && this.autoCloseTimeoutId) {
Expand Down Expand Up @@ -205,7 +205,7 @@ export class Alert implements OpenCloseComponent, LoadableComponent, T9nComponen
window.clearTimeout(this.queueTimeout);
disconnectLocalized(this);
disconnectMessages(this);
this.slottedInShell = false;
this.embedded = false;
}

render(): VNode {
Expand All @@ -227,7 +227,7 @@ export class Alert implements OpenCloseComponent, LoadableComponent, T9nComponen
[CSS.container]: true,
[CSS.containerQueued]: queued,
[`${CSS.container}--${placement}`]: true,
[CSS.containerSlottedInShell]: this.slottedInShell,
[CSS.containerEmbedded]: this.embedded,
[CSS.focused]: this.keyBoardFocus,
}}
onPointerEnter={this.autoClose && this.autoCloseTimeoutId ? this.handleMouseOver : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const CSS = {
dismissProgress: "dismiss-progress",
footer: "footer",
icon: "icon",
containerSlottedInShell: "container--slotted-in-shell",
containerEmbedded: "container--embedded",
queueCount: "queue-count",
queueCountActive: "queue-count--active",
textContainer: "text-container",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@ import {
t9n,
} from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { GlobalTestProps } from "../../tests/utils";
import { CSS, SLOTS } from "./resources";

type TestWindow = GlobalTestProps<{
beforeClose: () => Promise<void>;
}>;

describe("calcite-flow-item", () => {
describe("renders", () => {
renders("calcite-flow-item", { display: "flex" });
Expand Down Expand Up @@ -210,8 +215,7 @@ describe("calcite-flow-item", () => {

await page.$eval(
"calcite-flow-item",
(el: HTMLCalciteFlowItemElement) =>
(el.beforeClose = (window as typeof window & Pick<typeof el, "beforeClose">).beforeClose),
(el: HTMLCalciteFlowItemElement) => (el.beforeClose = (window as TestWindow).beforeClose),
);

await page.waitForChanges();
Expand Down Expand Up @@ -327,4 +331,21 @@ describe("calcite-flow-item", () => {
expect(toggleSpy).toHaveReceivedEventTimes(1);
expect(await panel.getProperty("closed")).toBe(true);
});

it("should set embedded on slotted alerts", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

This is testing an internal prop. If screenshot tests cover this, we should drop this (and similar) tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot tests do cover this, but I do think this adds some protection.

const page = await newE2EPage({
html: html`<calcite-flow-item closable>
test
<calcite-alert slot="alerts" open label="this is a default alert">
<div slot="title">Hello there!</div>
<div slot="message">This is an alert with a general piece of information. Cool, innit?</div>
</calcite-alert>
</calcite-flow-item>`,
});
await page.waitForChanges();

const alert = await page.find("calcite-alert");

expect(await alert.getProperty("embedded")).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const { collapseDirection, scale } = ATTRIBUTES;
interface FlowItemStoryArgs
extends Pick<
FlowItem,
"closed" | "disabled" | "closable" | "collapsible" | "collapsed" | "collapseDirection" | "loading"
"closed" | "disabled" | "closable" | "collapsible" | "collapsed" | "collapseDirection" | "loading" | "scale"
> {
heightScale: string;
}
Expand Down Expand Up @@ -263,3 +263,13 @@ export const withNoHeaderBorderBlockEnd_TestOnly = (): string =>
html`<calcite-flow-item style="--calcite-flow-item-header-border-block-end:none;" height-scale="s" heading="My Panel"
>Slotted content!</calcite-flow-item
>`;

export const withAlertsSlot = (): string => html`
<calcite-flow-item height-scale="s" heading="My Panel" style="width: 500px; height:200px">
Slotted content!
<calcite-alert slot="alerts" open label="this is a default alert" scale="s">
<div slot="title">Hello there!</div>
<div slot="message">This is an alert with a general piece of information. Cool, innit?</div>
</calcite-alert>
</calcite-flow-item>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { CSS, ICONS, SLOTS } from "./resources";
/**
* @slot - A slot for adding custom content.
* @slot action-bar - A slot for adding a `calcite-action-bar` to the component.
* @slot alerts - A slot for adding `calcite-alert`s to the component.
* @slot content-top - A slot for adding content above the unnamed (default) slot and below the action-bar slot (if populated).
* @slot content-bottom - A slot for adding content below the unnamed (default) slot and above the footer slot (if populated)
* @slot header-actions-start - A slot for adding `calcite-action`s or content to the start side of the component's header.
Expand Down Expand Up @@ -396,6 +397,7 @@ export class FlowItem
>
{this.renderBackButton()}
<slot name={SLOTS.actionBar} slot={PANEL_SLOTS.actionBar} />
<slot name={SLOTS.alerts} slot={PANEL_SLOTS.alerts} />
<slot name={SLOTS.headerActionsStart} slot={PANEL_SLOTS.headerActionsStart} />
<slot name={SLOTS.headerActionsEnd} slot={PANEL_SLOTS.headerActionsEnd} />
<slot name={SLOTS.headerContent} slot={PANEL_SLOTS.headerContent} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const ICONS = {

export const SLOTS = {
actionBar: "action-bar",
alerts: "alerts",
contentTop: "content-top",
contentBottom: "content-bottom",
headerActionsStart: "header-actions-start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ slot[name="primary"] {
* Conditional styles for when Modal is slotted in Shell
*/

.container.slotted-in-shell {
.container--embedded {
position: absolute;
pointer-events: auto;
calcite-scrim {
Expand Down
22 changes: 11 additions & 11 deletions packages/calcite-components/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ export class Modal
/** When `true`, prevents the component from expanding to the entire screen on mobile devices. */
@Prop({ reflect: true }) docked: boolean;

/**
* This internal property, managed by a containing calcite-shell, is used
* to inform the component if special configuration or styles are needed
*
* @internal
*/
@Prop({ mutable: true }) embedded = false;

/** When `true`, disables the default close on escape behavior. */
@Prop({ reflect: true }) escapeDisabled = false;

Expand Down Expand Up @@ -156,14 +164,6 @@ export class Modal
/* wired up by t9n util */
}

/**
* This internal property, managed by a containing calcite-shell, is used
* to inform the component if special configuration or styles are needed
*
* @internal
*/
@Prop({ mutable: true }) slottedInShell: boolean;

//--------------------------------------------------------------------------
//
// Lifecycle
Expand Down Expand Up @@ -202,7 +202,7 @@ export class Modal
deactivateFocusTrap(this);
disconnectLocalized(this);
disconnectMessages(this);
this.slottedInShell = false;
this.embedded = false;
}

render(): VNode {
Expand All @@ -217,7 +217,7 @@ export class Modal
class={{
[CSS.container]: true,
[CSS.containerOpen]: this.opened,
[CSS.slottedInShell]: this.slottedInShell,
[CSS.containerEmbedded]: this.embedded,
}}
>
<calcite-scrim class={CSS.scrim} onClick={this.handleOutsideClose} />
Expand Down Expand Up @@ -534,7 +534,7 @@ export class Modal
this.titleId = ensureId(titleEl);
this.contentId = ensureId(contentEl);

if (!this.slottedInShell) {
if (!this.embedded) {
if (totalOpenModals === 0) {
initialDocumentOverflowStyle = document.documentElement.style.overflow;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ export const CSS = {
primary: "primary",
container: "container",
containerOpen: "container--open",
containerEmbedded: "container--embedded",
content: "content",
contentNoFooter: "content--no-footer",
contentBottom: "content-bottom",
contentTop: "content-top",
slottedInShell: "slotted-in-shell",

// these classes help apply the animation in phases to only set transform on open/close
// this helps avoid a positioning issue for any floating-ui-owning children
Expand Down
Loading
Loading