add comments to focusTrap options and cleanu up tests

Esri · Oct 3, 2024 · 06e68ff · 06e68ff
1 parent 7e3dda8
commit 06e68ff
Show file tree

Hide file tree

Showing 6 changed files with 4 additions and 30 deletions.
diff --git a/packages/calcite-components/src/components/dialog/dialog.tsx b/packages/calcite-components/src/components/dialog/dialog.tsx
@@ -232,6 +232,7 @@ export class Dialog
     connectMessages(this);
     connectFocusTrap(this, {
       focusTrapOptions: {
+        // Scrim has it's own close handler, allow it to take over.
         clickOutsideDeactivates: false,
         escapeDeactivates: (event) => {
           if (event.defaultPrevented || this.escapeDisabled) {

diff --git a/packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts b/packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts
@@ -428,8 +428,6 @@ describe("calcite-input-date-picker", () => {
       }
 
       it("toggles the date picker when clicked", async () => {
-        const openSpy = await inputDatePicker.spyOnEvent("calciteInputDatePickerOpen");
-        const closeSpy = await inputDatePicker.spyOnEvent("calciteInputDatePickerClose");
         const calendar = await page.find(`calcite-input-date-picker >>> .${CSS.calendarWrapper}`);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(false);
@@ -454,13 +452,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await startInput.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(1);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(true);
 
         await startInput.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(1);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(false);
 
@@ -469,13 +465,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await startInputToggle.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(2);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(true);
 
         await startInputToggle.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(2);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(false);
 
@@ -484,13 +478,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await endInput.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(3);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(true);
 
         await endInput.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(3);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(false);
 
@@ -499,13 +491,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await endInputToggle.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(4);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(true);
 
         await endInputToggle.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(4);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(false);
 
@@ -514,13 +504,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await startInput.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(5);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(true);
 
         await startInputToggle.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(5);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(false);
 
@@ -529,13 +517,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await startInputToggle.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(6);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(true);
 
         await startInput.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(6);
 
         expect(await isCalendarVisible(calendar, "start")).toBe(false);
 
@@ -544,13 +530,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await endInput.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(7);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(true);
 
         await endInputToggle.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(7);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(false);
 
@@ -559,13 +543,11 @@ describe("calcite-input-date-picker", () => {
         await resetFocus(page);
         await endInputToggle.click();
         await page.waitForChanges();
-        expect(openSpy).toHaveReceivedEventTimes(8);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(true);
 
         await endInput.click();
         await page.waitForChanges();
-        expect(closeSpy).toHaveReceivedEventTimes(8);
 
         expect(await isCalendarVisible(calendar, "end")).toBe(false);
 

diff --git a/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx b/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx
@@ -999,6 +999,7 @@ export class InputDatePicker
       focusTrapEl: el,
       focusTrapOptions: {
         allowOutsideClick: true,
+        // Allow outside click and let the popover manager take care of closing the popover.
         clickOutsideDeactivates: false,
         initialFocus: false,
         setReturnFocus: false,

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
@@ -933,45 +933,37 @@ describe("calcite-input-time-picker", () => {
 
     it("toggles the time picker when clicked", async () => {
       let popover = await page.find("calcite-input-time-picker >>> calcite-popover");
-      const openSpy = await inputTimePicker.spyOnEvent("calciteInputTimePickerOpen");
-      const closeSpy = await inputTimePicker.spyOnEvent("calciteInputTimePickerClose");
 
       expect(await popover.isVisible()).toBe(false);
 
       await inputTimePicker.click();
       await page.waitForChanges();
-      expect(openSpy).toHaveReceivedEventTimes(1);
       popover = await page.find("calcite-input-time-picker >>> calcite-popover");
 
       expect(await popover.isVisible()).toBe(true);
 
       await inputTimePicker.click();
       await page.waitForChanges();
-      expect(closeSpy).toHaveReceivedEventTimes(1);
       popover = await page.find("calcite-input-time-picker >>> calcite-popover");
 
       expect(await popover.isVisible()).toBe(false);
     });
 
     it("toggles the time picker when using arrow down/escape key", async () => {
       let popover = await page.find("calcite-input-time-picker >>> calcite-popover");
-      const openSpy = await inputTimePicker.spyOnEvent("calciteInputTimePickerOpen");
-      const closeSpy = await inputTimePicker.spyOnEvent("calciteInputTimePickerClose");
 
       expect(await popover.isVisible()).toBe(false);
 
       await inputTimePicker.callMethod("setFocus");
       await page.waitForChanges();
       await page.keyboard.press("ArrowDown");
       await page.waitForChanges();
-      expect(openSpy).toHaveReceivedEventTimes(1);
       popover = await page.find("calcite-input-time-picker >>> calcite-popover");
 
       expect(await popover.isVisible()).toBe(true);
 
       await page.keyboard.press("Escape");
       await page.waitForChanges();
-      expect(closeSpy).toHaveReceivedEventTimes(1);
       popover = await page.find("calcite-input-time-picker >>> calcite-popover");
 
       expect(await popover.isVisible()).toBe(false);

diff --git a/packages/calcite-components/src/components/modal/modal.tsx b/packages/calcite-components/src/components/modal/modal.tsx
@@ -199,6 +199,7 @@ export class Modal
     connectMessages(this);
     connectFocusTrap(this, {
       focusTrapOptions: {
+        // Scrim has it's own close handler, allow it to take over.
         clickOutsideDeactivates: false,
         escapeDeactivates: (event) => {
           if (event.defaultPrevented || this.escapeDisabled) {

diff --git a/packages/calcite-components/src/components/popover/popover.tsx b/packages/calcite-components/src/components/popover/popover.tsx
@@ -306,10 +306,7 @@ export class Popover
           const outsideClick = !path.includes(this.el);
           const shouldCloseOnOutsideClick = this.autoClose && outsideClick;
 
-          return (
-            (shouldCloseOnOutsideClick && this.triggerDisabled) ||
-            (shouldCloseOnOutsideClick && isReferenceElementInPath)
-          );
+          return shouldCloseOnOutsideClick && (this.triggerDisabled || isReferenceElementInPath);
         },
       },
     });