Merge branch '1511-fix-click-outside-popover' of https://github.com/B…

…umbleB2na/ui-components into 1511-fix-click-outside-popover

_templates/web/src/pages/PopoverPage.svelte

@@ -3,8 +3,10 @@
 </script>
 
 <goa-popover>
-  <p>This is a popover</p>
-  It can be used for a number of different contexts.
+  <div tabindex="-1">
+    <p>This is a popover</p>
+    It can be used for a number of different contexts.
+  </div>
   <div slot="target">
     <goa-button type="secondary" size="compact">Click me</goa-button>
   </div>

libs/web-components/src/components/calendar/Calendar.svelte

@@ -304,6 +304,7 @@
   style={calculateMargin(mt, mr, mb, ml)}
   class:bordered={bordered === "true"}
   data-testid={testid}
+  tabindex="-1"
 >
   <goa-block mb="m">
     <goa-form-item label="Month" mt="0">

libs/web-components/src/components/dropdown/Dropdown.svelte

@@ -311,7 +311,7 @@
     setTimeout(async () => {
       syncFilteredOptions();
       _isMenuVisible = true;
-      // _inputEl?.focus();
+      _inputEl?.focus();
     }, 0);
   }
 
@@ -406,9 +406,9 @@
 
   function onClearIconKeyDown(e: KeyboardEvent) {
     if (e.key === "Enter" || e.key === " ") {
-      e.stopPropagation();
       reset();
       showMenu();
+      e.stopPropagation();
     }
   }
 
@@ -439,9 +439,7 @@
   }
 
   async function onChevronClick(e: Event) {
-    await tick();
     showMenu();
-    e.stopPropagation();
   }
 
   function onFocus(e: Event) {
@@ -830,7 +828,7 @@
 
   .dropdown-icon--arrow,
   .dropdown-icon--clear {
-    margin-right: var(--goa-space-s);
+    padding-right: var(--goa-space-s);
   }
 
   /* TODO: add indicator to when the reset button has focus state */

libs/web-components/src/components/popover/Popover.svelte

@@ -16,6 +16,7 @@
     getSlottedChildren,
     styles,
     toBoolean,
+    isElementContainedInSlotsRecursive,
   } from "../../common/utils";
   import type { Spacing } from "../../common/styling";
 
@@ -104,6 +105,18 @@
       ) as HTMLElement) || _targetEl;
   });
 
+  function handleFocusOut(e: FocusEvent) {
+    if (!_focusTrapEl) return;
+
+    const activeElement = e.relatedTarget;
+    const isFocusInPopover =
+      activeElement instanceof Element &&
+      isElementContainedInSlotsRecursive(_rootEl, activeElement);
+    if (!isFocusInPopover) {
+      closePopover();
+    }
+  }
+
   // Functions
 
   // Called on window popstate changes. This allows for clicking links within
@@ -139,11 +152,14 @@
   // Opens the popover and adds the required binding to the new slot element
   function openPopover() {
     if (_disabled) return;
+
+    _initFocusedEl.focus();
     (async () => {
       _open = true;
       await tick();
       _focusTrapEl.addEventListener("keydown", onFocusTrapEvent, true);
       _rootEl.dispatchEvent(new CustomEvent("_open", { composed: true }));
+      makeContentFocusable();
     })();
   }
 
@@ -157,6 +173,20 @@
     _rootEl.dispatchEvent(new CustomEvent("_close", { composed: true }));
   }
 
+  // Ensures that all immediate children of the popover content are included
+  // in event.relatedTarget that bubbles up to popover 'focusout' event handler
+  function makeContentFocusable() {
+    const content = $$slots.default;
+    if (content && _focusTrapEl) {
+      const immediateChildren = getSlottedChildren(_focusTrapEl);
+      immediateChildren.forEach((child) => {
+        if ((child as HTMLElement).tabIndex < 0) {
+          (child as HTMLElement).tabIndex = -1;
+        }
+      });
+    }
+  }
+
   function getBoundingClientRectWithMargins(
     el: Element,
   ): Omit<DOMRect, "toJSON"> {
@@ -225,6 +255,7 @@
 <div
   bind:this={_rootEl}
   data-testid={testid}
+  on:focusout={handleFocusOut}
   style={styles(
     _relative && "position: relative",
     height === "full" && "height: 100%;",
@@ -252,18 +283,12 @@
   </div>
 
   {#if _open}
-    <!-- svelte-ignore a11y-no-static-element-interactions -->
-    <!-- svelte-ignore a11y-click-events-have-key-events -->
-    <div
-      data-testid="popover-background"
-      class="popover-background"
-      on:click={closePopover}
-    />
     <div class="popover-container">
       <section
         bind:clientHeight={_sectionHeight}
         bind:this={_popoverEl}
         data-testid="popover-content"
+        tabindex="-1"
         class="popover-content"
         style={styles(
           style("width", width),
@@ -328,11 +353,4 @@
     list-style-type: none;
     line-height: 2rem;
   }
-
-  .popover-background {
-    cursor: default;
-    position: fixed;
-    z-index: 98;
-    inset: 0;
-  }
 </style>

libs/web-components/src/components/popover/PopoverWrapper.test.svelte

@@ -2,16 +2,14 @@
 
 <!-- Script -->
 <script lang="ts">
+  import Popover from "./Popover.svelte";
+
   export let content: string = "";
   export let targetTrigger: string = "";
 </script>
 
 <!-- HTML -->
-<goa-popover>
-  {#if content}
-    <div slot="content">{content}</div>
-  {/if}
-  {#if targetTrigger}
-    <div slot="target">{targetTrigger}</div>
-  {/if}
-</goa-popover>
+<Popover>
+  <div slot="target">{@html targetTrigger}</div>
+  {@html content}
+</Popover>

libs/web-components/src/components/popover/popover.spec.ts

@@ -1,35 +1,32 @@
-import { render, fireEvent } from "@testing-library/svelte";
+import { render } from "@testing-library/svelte";
+import userEvent, { UserEvent } from "@testing-library/user-event";
 import PopoverWrapper from "./PopoverWrapper.test.svelte";
 import Popover from "./Popover.svelte";
 import { it } from "vitest";
 
-it("should render", async () => {
-  const { container } = render(PopoverWrapper, {
-    content: "This is content",
-    targetTrigger: "Clik Action",
-  });
+let user: UserEvent;
 
-  const slotContent = container.querySelector("[slot=content]");
-  const slotTarget = container.querySelector("[slot=target]");
+beforeEach(() => {
+  user = userEvent.setup();
+});
 
-  expect(slotContent).toBeTruthy();
-  expect(slotTarget).toBeTruthy();
+it("should render without content by default", async () => {
+  const result = render(Popover);
 
-  expect(slotContent?.innerHTML).toContain(
-    "This is content",
-  );
-  expect(slotTarget?.innerHTML).toContain(
-    "Clik Action",
-  );
+  const slotContent = result.queryByTestId("popover-content");
+  const slotTarget = result.queryByTestId("popover-target");
+
+  expect(slotContent).toBeFalsy();
+  expect(slotTarget).toBeTruthy();
 });
 
 it("should open content when target is clicked", async () => {
-  const result = render(Popover, {minwidth: "8rem", maxwidth: "16rem"});
+  const result = render(Popover, { minwidth: "8rem", maxwidth: "16rem" });
   const target = result.queryByTestId("popover-target");
   expect(target).toBeTruthy();
 
   expect(result.queryByTestId("popover-content")).toBeNull();
-  target && await fireEvent.click(target);
+  target && (await user.click(target));
   const popOverContent = result.queryByTestId("popover-content");
   expect(popOverContent).toBeTruthy();
   const style = popOverContent?.getAttribute("style");
@@ -40,16 +37,60 @@ it("should open content when target is clicked", async () => {
 
 it("should close content when clicked outside the content container", async () => {
   const result = render(Popover);
-  const target = result.queryByTestId("popover-target");
 
+  const target = result.queryByTestId("popover-target");
   expect(target).toBeTruthy();
-  target && await fireEvent.click(target);
+  expect(result.queryByTestId("popover-content")).toBeFalsy();
 
-  const background = result.queryByTestId("popover-background");
-  expect(background).toBeTruthy();
+  // click on popover target
+  target && (await user.click(target));
   expect(result.queryByTestId("popover-content")).toBeTruthy();
-  expect(result.queryByTestId("popover-background")).toBeTruthy();
 
-  background && await fireEvent.click(background);
-  expect(result.queryByTestId("popover-content")).toBeNull();
+  // click outside of popover
+  await user.click(document.body);
+  expect(result.queryByTestId("popover-content")).toBeFalsy();
+});
+
+it("should not close content when clicked focusable target then click focusable content inside the content container", async () => {
+  // arrange to test that focusout event bubbles up to popover component
+  const result = render(PopoverWrapper, {
+    content: `<button data-testid="clickable-content">Click me</button>`,
+    targetTrigger: `<div><button data-testid="clickable-target">Click Action</button></div>`,
+  });
+
+  const target = result.queryByTestId("clickable-target");
+  expect(target).toBeTruthy();
+  expect(result.queryByTestId("clickable-content")).toBeFalsy();
+
+  // click on focusable element within popover target
+  target && (await user.click(target));
+  expect(result.queryByTestId("clickable-content")).toBeTruthy();
+
+  // click on focusable content inside popover content
+  const button = result.queryByTestId("clickable-content");
+  button && (await user.click(button));
+  expect(result.queryByTestId("clickable-content")).toBeTruthy();
+});
+
+it("should not close content when clicked non-focusable target then click non-focusable content inside the content container", async () => {
+  // arrange to test that focusout event bubbles up to popover component
+  const result = render(PopoverWrapper, {
+    content: `<span data-testid="non-focusable-content">Click me</span>`,
+    targetTrigger: `<span data-testid="non-focusable-target">Click Action</span>`,
+  });
+
+  const target = result.queryByTestId("non-focusable-target");
+  expect(target).toBeTruthy();
+  expect(result.queryByTestId("popover-content")).toBeFalsy();
+
+  // click on non-focusable element within popover target
+  target && (await user.click(target));
+  expect(await result.findByTestId("popover-content")).toBeTruthy();
+
+  // click on non-focusable content within popover content
+  const contentNonfocusableEl = result.queryByTestId("non-focusable-content");
+  expect(contentNonfocusableEl?.tabIndex).toBe(-1);
+
+  contentNonfocusableEl && (await user.click(contentNonfocusableEl));
+  expect(result.queryByTestId("popover-content")).toBeTruthy();
 });