From 6c9fe067a32c5532631e718bd4a79d474d91f674 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Sat, 3 Aug 2024 00:09:22 -0700 Subject: [PATCH 01/15] fix: ensure open/close events emit correctly if target element has multiple transitioned props --- .../src/components/sheet/sheet.e2e.ts | 14 +++++- .../src/components/sheet/sheet.tsx | 8 +++- .../src/tests/commonTests/openClose.ts | 46 +++++++++++-------- packages/calcite-components/src/utils/dom.ts | 2 +- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/packages/calcite-components/src/components/sheet/sheet.e2e.ts b/packages/calcite-components/src/components/sheet/sheet.e2e.ts index d37aab9f41b..6e3646d100e 100644 --- a/packages/calcite-components/src/components/sheet/sheet.e2e.ts +++ b/packages/calcite-components/src/components/sheet/sheet.e2e.ts @@ -1,6 +1,6 @@ import { newE2EPage } from "@stencil/core/testing"; import { html } from "../../../support/formatting"; -import { focusable, renders, hidden, defaults, accessible } from "../../tests/commonTests"; +import { accessible, defaults, focusable, hidden, openClose, renders } from "../../tests/commonTests"; import { GlobalTestProps, newProgrammaticE2EPage, skipAnimations } from "../../tests/utils"; import { CSS } from "./resources"; @@ -79,6 +79,18 @@ describe("calcite-sheet properties", () => { }); }); + describe("openClose", () => { + describe("default", () => { + openClose("calcite-modal"); + }); + + describe("initially open", () => { + openClose("calcite-modal", { + initialToggleValue: true, + }); + }); + }); + it("sets custom width correctly", async () => { const page = await newE2EPage(); // set large page to ensure test sheet isn't becoming fullscreen diff --git a/packages/calcite-components/src/components/sheet/sheet.tsx b/packages/calcite-components/src/components/sheet/sheet.tsx index 27b52aeb25f..8cc68f2dbf1 100644 --- a/packages/calcite-components/src/components/sheet/sheet.tsx +++ b/packages/calcite-components/src/components/sheet/sheet.tsx @@ -178,13 +178,14 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo [CSS.containerEmbedded]: this.embedded, [CSS_UTILITY.rtl]: dir === "rtl", }} + ref={this.setTransitionEl} >
@@ -297,9 +298,12 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo deactivateFocusTrap(this); } + private setContentId = (el: HTMLDivElement): void => { + this.contentId = ensureId(el); + }; + private setTransitionEl = (el: HTMLDivElement): void => { this.transitionEl = el; - this.contentId = ensureId(el); }; private openEnd = (): void => { diff --git a/packages/calcite-components/src/tests/commonTests/openClose.ts b/packages/calcite-components/src/tests/commonTests/openClose.ts index a423639ac78..9a49954c89f 100644 --- a/packages/calcite-components/src/tests/commonTests/openClose.ts +++ b/packages/calcite-components/src/tests/commonTests/openClose.ts @@ -1,6 +1,6 @@ import { E2EPage } from "@stencil/core/testing"; import { toHaveNoViolations } from "jest-axe"; -import { GlobalTestProps, newProgrammaticE2EPage, skipAnimations } from "../utils"; +import { GlobalTestProps, newProgrammaticE2EPage } from "../utils"; import { getTag, simplePageSetup } from "./utils"; import { TagOrHTML } from "./interfaces"; @@ -117,7 +117,11 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti ); } - async function testOpenCloseEvents(componentTagOrHTML: TagOrHTML, page: E2EPage): Promise { + async function testOpenCloseEvents( + componentTagOrHTML: TagOrHTML, + page: E2EPage, + animationsEnabled = true, + ): Promise { const tag = getTag(componentTagOrHTML); const element = await page.find(tag); @@ -129,6 +133,13 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti eventSequence.map(async (event) => await element.spyOnEvent(event)), ); + function assertEventSequence(expectedTimesPerEvent: [number, number, number, number]): void { + expect(beforeOpenSpy).toHaveReceivedEventTimes(expectedTimesPerEvent[0]); + expect(openSpy).toHaveReceivedEventTimes(expectedTimesPerEvent[1]); + expect(beforeCloseSpy).toHaveReceivedEventTimes(expectedTimesPerEvent[2]); + expect(closeSpy).toHaveReceivedEventTimes(expectedTimesPerEvent[3]); + } + await page.waitForChanges(); if (customizedOptions.beforeToggle) { @@ -140,12 +151,14 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti await page.waitForChanges(); await beforeOpenEvent; + + if (animationsEnabled) { + assertEventSequence([1, 0, 0, 0]); + } + await openEvent; - expect(beforeOpenSpy).toHaveReceivedEventTimes(1); - expect(openSpy).toHaveReceivedEventTimes(1); - expect(beforeCloseSpy).toHaveReceivedEventTimes(0); - expect(closeSpy).toHaveReceivedEventTimes(0); + assertEventSequence([1, 1, 0, 0]); if (customizedOptions.beforeToggle) { await customizedOptions.beforeToggle.close(page); @@ -156,25 +169,21 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti await page.waitForChanges(); await beforeCloseEvent; + + if (animationsEnabled) { + assertEventSequence([1, 1, 1, 0]); + } + await closeEvent; - expect(beforeCloseSpy).toHaveReceivedEventTimes(1); - expect(closeSpy).toHaveReceivedEventTimes(1); - expect(beforeOpenSpy).toHaveReceivedEventTimes(1); - expect(openSpy).toHaveReceivedEventTimes(1); + assertEventSequence([1, 1, 1, 1]); expect(await page.evaluate(() => (window as EventOrderWindow).events)).toEqual(eventSequence); } - /** - * `skipAnimations` utility sets the animation duration to 0.01. This is a workaround for an issue with the animation utility. - * Because this still leaves a very small duration, we can still test the animation events, but faster. - */ - if (customizedOptions.initialToggleValue === true) { it("emits on initialization with animations enabled", async () => { const page = await newProgrammaticE2EPage(); - await skipAnimations(page); await setUpPage(componentTagOrHTML, page); await testOpenCloseEvents(componentTagOrHTML, page); }); @@ -185,12 +194,11 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti content: `:root { --calcite-duration-factor: 0; }`, }); await setUpPage(componentTagOrHTML, page); - await testOpenCloseEvents(componentTagOrHTML, page); + await testOpenCloseEvents(componentTagOrHTML, page, false); }); } else { it(`emits with animations enabled`, async () => { const page = await simplePageSetup(componentTagOrHTML); - await skipAnimations(page); await setUpPage(componentTagOrHTML, page); await testOpenCloseEvents(componentTagOrHTML, page); }); @@ -201,7 +209,7 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti content: `:root { --calcite-duration-factor: 0; }`, }); await setUpPage(componentTagOrHTML, page); - await testOpenCloseEvents(componentTagOrHTML, page); + await testOpenCloseEvents(componentTagOrHTML, page, false); }); } } diff --git a/packages/calcite-components/src/utils/dom.ts b/packages/calcite-components/src/utils/dom.ts index 8226f654442..a80d1dbc370 100644 --- a/packages/calcite-components/src/utils/dom.ts +++ b/packages/calcite-components/src/utils/dom.ts @@ -737,7 +737,7 @@ export async function whenTransitionOrAnimationDone( const allProps = type === "transition" ? style.transitionProperty : style.animationName; const allDurationsArray = allDurations.split(","); - const allPropsArray = allProps.split(","); + const allPropsArray = allProps.split(",").map((prop) => prop.trim()); const propIndex = allPropsArray.indexOf(transitionPropOrAnimationName); const duration = allDurationsArray[propIndex] ?? From c5310b07a772f61829a4d5f63edafe37c3fd1a2b Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 5 Aug 2024 17:01:28 -0700 Subject: [PATCH 02/15] fix copypasta --- packages/calcite-components/src/components/sheet/sheet.e2e.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/calcite-components/src/components/sheet/sheet.e2e.ts b/packages/calcite-components/src/components/sheet/sheet.e2e.ts index 6e3646d100e..ef9d6500fc8 100644 --- a/packages/calcite-components/src/components/sheet/sheet.e2e.ts +++ b/packages/calcite-components/src/components/sheet/sheet.e2e.ts @@ -81,11 +81,11 @@ describe("calcite-sheet properties", () => { describe("openClose", () => { describe("default", () => { - openClose("calcite-modal"); + openClose("calcite-sheet"); }); describe("initially open", () => { - openClose("calcite-modal", { + openClose("calcite-sheet", { initialToggleValue: true, }); }); From ea2b4bfa813a9470510de6ed9ac712e0df38a7f0 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 5 Aug 2024 17:03:59 -0700 Subject: [PATCH 03/15] revisit timing test approach to avoid protocol errors when events are not fired properly --- .../src/tests/commonTests/openClose.ts | 59 +++++++++++++------ 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/packages/calcite-components/src/tests/commonTests/openClose.ts b/packages/calcite-components/src/tests/commonTests/openClose.ts index 9a49954c89f..df7062ae21c 100644 --- a/packages/calcite-components/src/tests/commonTests/openClose.ts +++ b/packages/calcite-components/src/tests/commonTests/openClose.ts @@ -33,6 +33,11 @@ interface OpenCloseOptions { * Optional argument with functions to simulate user input (mouse or keyboard), to open or close the component. */ beforeToggle?: BeforeToggle; + + /** + * When `true`, the test will assert that the delays match those used when animation is disabled + */ + willUseFallback?: boolean; } /** @@ -75,6 +80,7 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti const defaultOptions: OpenCloseOptions = { initialToggleValue: false, openPropName: "open", + willUseFallback: false, }; const customizedOptions = { ...defaultOptions, ...options }; @@ -117,6 +123,18 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti ); } + type OpenCloseName = "beforeOpen" | "open" | "beforeClose" | "close"; + + function toOpenCloseName(eventName: string): OpenCloseName { + return eventName.includes("BeforeOpen") + ? "beforeOpen" + : eventName.includes("Open") + ? "open" + : eventName.includes("BeforeClose") + ? "beforeClose" + : "close"; + } + async function testOpenCloseEvents( componentTagOrHTML: TagOrHTML, page: E2EPage, @@ -125,9 +143,19 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti const tag = getTag(componentTagOrHTML); const element = await page.find(tag); - const [beforeOpenEvent, openEvent, beforeCloseEvent, closeEvent] = eventSequence.map((event) => - page.waitForEvent(event), - ); + const timestamps: Record = { + beforeOpen: undefined, + open: undefined, + beforeClose: undefined, + close: undefined, + }; + + const [beforeOpenEvent, openEvent, beforeCloseEvent, closeEvent] = eventSequence.map((event) => { + return page.waitForEvent(event).then((spy) => { + timestamps[toOpenCloseName(event)] = Date.now(); + return spy; + }); + }); const [beforeOpenSpy, openSpy, beforeCloseSpy, closeSpy] = await Promise.all( eventSequence.map(async (event) => await element.spyOnEvent(event)), @@ -149,13 +177,7 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti } await page.waitForChanges(); - await beforeOpenEvent; - - if (animationsEnabled) { - assertEventSequence([1, 0, 0, 0]); - } - await openEvent; assertEventSequence([1, 1, 0, 0]); @@ -167,25 +189,28 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti } await page.waitForChanges(); - await beforeCloseEvent; - - if (animationsEnabled) { - assertEventSequence([1, 1, 1, 0]); - } - await closeEvent; assertEventSequence([1, 1, 1, 1]); expect(await page.evaluate(() => (window as EventOrderWindow).events)).toEqual(eventSequence); + + const delayDeltaThreshold = 10; + const delayBetweenBeforeOpenAndOpen = timestamps.open - timestamps.beforeOpen; + const delayBetweenBeforeCloseAndClose = timestamps.close - timestamps.beforeClose; + + const matcherName = animationsEnabled ? "toBeGreaterThan" : ("toBeLessThanOrEqual" as const); + + expect(delayBetweenBeforeOpenAndOpen)[matcherName](delayDeltaThreshold); + expect(delayBetweenBeforeCloseAndClose)[matcherName](delayDeltaThreshold); } if (customizedOptions.initialToggleValue === true) { it("emits on initialization with animations enabled", async () => { const page = await newProgrammaticE2EPage(); await setUpPage(componentTagOrHTML, page); - await testOpenCloseEvents(componentTagOrHTML, page); + await testOpenCloseEvents(componentTagOrHTML, page, !customizedOptions.willUseFallback); }); it("emits on initialization with animations disabled", async () => { @@ -200,7 +225,7 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti it(`emits with animations enabled`, async () => { const page = await simplePageSetup(componentTagOrHTML); await setUpPage(componentTagOrHTML, page); - await testOpenCloseEvents(componentTagOrHTML, page); + await testOpenCloseEvents(componentTagOrHTML, page, !customizedOptions.willUseFallback); }); it(`emits with animations disabled`, async () => { From bdfb459f6da5e470ff46fb9a59fdbc2b7d5c81fd Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 5 Aug 2024 19:33:34 -0700 Subject: [PATCH 04/15] fix tooltip test assertions --- .../calcite-components/src/components/tooltip/tooltip.e2e.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts b/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts index cf8d0c4b271..2bf7c84768a 100644 --- a/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts +++ b/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts @@ -167,7 +167,10 @@ describe("calcite-tooltip", () => { describe("openClose", () => { openClose(simpleTooltipHtml); - openClose(tooltipDisplayNoneHtml); + + describe("parent has display none", () => { + openClose(tooltipDisplayNoneHtml, { willUseFallback: true }); + }); }); it("should have zIndex of 901", async () => { From 859fb5a9f5440375186a16f18f9da199bec87716 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 5 Aug 2024 20:08:04 -0700 Subject: [PATCH 05/15] increase duration for animated cases to ensure assertions --- .../calcite-components/src/tests/commonTests/openClose.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/calcite-components/src/tests/commonTests/openClose.ts b/packages/calcite-components/src/tests/commonTests/openClose.ts index df7062ae21c..b639fda1778 100644 --- a/packages/calcite-components/src/tests/commonTests/openClose.ts +++ b/packages/calcite-components/src/tests/commonTests/openClose.ts @@ -209,6 +209,9 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti if (customizedOptions.initialToggleValue === true) { it("emits on initialization with animations enabled", async () => { const page = await newProgrammaticE2EPage(); + await page.addStyleTag({ + content: `:root { --calcite-duration-factor: 2; }`, + }); await setUpPage(componentTagOrHTML, page); await testOpenCloseEvents(componentTagOrHTML, page, !customizedOptions.willUseFallback); }); @@ -224,6 +227,9 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti } else { it(`emits with animations enabled`, async () => { const page = await simplePageSetup(componentTagOrHTML); + await page.addStyleTag({ + content: `:root { --calcite-duration-factor: 2; }`, + }); await setUpPage(componentTagOrHTML, page); await testOpenCloseEvents(componentTagOrHTML, page, !customizedOptions.willUseFallback); }); From ffd188ad550deafd75ed0680f482b015a6e561f4 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 5 Aug 2024 20:09:52 -0700 Subject: [PATCH 06/15] drop redundant alert transition item (targets all) --- packages/calcite-components/src/components/alert/alert.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/calcite-components/src/components/alert/alert.scss b/packages/calcite-components/src/components/alert/alert.scss index 8842ab0705e..87df1ea6199 100644 --- a/packages/calcite-components/src/components/alert/alert.scss +++ b/packages/calcite-components/src/components/alert/alert.scss @@ -38,7 +38,6 @@ $border-style: 1px solid var(--calcite-color-border-3); inline-size: var(--calcite-alert-width); max-inline-size: calc(100% - (var(--calcite-alert-edge-distance) * 2)); transition: - var(--calcite-internal-animation-timing-slow) $easing-function, opacity var(--calcite-internal-animation-timing-slow) $easing-function, all var(--calcite-animation-timing) ease-in-out; From cbf17431ab033c9833551c0abc45f611da3b83a7 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 12 Aug 2024 20:42:31 -0700 Subject: [PATCH 07/15] fix block open/close events --- .../calcite-components/src/components/block/block.scss | 3 ++- .../calcite-components/src/components/block/block.tsx | 9 +++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/calcite-components/src/components/block/block.scss b/packages/calcite-components/src/components/block/block.scss index 2cafa01e263..e1677c925f6 100644 --- a/packages/calcite-components/src/components/block/block.scss +++ b/packages/calcite-components/src/components/block/block.scss @@ -10,8 +10,9 @@ @extend %component-host; @extend %component-spacing; @apply transition-margin ease-cubic border-color-3 flex flex-shrink-0 flex-grow-0 - flex-col border-0 border-b border-solid p-0 duration-150; + flex-col border-0 border-b border-solid p-0; flex-basis: auto; + transition-duration: var(--calcite-animation-timing); } @include disabled(); diff --git a/packages/calcite-components/src/components/block/block.tsx b/packages/calcite-components/src/components/block/block.tsx index ab7eab7f5b1..c6845ec1dfd 100644 --- a/packages/calcite-components/src/components/block/block.tsx +++ b/packages/calcite-components/src/components/block/block.tsx @@ -224,7 +224,7 @@ export class Block @State() hasEndActions = false; - openTransitionProp = "opacity"; + openTransitionProp = "margin-top"; transitionEl: HTMLElement; @@ -239,6 +239,8 @@ export class Block connectInteractive(this); connectLocalized(this); connectMessages(this); + + this.transitionEl = this.el; } disconnectedCallback(): void { @@ -301,10 +303,6 @@ export class Block this.calciteBlockToggle.emit(); }; - private setTransitionEl = (el: HTMLElement): void => { - this.transitionEl = el; - }; - private actionsEndSlotChangeHandler = (event: Event): void => { this.hasEndActions = slotChangeHasAssignedElement(event); }; @@ -490,7 +488,6 @@ export class Block class={CSS.content} hidden={!open} id={IDS.content} - ref={this.setTransitionEl} > {this.renderScrim()} From 3f79d4a0db452d6443a5912ae547f12f09cd7f12 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Tue, 13 Aug 2024 15:43:21 -0700 Subject: [PATCH 08/15] fix notice open/close events --- .../src/components/notice/notice.scss | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/calcite-components/src/components/notice/notice.scss b/packages/calcite-components/src/components/notice/notice.scss index 13afdb1f8ad..3cf1ee07538 100644 --- a/packages/calcite-components/src/components/notice/notice.scss +++ b/packages/calcite-components/src/components/notice/notice.scss @@ -75,13 +75,14 @@ pointer-events-none my-0 box-border - hidden + flex w-full - opacity-0 - transition-default; + opacity-0; max-block-size: 0; + transition-property: opacity, max-block-size; + transition-duration: var(--calcite-animation-timing); text-align: start; - border-inline-start: 0 solid; + border-inline-start: var(--calcite-border-width-md) solid; box-shadow: 0 0 0 0 transparent; } @@ -100,10 +101,8 @@ :host([open]) .container { @apply shadow-1 pointer-events-auto - flex max-h-full items-center - border-2 opacity-100; } From 37f705b717165ba7e8200b811828cee75cdfb39e Mon Sep 17 00:00:00 2001 From: JC Franco Date: Tue, 13 Aug 2024 16:52:04 -0700 Subject: [PATCH 09/15] fix input-time-picker open/close events --- .../input-time-picker/input-time-picker.tsx | 75 ++++++++----------- 1 file changed, 32 insertions(+), 43 deletions(-) diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx index 374842f1687..8d48f568b2b 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx @@ -75,7 +75,6 @@ import { updateMessages, } from "../../utils/t9n"; import { getSupportedLocale } from "../../utils/locale"; -import { onToggleOpenCloseComponent, OpenCloseComponent } from "../../utils/openCloseComponent"; import { decimalPlaces } from "../../utils/math"; import { getIconScale } from "../../utils/component"; import { Validation } from "../functional/Validation"; @@ -169,7 +168,6 @@ export class InputTimePicker LabelableComponent, LoadableComponent, LocalizedComponent, - OpenCloseComponent, T9nComponent { //-------------------------------------------------------------------------- @@ -184,14 +182,12 @@ export class InputTimePicker @Watch("open") openHandler(): void { - onToggleOpenCloseComponent(this); - if (this.disabled || this.readOnly) { this.open = false; return; } - this.reposition(true); + this.popoverEl.open = this.open; } /** When `true`, interaction is prevented and the component is displayed with lower opacity. */ @@ -395,10 +391,6 @@ export class InputTimePicker private referenceElementId = `input-time-picker-${guid()}`; - openTransitionProp = "opacity"; - - transitionEl: HTMLCalciteInputElement; - //-------------------------------------------------------------------------- // // State @@ -559,21 +551,42 @@ export class InputTimePicker // // -------------------------------------------------------------------------- - onBeforeOpen(): void { + private popoverBeforeOpenHandler = (event: CustomEvent): void => { + event.stopPropagation(); this.calciteInputTimePickerBeforeOpen.emit(); - } + }; - onOpen(): void { + private popoverOpenHandler = (event: CustomEvent): void => { + event.stopPropagation(); this.calciteInputTimePickerOpen.emit(); - } + }; - onBeforeClose(): void { + private popoverBeforeCloseHandler = (event: CustomEvent): void => { + event.stopPropagation(); this.calciteInputTimePickerBeforeClose.emit(); - } - onClose(): void { + activateFocusTrap(this, { + onActivate: () => { + if (this.focusOnOpen) { + this.calciteTimePickerEl.setFocus(); + this.focusOnOpen = false; + } + }, + }); + }; + + private popoverCloseHandler = (event: CustomEvent): void => { + event.stopPropagation(); this.calciteInputTimePickerClose.emit(); - } + + deactivateFocusTrap(this, { + onDeactivate: () => { + this.calciteInputEl.setFocus(); + this.focusOnOpen = false; + }, + }); + this.open = false; + }; syncHiddenFormInput(input: HTMLInputElement): void { syncHiddenFormInput("time", this, input); @@ -685,27 +698,6 @@ export class InputTimePicker return timeString; } - private popoverCloseHandler = () => { - deactivateFocusTrap(this, { - onDeactivate: () => { - this.calciteInputEl.setFocus(); - this.focusOnOpen = false; - }, - }); - this.open = false; - }; - - private popoverOpenHandler = () => { - activateFocusTrap(this, { - onActivate: () => { - if (this.focusOnOpen) { - this.calciteTimePickerEl.setFocus(); - this.focusOnOpen = false; - } - }, - }); - }; - keyDownHandler = (event: KeyboardEvent): void => { const { defaultPrevented, key } = event; @@ -869,7 +861,6 @@ export class InputTimePicker private setInputAndTransitionEl = (el: HTMLCalciteInputElement): void => { this.calciteInputEl = el; - this.transitionEl = el; }; private setCalciteTimePickerEl = (el: HTMLCalciteTimePickerElement): void => { @@ -978,9 +969,6 @@ export class InputTimePicker async componentWillLoad(): Promise { setUpLoadableComponent(this); await Promise.all([setUpMessages(this), this.loadDateTimeLocaleData()]); - if (this.open) { - onToggleOpenCloseComponent(this); - } } componentDidLoad() { @@ -1046,9 +1034,10 @@ export class InputTimePicker id={dialogId} label={messages.chooseTime} lang={this.effectiveLocale} + onCalcitePopoverBeforeClose={this.popoverBeforeCloseHandler} + onCalcitePopoverBeforeOpen={this.popoverBeforeOpenHandler} onCalcitePopoverClose={this.popoverCloseHandler} onCalcitePopoverOpen={this.popoverOpenHandler} - open={this.open} overlayPositioning={this.overlayPositioning} placement={this.placement} ref={this.setCalcitePopoverEl} From 9e6786a2aa533bc5cd4c5c57374c373c2156d269 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 14 Aug 2024 10:55:06 -0700 Subject: [PATCH 10/15] adjust minimum delay used between animation and non-animation tests --- packages/calcite-components/src/tests/commonTests/openClose.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/calcite-components/src/tests/commonTests/openClose.ts b/packages/calcite-components/src/tests/commonTests/openClose.ts index b639fda1778..a22a18e0732 100644 --- a/packages/calcite-components/src/tests/commonTests/openClose.ts +++ b/packages/calcite-components/src/tests/commonTests/openClose.ts @@ -196,7 +196,7 @@ export function openClose(componentTagOrHTML: TagOrHTML, options?: OpenCloseOpti expect(await page.evaluate(() => (window as EventOrderWindow).events)).toEqual(eventSequence); - const delayDeltaThreshold = 10; + const delayDeltaThreshold = 100; // smallest internal animation timing used const delayBetweenBeforeOpenAndOpen = timestamps.open - timestamps.beforeOpen; const delayBetweenBeforeCloseAndClose = timestamps.close - timestamps.beforeClose; From 6f96d21bfb14f1189d4626866bba1fd17281870e Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 14 Aug 2024 11:00:04 -0700 Subject: [PATCH 11/15] fix block tests --- packages/calcite-components/src/components/block/block.e2e.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/calcite-components/src/components/block/block.e2e.ts b/packages/calcite-components/src/components/block/block.e2e.ts index 714121b03a4..984ac12335e 100644 --- a/packages/calcite-components/src/components/block/block.e2e.ts +++ b/packages/calcite-components/src/components/block/block.e2e.ts @@ -13,6 +13,7 @@ import { } from "../../tests/commonTests"; import { html } from "../../../support/formatting"; import { openClose } from "../../tests/commonTests"; +import { skipAnimations } from "../../tests/utils"; import { CSS, SLOTS } from "./resources"; describe("calcite-block", () => { @@ -189,6 +190,7 @@ describe("calcite-block", () => { const heading = "heading"; const page = await newE2EPage(); await page.setContent(html``); + await skipAnimations(page); const messages = await import(`./assets/block/t9n/messages.json`); const element = await page.find("calcite-block"); @@ -251,6 +253,7 @@ describe("calcite-block", () => {
fake space/enter-bubbling control
`); + await skipAnimations(page); const control = await page.find(".nested-control"); expect(await control.isVisible()).toBe(true); From 10b539e3dc7aba34bd2bf2c53f5629f6d80c8688 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 14 Aug 2024 11:21:50 -0700 Subject: [PATCH 12/15] fix focus trap regression --- .../input-time-picker/input-time-picker.e2e.ts | 4 +--- .../components/input-time-picker/input-time-picker.tsx | 10 +++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts index 6a7988adde7..e7ba99f7d87 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts @@ -859,8 +859,8 @@ describe("calcite-input-time-picker", () => { html`
next sibling
`, ); + await skipAnimations(page); const popover = await page.find("calcite-input-time-picker >>> calcite-popover"); - const stopgapDelayUntilOpenCloseEventsAreImplemented = 500; await page.keyboard.press("Tab"); expect(await getFocusedElementProp(page, "tagName")).toBe("CALCITE-INPUT-TIME-PICKER"); @@ -876,7 +876,6 @@ describe("calcite-input-time-picker", () => { await page.keyboard.press("ArrowDown"); await page.waitForChanges(); - await page.waitForTimeout(stopgapDelayUntilOpenCloseEventsAreImplemented); expect(await popover.isVisible()).toBe(true); expect(await getFocusedElementProp(page, "tagName", { shadow: true })).toBe("CALCITE-TIME-PICKER"); @@ -891,7 +890,6 @@ describe("calcite-input-time-picker", () => { await page.keyboard.press("Escape"); await page.waitForChanges(); - await page.waitForTimeout(stopgapDelayUntilOpenCloseEventsAreImplemented); expect(await popover.isVisible()).toBe(false); expect(await getFocusedElementProp(page, "tagName")).toBe("CALCITE-INPUT-TIME-PICKER"); diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx index 8d48f568b2b..8af300c8740 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx @@ -559,11 +559,6 @@ export class InputTimePicker private popoverOpenHandler = (event: CustomEvent): void => { event.stopPropagation(); this.calciteInputTimePickerOpen.emit(); - }; - - private popoverBeforeCloseHandler = (event: CustomEvent): void => { - event.stopPropagation(); - this.calciteInputTimePickerBeforeClose.emit(); activateFocusTrap(this, { onActivate: () => { @@ -575,6 +570,11 @@ export class InputTimePicker }); }; + private popoverBeforeCloseHandler = (event: CustomEvent): void => { + event.stopPropagation(); + this.calciteInputTimePickerBeforeClose.emit(); + }; + private popoverCloseHandler = (event: CustomEvent): void => { event.stopPropagation(); this.calciteInputTimePickerClose.emit(); From 4914e954bde2e93ececbbc0860e17a850f16eec2 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 14 Aug 2024 11:53:22 -0700 Subject: [PATCH 13/15] tidy up --- .../src/components/input-time-picker/input-time-picker.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx index 8af300c8740..cf1b87d132d 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx @@ -187,6 +187,7 @@ export class InputTimePicker return; } + // we set the property instead of the attribute to ensure popover's open/close events are emitted properly this.popoverEl.open = this.open; } @@ -859,7 +860,7 @@ export class InputTimePicker this.popoverEl = el; }; - private setInputAndTransitionEl = (el: HTMLCalciteInputElement): void => { + private setInputEl = (el: HTMLCalciteInputElement): void => { this.calciteInputEl = el; }; @@ -1022,7 +1023,7 @@ export class InputTimePicker onCalciteInputTextInput={this.calciteInternalInputInputHandler} onCalciteInternalInputTextFocus={this.calciteInternalInputFocusHandler} readOnly={readOnly} - ref={this.setInputAndTransitionEl} + ref={this.setInputEl} role="combobox" scale={this.scale} status={this.status} From d7250c8404c8d79977a4baaae578cd92a9ee5dff Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 14 Aug 2024 15:36:54 -0700 Subject: [PATCH 14/15] fix initially open input-time-picker use case --- .../src/components/input-time-picker/input-time-picker.e2e.ts | 4 ++++ .../src/components/input-time-picker/input-time-picker.tsx | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts index e7ba99f7d87..51c96aa6570 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts @@ -101,6 +101,10 @@ describe("calcite-input-time-picker", () => { describe("openClose", () => { openClose("calcite-input-time-picker"); + + describe("initially open", () => { + openClose("calcite-input-time-picker", { initialToggleValue: true }); + }); }); it("when set to readOnly, element still focusable but won't display the controls or allow for changing the value", async () => { diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx index cf1b87d132d..9cb9b2f212d 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx @@ -177,7 +177,6 @@ export class InputTimePicker //-------------------------------------------------------------------------- /** When `true`, displays the `calcite-time-picker` component. */ - @Prop({ reflect: true, mutable: true }) open = false; @Watch("open") @@ -985,6 +984,8 @@ export class InputTimePicker }), ); } + + this.openHandler(); } disconnectedCallback() { From 8662334eb6275025fa942303c5719d1f2286a10e Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 14 Aug 2024 15:37:30 -0700 Subject: [PATCH 15/15] ensure initially open tests are in a separate describe block and update the helper doc accordingly --- .../src/components/dialog/dialog.e2e.ts | 11 +++++----- .../src/components/modal/modal.e2e.ts | 11 +++++----- .../src/components/sheet/sheet.e2e.ts | 4 +--- .../src/tests/commonTests/openClose.ts | 22 ++++++++++--------- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/packages/calcite-components/src/components/dialog/dialog.e2e.ts b/packages/calcite-components/src/components/dialog/dialog.e2e.ts index 8e83bdcd82e..c7d084d30a5 100644 --- a/packages/calcite-components/src/components/dialog/dialog.e2e.ts +++ b/packages/calcite-components/src/components/dialog/dialog.e2e.ts @@ -29,12 +29,13 @@ describe("calcite-dialog", () => { }); describe("openClose", () => { - const openCloseOptions = { - initialToggleValue: true, - }; - openClose("calcite-dialog"); - openClose("calcite-dialog", openCloseOptions); + + describe("initially open", () => { + openClose("calcite-dialog", { + initialToggleValue: true, + }); + }); }); describe("slots", () => { diff --git a/packages/calcite-components/src/components/modal/modal.e2e.ts b/packages/calcite-components/src/components/modal/modal.e2e.ts index a57d1ba3939..d1914311e15 100644 --- a/packages/calcite-components/src/components/modal/modal.e2e.ts +++ b/packages/calcite-components/src/components/modal/modal.e2e.ts @@ -14,12 +14,13 @@ describe("calcite-modal", () => { }); describe("openClose", () => { - const openCloseOptions = { - initialToggleValue: true, - }; - openClose("calcite-modal"); - openClose("calcite-modal", openCloseOptions); + + describe("initially open", () => { + openClose("calcite-modal", { + initialToggleValue: true, + }); + }); }); describe("slots", () => { diff --git a/packages/calcite-components/src/components/sheet/sheet.e2e.ts b/packages/calcite-components/src/components/sheet/sheet.e2e.ts index ef9d6500fc8..94b50a9d8df 100644 --- a/packages/calcite-components/src/components/sheet/sheet.e2e.ts +++ b/packages/calcite-components/src/components/sheet/sheet.e2e.ts @@ -80,9 +80,7 @@ describe("calcite-sheet properties", () => { }); describe("openClose", () => { - describe("default", () => { - openClose("calcite-sheet"); - }); + openClose("calcite-sheet"); describe("initially open", () => { openClose("calcite-sheet", { diff --git a/packages/calcite-components/src/tests/commonTests/openClose.ts b/packages/calcite-components/src/tests/commonTests/openClose.ts index a22a18e0732..fa5386ae8ea 100644 --- a/packages/calcite-components/src/tests/commonTests/openClose.ts +++ b/packages/calcite-components/src/tests/commonTests/openClose.ts @@ -61,16 +61,18 @@ interface OpenCloseOptions { * } * }); * - * openClose("calcite-combobox", { - * initialToggleValue: true, - * beforeToggle: { - * close: async (page) => { - * await page.keyboard.press("Tab"); - * await page.waitForChanges(); - * }, - * } - * } - * }) + * describe("initially open", () => { + * openClose("calcite-combobox", { + * initialToggleValue: true, + * beforeToggle: { + * close: async (page) => { + * await page.keyboard.press("Tab"); + * await page.waitForChanges(); + * }, + * } + * } + * }); + * }); * * @param componentTagOrHTML - The component tag or HTML markup to test against. * @param {object} [options] - Additional options to assert.