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(pagination, split-button, dropdown, date-picker) action-group): add setFocus method #6405

Merged
merged 12 commits into from
Feb 3, 2023
Merged
25 changes: 10 additions & 15 deletions src/components/action-group/action-group.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,26 @@
import { accessible, hidden, renders, slots, t9n } from "../../tests/commonTests";
import { newE2EPage } from "@stencil/core/testing";
import { accessible, focusable, hidden, renders, slots, t9n } from "../../tests/commonTests";
import { SLOTS } from "./resources";

const actionGroupHTML = `<calcite-action-group scale="l">
<calcite-action id="plus" slot="menu-actions" text="Add" icon="plus"></calcite-action>
<calcite-action id="banaa" slot="menu-actions" text="Banana" icon="banana"></calcite-action>
</calcite-action-group>`;

describe("calcite-action-group", () => {
it("renders", async () => renders("calcite-action-group", { display: "flex" }));

it("focusable", async () => focusable(actionGroupHTML, { shadowFocusTargetSelector: "calcite-action" }));

it("honors hidden attribute", async () => hidden("calcite-action-group"));

it("should be accessible", async () =>
accessible(`
<calcite-action-group>
<calcite-action text="Add" icon="plus"></calcite-action>
</calcite-action-group>
`));
it("should be accessible", async () => accessible(actionGroupHTML));

it("has slots", () => slots("calcite-action-group", SLOTS));

it("should honor scale of expand icon", async () => {
const page = await newE2EPage({
html: `<calcite-action-group scale="l">
<calcite-action slot="menu-actions" text="Add" icon="plus"></calcite-action>
<calcite-action slot="menu-actions" text="Add" icon="plus"></calcite-action>
</calcite-action-group>`
});

const page = await newE2EPage({ html: actionGroupHTML });
const menu = await page.find(`calcite-action-group >>> calcite-action-menu`);

expect(await menu.getProperty("scale")).toBe("l");
});

Expand Down
30 changes: 27 additions & 3 deletions src/components/action-group/action-group.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import { Component, Element, h, Prop, Watch } from "@stencil/core";
import { Fragment, State, VNode } from "@stencil/core/internal";
import { Component, Element, Fragment, h, Method, Prop, State, VNode, Watch } from "@stencil/core";
import { CalciteActionMenuCustomEvent } from "../../components";
import {
ConditionalSlotComponent,
connectConditionalSlotComponent,
disconnectConditionalSlotComponent
} from "../../utils/conditionalSlot";
import { getSlotted } from "../../utils/dom";
import {
componentLoaded,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
} from "../../utils/loadable";
import { connectLocalized, disconnectLocalized, LocalizedComponent } from "../../utils/locale";
import {
connectMessages,
Expand All @@ -33,7 +38,9 @@ import { ICONS, SLOTS } from "./resources";
},
assetsDirs: ["assets"]
})
export class ActionGroup implements ConditionalSlotComponent, LocalizedComponent, T9nComponent {
export class ActionGroup
implements ConditionalSlotComponent, LoadableComponent, LocalizedComponent, T9nComponent
{
// --------------------------------------------------------------------------
//
// Properties
Expand Down Expand Up @@ -103,6 +110,18 @@ export class ActionGroup implements ConditionalSlotComponent, LocalizedComponent

@State() defaultMessages: ActionGroupMessages;

//--------------------------------------------------------------------------
//
// Public Methods
//
//--------------------------------------------------------------------------

/** Sets focus on the component's first focusable element. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
this.el.focus();
}
// --------------------------------------------------------------------------
//
// Lifecycle
Expand All @@ -122,9 +141,14 @@ export class ActionGroup implements ConditionalSlotComponent, LocalizedComponent
}

async componentWillLoad(): Promise<void> {
setUpLoadableComponent(this);
await setUpMessages(this);
}

componentDidLoad(): void {
setComponentLoaded(this);
}

// --------------------------------------------------------------------------
//
// Component Methods
Expand Down
11 changes: 8 additions & 3 deletions src/components/date-picker/date-picker.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { E2EPage, newE2EPage } from "@stencil/core/testing";
import { html } from "../../../support/formatting";
import { defaults, hidden, renders, t9n } from "../../tests/commonTests";
import { defaults, focusable, hidden, renders, t9n } from "../../tests/commonTests";
import { skipAnimations } from "../../tests/utils";
import { dateFromISO } from "../../utils/date";
import { formatTimePart } from "../../utils/time";

describe("calcite-date-picker", () => {
Expand All @@ -18,6 +17,11 @@ describe("calcite-date-picker", () => {
}
]));

it("focusable", async () =>
focusable("<calcite-date-picker></calcite-date-picker>", {
shadowFocusTargetSelector: "calcite-date-picker-month-header"
}));

const animationDurationInMs = 200;

it("fires a calciteDatePickerChange event when changing year in header", async () => {
Expand Down Expand Up @@ -209,7 +213,8 @@ describe("calcite-date-picker", () => {
await page.setContent("<calcite-date-picker value='2000-11-27'></calcite-date-picker>");
const date = await page.find("calcite-date-picker");
const changedEvent = await page.spyOnEvent("calciteDatePickerChange");
await date.setProperty("value", "2001-10-28");
date.setProperty("value", "2001-10-28");
Copy link
Member

Choose a reason for hiding this comment

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

Nice cleanup. Could you also add a await page.waitForChanges(); here?

await page.waitForChanges();
expect(changedEvent).toHaveReceivedEventTimes(0);
});

Expand Down
27 changes: 26 additions & 1 deletion src/components/date-picker/date-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
EventEmitter,
h,
Host,
Method,
Prop,
State,
VNode,
Expand All @@ -19,6 +20,12 @@ import {
HoverRange,
setEndOfDay
} from "../../utils/date";
import {
componentLoaded,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
} from "../../utils/loadable";
import {
connectLocalized,
disconnectLocalized,
Expand Down Expand Up @@ -46,7 +53,7 @@ import { DateLocaleData, getLocaleData, getValueAsDateRange } from "./utils";
delegatesFocus: true
}
})
export class DatePicker implements LocalizedComponent, T9nComponent {
export class DatePicker implements LocalizedComponent, LoadableComponent, T9nComponent {
//--------------------------------------------------------------------------
//
// Element
Expand Down Expand Up @@ -188,6 +195,19 @@ export class DatePicker implements LocalizedComponent, T9nComponent {

@State() endAsDate: Date;

//--------------------------------------------------------------------------
//
// Public Methods
//
//--------------------------------------------------------------------------

/** Sets focus on the component's first focusable element. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
Copy link
Member

Choose a reason for hiding this comment

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

Sidebar: maybe we need a new util or to update focusElement to do this for us (wait for load then focus).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that could be nice. Matt started a PR (#6156) to try to cut down the setFocus boilerplate but from our discussion it sounds like we can't use componentOnReady() internally. So your suggestion is a good plan B

this.el.focus();
}

// --------------------------------------------------------------------------
//
// Lifecycle
Expand Down Expand Up @@ -218,12 +238,17 @@ export class DatePicker implements LocalizedComponent, T9nComponent {
}

async componentWillLoad(): Promise<void> {
setUpLoadableComponent(this);
await this.loadLocaleData();
this.onMinChanged(this.min);
this.onMaxChanged(this.max);
await setUpMessages(this);
}

componentDidLoad(): void {
setComponentLoaded(this);
}

render(): VNode {
const date = dateFromRange(
this.range && Array.isArray(this.valueAsDate) ? this.valueAsDate[0] : this.valueAsDate,
Expand Down
46 changes: 19 additions & 27 deletions src/components/dropdown/dropdown.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
import { E2EPage, newE2EPage } from "@stencil/core/testing";
import dedent from "dedent";
import { html } from "../../../support/formatting";
import { accessible, defaults, disabled, floatingUIOwner, hidden, renders } from "../../tests/commonTests";
import { focusable, accessible, defaults, disabled, floatingUIOwner, hidden, renders } from "../../tests/commonTests";
import { GlobalTestProps } from "../../tests/utils";
import { CSS } from "./resources";

const simpleDropdownHTML = html`<calcite-dropdown>
<calcite-button slot="trigger">Open dropdown</calcite-button>
<calcite-dropdown-group id="group-1">
<calcite-dropdown-item id="item-1"> Dropdown Item Content </calcite-dropdown-item>
<calcite-dropdown-item id="item-2" selected> Dropdown Item Content </calcite-dropdown-item>
<calcite-dropdown-item id="item-3"> Dropdown Item Content </calcite-dropdown-item>
</calcite-dropdown-group>
</calcite-dropdown>`;

describe("calcite-dropdown", () => {
it("renders", () =>
renders(
html`<calcite-dropdown>
<calcite-button slot="trigger">Open dropdown</calcite-button>
<calcite-dropdown-group id="group-1">
<calcite-dropdown-item id="item-1"> Dropdown Item Content </calcite-dropdown-item>
<calcite-dropdown-item id="item-2" selected> Dropdown Item Content </calcite-dropdown-item>
<calcite-dropdown-item id="item-3"> Dropdown Item Content </calcite-dropdown-item>
</calcite-dropdown-group>
</calcite-dropdown>`,
{ display: "inline-flex" }
));
it("focusable", async () =>
focusable(simpleDropdownHTML, {
focusTargetSelector: '[slot="trigger"]'
}));

it("renders", () => renders(simpleDropdownHTML, { display: "inline-flex" }));

it("honors hidden attribute", async () => hidden("calcite-dropdown"));

Expand All @@ -33,18 +36,7 @@ describe("calcite-dropdown", () => {
}
]));

it("can be disabled", () =>
disabled(
html`<calcite-dropdown>
<calcite-button slot="trigger">Open dropdown</calcite-button>
<calcite-dropdown-group id="group-1">
<calcite-dropdown-item id="item-1"> Dropdown Item Content </calcite-dropdown-item>
<calcite-dropdown-item id="item-2" selected> Dropdown Item Content </calcite-dropdown-item>
<calcite-dropdown-item id="item-3"> Dropdown Item Content </calcite-dropdown-item>
</calcite-dropdown-group>
</calcite-dropdown>`,
{ focusTarget: "child" }
));
it("can be disabled", () => disabled(simpleDropdownHTML, { focusTarget: "child" }));

interface SelectedItemsAssertionOptions {
/**
Expand Down Expand Up @@ -748,7 +740,7 @@ describe("calcite-dropdown", () => {
expect(calciteDropdownOpen).toHaveReceivedEventTimes(1);
expect(calciteDropdownClose).toHaveReceivedEventTimes(0);

await trigger.focus();
await element.callMethod("setFocus");
await page.keyboard.press("Space");
await page.waitForChanges();
expect(await dropdownWrapper.isVisible()).toBe(false);
Expand Down Expand Up @@ -793,7 +785,7 @@ describe("calcite-dropdown", () => {
expect(calciteDropdownOpen).toHaveReceivedEventTimes(1);
expect(calciteDropdownClose).toHaveReceivedEventTimes(0);

await trigger.focus();
await element.callMethod("setFocus");
await page.keyboard.press("Space");
await page.waitForChanges();
expect(await dropdownWrapper.isVisible()).toBe(false);
Expand Down
28 changes: 27 additions & 1 deletion src/components/dropdown/dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ import {
import { guid } from "../../utils/guid";
import { InteractiveComponent, updateHostInteraction } from "../../utils/interactive";
import { isActivationKey } from "../../utils/key";
import {
componentLoaded,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
} from "../../utils/loadable";
import { createObserver } from "../../utils/observers";
import {
connectOpenCloseComponent,
Expand All @@ -56,7 +62,9 @@ import { SLOTS } from "./resources";
delegatesFocus: true
}
})
export class Dropdown implements InteractiveComponent, OpenCloseComponent, FloatingUIComponent {
export class Dropdown
implements InteractiveComponent, LoadableComponent, OpenCloseComponent, FloatingUIComponent
{
//--------------------------------------------------------------------------
//
// Element
Expand Down Expand Up @@ -185,6 +193,19 @@ export class Dropdown implements InteractiveComponent, OpenCloseComponent, Float
*/
@Prop({ reflect: true }) width: Scale;

//--------------------------------------------------------------------------
//
// Public Methods
//
//--------------------------------------------------------------------------

/** Sets focus on the component's first focusable element. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
this.el.focus();
}

//--------------------------------------------------------------------------
//
// Lifecycle
Expand All @@ -201,7 +222,12 @@ export class Dropdown implements InteractiveComponent, OpenCloseComponent, Float
connectOpenCloseComponent(this);
}

componentWillLoad(): void {
setUpLoadableComponent(this);
}

componentDidLoad(): void {
setComponentLoaded(this);
this.reposition(true);
}

Expand Down
16 changes: 13 additions & 3 deletions src/components/pagination/pagination.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
import { newE2EPage, E2EElement, E2EPage } from "@stencil/core/testing";
import { accessible, hidden, renders, t9n } from "../../tests/commonTests";
import { CSS } from "./resources";
import { E2EElement, E2EPage, newE2EPage } from "@stencil/core/testing";
import { html } from "../../../support/formatting";
import { accessible, focusable, hidden, renders, t9n } from "../../tests/commonTests";
import { CSS } from "./resources";

describe("calcite-pagination", () => {
it("renders", async () => renders("calcite-pagination", { display: "flex" }));

it("focuses previous button when not on the first page", async () =>
focusable('<calcite-pagination page-size="1" start-item="2" total-items="10"></calcite-pagination>', {
shadowFocusTargetSelector: `.${CSS.previous}`
}));

it("focuses page number 1 when on the first page", async () =>
focusable('<calcite-pagination page-size="1" start-item="1" total-items="10"></calcite-pagination>', {
shadowFocusTargetSelector: `.${CSS.page}`
}));

it("honors hidden attribute", async () => hidden("calcite-pagination"));

it("is accessible", async () => accessible(`<calcite-pagination></calcite-pagination>`));
Expand Down
Loading