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

refactor: create a common resource to store debounce consts #9829

Merged
merged 19 commits into from
Jul 27, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import {
t9n,
} from "../../tests/commonTests";
import { getFocusedElementProp } from "../../tests/utils";
import { DEBOUNCE } from "../../utils/resources";
import { CSS, SLOTS } from "./resources";
import { overflowActionsDebounceInMs } from "./utils";

describe("calcite-action-bar", () => {
describe("renders", () => {
Expand Down Expand Up @@ -406,7 +406,7 @@ describe("calcite-action-bar", () => {
</calcite-action-bar>
</div>`,
});
await page.waitForTimeout(overflowActionsDebounceInMs);
await page.waitForTimeout(DEBOUNCE.resize);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(2);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(0);
Expand All @@ -423,7 +423,7 @@ describe("calcite-action-bar", () => {
<calcite-action text="Table" icon="table"></calcite-action>`,
);
});
await page.waitForTimeout(overflowActionsDebounceInMs + 10);
await page.waitForTimeout(DEBOUNCE.resize + 10);
await page.waitForChanges();

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
Expand Down Expand Up @@ -454,15 +454,15 @@ describe("calcite-action-bar", () => {
</div>`,
);
await page.waitForChanges();
await page.waitForTimeout(overflowActionsDebounceInMs + 10);
await page.waitForTimeout(DEBOUNCE.resize + 10);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(0);

const actionBar = await page.find("calcite-action-bar");
actionBar.setProperty("overflowActionsDisabled", false);
await page.waitForChanges();
await page.waitForTimeout(overflowActionsDebounceInMs + 10);
await page.waitForTimeout(DEBOUNCE.resize + 10);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(7);
Expand Down Expand Up @@ -490,7 +490,7 @@ describe("calcite-action-bar", () => {
</calcite-action-bar>
</div>`,
});
await page.waitForTimeout(overflowActionsDebounceInMs + 10);
await page.waitForTimeout(DEBOUNCE.resize + 10);

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
expect(await page.findAll(slottedActionsSelector)).toHaveLength(7);
Expand All @@ -499,7 +499,7 @@ describe("calcite-action-bar", () => {
element.style.height = "550px";
});

await page.waitForTimeout(overflowActionsDebounceInMs + 10);
await page.waitForTimeout(DEBOUNCE.resize + 10);
await page.waitForChanges();

expect(await page.findAll(dynamicGroupActionsSelector)).toHaveLength(8);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,10 @@ import {
import { ExpandToggle, toggleChildActionText } from "../functional/ExpandToggle";
import { Layout, Position, Scale } from "../interfaces";
import { OverlayPositioning } from "../../utils/floating-ui";
import { DEBOUNCE } from "../../utils/resources";
import { ActionBarMessages } from "./assets/action-bar/t9n";
import { CSS, SLOTS } from "./resources";
import {
geActionDimensions,
getOverflowCount,
overflowActions,
overflowActionsDebounceInMs,
queryActions,
} from "./utils";
import { geActionDimensions, getOverflowCount, overflowActions, queryActions } from "./utils";

/**
* @slot - A slot for adding `calcite-action`s that will appear at the top of the component.
Expand Down Expand Up @@ -342,7 +337,7 @@ export class ActionBar
expanded,
overflowCount,
});
}, overflowActionsDebounceInMs);
}, DEBOUNCE.resize);

toggleExpand = (): void => {
this.expanded = !this.expanded;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { SLOTS as ACTION_GROUP_SLOTS } from "../action-group/resources";
import { SLOTS as ACTION_MENU_SLOTS } from "../action-menu/resources";
import { Layout } from "../interfaces";

export const overflowActionsDebounceInMs = 150;
Copy link
Member

Choose a reason for hiding this comment

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

✨🧹✨

const groupBufferPx = 2;

const getAverage = (arr: number[]) => arr.reduce((p, c) => p + c, 0) / arr.length;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { CSS as ComboboxItemCSS } from "../combobox-item/resources";
import { CSS as XButtonCSS } from "../functional/XButton";
import { getElementXY, newProgrammaticE2EPage, skipAnimations } from "../../tests/utils";
import { assertCaretPosition } from "../../tests/utils";
import { DEBOUNCE } from "../../utils/resources";
import { CSS } from "./resources";

const selectionModes = ["single", "single-persist", "ancestors", "multiple"];
Expand Down Expand Up @@ -201,6 +202,7 @@ describe("calcite-combobox", () => {

await combobox.type(text);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

expect(await combobox.getProperty("open")).toBe(true);

Expand All @@ -209,6 +211,7 @@ describe("calcite-combobox", () => {
}

await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);
expect(await combobox.getProperty("open")).toBe(false);
});

Expand Down Expand Up @@ -256,6 +259,7 @@ describe("calcite-combobox", () => {
await page.waitForChanges();

await input.type("undefined");
await page.waitForTimeout(DEBOUNCE.filter);
await page.waitForChanges();

expect(await items[0].isVisible()).toBe(false);
Expand Down Expand Up @@ -287,6 +291,7 @@ describe("calcite-combobox", () => {

await combobox.press("s");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);
expect(filterEventSpy).toHaveReceivedEventTimes(1);

expect(await items[0].isVisible()).toBe(true);
Expand All @@ -299,6 +304,7 @@ describe("calcite-combobox", () => {

await combobox.press("i");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);
expect(filterEventSpy).toHaveReceivedEventTimes(2);

expect(await items[0].isVisible()).toBe(true);
Expand All @@ -311,6 +317,7 @@ describe("calcite-combobox", () => {

await combobox.press("n");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);
expect(filterEventSpy).toHaveReceivedEventTimes(3);

expect(await items[0].isVisible()).toBe(true);
Expand Down Expand Up @@ -342,6 +349,7 @@ describe("calcite-combobox", () => {
await page.waitForChanges();
await combobox.type("Algeria");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const [lastItemX, lastItemY] = await getElementXY(page, "#item-4");

Expand Down Expand Up @@ -369,6 +377,7 @@ describe("calcite-combobox", () => {
await page.waitForChanges();
await combobox.type("two");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);
const one = await (await page.find("#one")).isVisible();
const two = await (await page.find("#two")).isVisible();
const three = await (await page.find("#three")).isVisible();
Expand Down Expand Up @@ -407,6 +416,7 @@ describe("calcite-combobox", () => {
const page = await newE2EPage();
await page.setContent(html` <calcite-combobox filter-text="1.2"> ${nestedComboboxChildren} </calcite-combobox> `);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const visibleItemsAndGroups = await page.findAll(
"calcite-combobox-item:not([hidden]), calcite-combobox-item-group:not([hidden])",
Expand Down Expand Up @@ -434,6 +444,7 @@ describe("calcite-combobox", () => {
const combobox = await page.find("calcite-combobox");
combobox.setProperty("filterText", "1.2");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const filteredItemsAndGroups = await page.findAll(
"calcite-combobox-item:not([hidden]), calcite-combobox-item-group:not([hidden])",
Expand All @@ -454,6 +465,7 @@ describe("calcite-combobox", () => {

combobox.setProperty("filterText", "");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const allVisibleItemAndGroups = await page.findAll(
"calcite-combobox-item:not([hidden]), calcite-combobox-item-group:not([hidden])",
Expand Down Expand Up @@ -500,10 +512,13 @@ describe("calcite-combobox", () => {
});

await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const combobox = await page.find("calcite-combobox");
combobox.setProperty("filterText", "foo");

await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const visibleItems = await page.findAll("calcite-combobox-item:not([hidden])");

Expand Down Expand Up @@ -1733,11 +1748,27 @@ describe("calcite-combobox", () => {
const button = await page.find("button");
await input.click();
await input.press("o");

await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

await input.press("Tab");

await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);
Copy link
Member

Choose a reason for hiding this comment

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

As a follow-up, let's see if this extra pair of waitForXs are needed. Also, some of the E2E/Puppeteer APIs call waitForChanges under the scenes, so some calls may not be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'm removing that waitForX double, seem so to pass without it.


let chips = await page.findAll("calcite-combobox >>> calcite-chip");
expect(chips.length).toBe(1);
await input.press("j");

await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

await button.click();

await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

chips = await page.findAll("calcite-combobox >>> calcite-chip");
expect(chips.length).toBe(2);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import {
import { connectLocalized, disconnectLocalized } from "../../utils/locale";
import { createObserver } from "../../utils/observers";
import { onToggleOpenCloseComponent, OpenCloseComponent } from "../../utils/openCloseComponent";
import { DEBOUNCE } from "../../utils/resources";
import {
connectMessages,
disconnectMessages,
Expand Down Expand Up @@ -1128,7 +1129,7 @@ export class Combobox
if (emit) {
this.calciteComboboxFilterChange.emit();
}
}, 100);
}, DEBOUNCE.filter);
})();

internalComboboxChangeEvent = (): void => {
Expand Down
10 changes: 5 additions & 5 deletions packages/calcite-components/src/components/filter/filter.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { E2EPage, newE2EPage } from "@stencil/core/testing";
import { accessible, defaults, disabled, focusable, hidden, reflects, renders, t9n } from "../../tests/commonTests";
import { CSS as INPUT_CSS } from "../input/resources";
import { DEBOUNCE_TIMEOUT } from "./resources";
import { DEBOUNCE } from "../../utils/resources";

describe("calcite-filter", () => {
describe("renders", () => {
Expand Down Expand Up @@ -190,7 +190,7 @@ describe("calcite-filter", () => {
it("updates filtered items after filtering", async () => {
const filter = await page.find("calcite-filter");
const filterChangeSpy = await page.spyOnEvent("calciteFilterChange");
await page.waitForTimeout(DEBOUNCE_TIMEOUT);
await page.waitForTimeout(DEBOUNCE.filter);
await page.waitForChanges();

expect(filterChangeSpy).toHaveReceivedEventTimes(0);
Expand All @@ -215,7 +215,7 @@ describe("calcite-filter", () => {
await page.$eval("calcite-filter", (filter: HTMLCalciteFilterElement): void => {
filter.items = filter.items.slice(3);
});
await page.waitForTimeout(DEBOUNCE_TIMEOUT);
await page.waitForTimeout(DEBOUNCE.filter);
await page.waitForChanges();

assertMatchingItems(await filter.getProperty("filteredItems"), ["jon"]);
Expand Down Expand Up @@ -274,15 +274,15 @@ describe("calcite-filter", () => {

it("should return matching value", async () => {
const filter = await page.find("calcite-filter");
await page.waitForTimeout(DEBOUNCE_TIMEOUT);
await page.waitForTimeout(DEBOUNCE.filter);
assertMatchingItems(await filter.getProperty("filteredItems"), ["harry"]);
});

it("should return no matching values", async () => {
const filter = await page.find("calcite-filter");
filter.setProperty("filterProps", ["description"]);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE_TIMEOUT);
await page.waitForTimeout(DEBOUNCE.filter);
assertMatchingItems(await filter.getProperty("filteredItems"), []);
});
});
Expand Down
5 changes: 3 additions & 2 deletions packages/calcite-components/src/components/filter/filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ import {
updateMessages,
} from "../../utils/t9n";
import { Scale } from "../interfaces";
import { DEBOUNCE } from "../../utils/resources";
import { FilterMessages } from "./assets/filter/t9n";
import { CSS, DEBOUNCE_TIMEOUT, ICONS } from "./resources";
import { CSS, ICONS } from "./resources";

@Component({
tag: "calcite-filter",
Expand Down Expand Up @@ -235,7 +236,7 @@ export class Filter
(value: string, emit = false, onFilter?: () => void): void =>
this.items.length &&
this.updateFiltered(filter(this.items, value, this.filterProps), emit, onFilter),
DEBOUNCE_TIMEOUT,
DEBOUNCE.filter,
);

inputHandler = (event: CustomEvent): void => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,3 @@ export const ICONS = {
search: "search",
close: "x",
} as const;

export const DEBOUNCE_TIMEOUT = 250;
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
t9n,
} from "../../tests/commonTests";
import { TagAndPage } from "../../tests/commonTests/interfaces";
import { DEBOUNCE } from "../../utils/resources";
import { toUserFriendlyName } from "./utils";

/*
Expand Down Expand Up @@ -227,6 +228,7 @@ describe("calcite-input-time-zone", () => {
await input.click();
await input.type(searchTerms[0]);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");

Expand All @@ -235,6 +237,7 @@ describe("calcite-input-time-zone", () => {
await clearSearchTerm(searchTerms[0]);
await input.type(searchTerms[1]);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");

Expand All @@ -243,13 +246,15 @@ describe("calcite-input-time-zone", () => {
await clearSearchTerm(searchTerms[1]);
await input.type(searchTerms[2]);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");

expect(matchedTimeZoneItems).toHaveLength(2);

await clearSearchTerm(searchTerms[1]);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");

Expand Down Expand Up @@ -448,6 +453,7 @@ describe("calcite-input-time-zone", () => {
await page.waitForChanges();
await input.type("(GMT-6)");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const sharedOffsetTimeZoneItems = await page.findAll(
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden])",
Expand All @@ -456,6 +462,7 @@ describe("calcite-input-time-zone", () => {

await sharedOffsetTimeZoneItems[1].click();
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const selectedTimeZoneItem = await page.find("calcite-input-time-zone >>> calcite-combobox-item[selected]");
const expectedTimeZoneItem = testTimeZoneItems[3];
Expand Down
Loading
Loading