Fix popover not closing on Escape or click outside (#2036)

* Fix popover not closing on Esc and click outside

* Add popover e2e test

src/components/VModal/VInputModal.vue

@@ -115,6 +115,7 @@ export default defineComponent({
       autoFocusOnHideRef: ref(false),
       triggerElementRef: ref(null),
       hideOnClickOutsideRef: ref(false),
+      trapFocusRef: ref(true),
       hideRef: ref(close),
       hideOnEscRef: ref(true),
       emit: emit as SetupContext['emit'],

src/components/VModal/VModalContent.vue

@@ -117,6 +117,10 @@ export default defineComponent({
       type: Boolean,
       default: true,
     },
+    trapFocus: {
+      type: Boolean,
+      default: true,
+    },
     triggerElement: {
       type: (process.server ? Object : HTMLElement) as PropType<HTMLElement>,
       default: null,
@@ -158,6 +162,7 @@ export default defineComponent({
       visibleRef: propsRefs.visible,
       autoFocusOnShowRef: propsRefs.autoFocusOnShow,
       autoFocusOnHideRef: propsRefs.autoFocusOnHide,
+      trapFocusRef: propsRefs.trapFocus,
       triggerElementRef: propsRefs.triggerElement,
       hideOnClickOutsideRef: propsRefs.hideOnClickOutside,
       hideRef: propsRefs.hide,

src/composables/use-dialog-content.ts

@@ -11,6 +11,7 @@ type Props = {
   visibleRef: Ref<boolean>
   autoFocusOnShowRef: Ref<boolean>
   autoFocusOnHideRef: Ref<boolean>
+  trapFocusRef: Ref<boolean>
   triggerElementRef: Ref<HTMLElement | null>
   hideOnClickOutsideRef: Ref<boolean>
   hideOnEscRef: Ref<boolean>

src/composables/use-focus-on-show.ts

@@ -15,6 +15,7 @@ type Props = {
   dialogRef: Ref<HTMLElement | null>
   visibleRef: Ref<boolean>
   autoFocusOnShowRef: Ref<boolean>
+  trapFocusRef: Ref<boolean>
   initialFocusElementRef?: Ref<HTMLElement | null>
 }
 
@@ -25,16 +26,19 @@ export const useFocusOnShow = ({
   dialogRef,
   visibleRef,
   autoFocusOnShowRef,
+  trapFocusRef,
   initialFocusElementRef = ref(null),
 }: Props) => {
   const { activate: activateFocusTrap, deactivate: deactivateFocusTrap } =
-    useFocusTrap(dialogRef, {
-      // Prevent FocusTrap from trying to focus the first element.
-      // We already do that in a more flexible, adaptive way in our Dialog composables.
-      initialFocus: false,
-      // if set to true, focus-trap prevents the default for the keyboard event, and we cannot handle it in our composables.
-      escapeDeactivates: false,
-    })
+    trapFocusRef.value
+      ? useFocusTrap(dialogRef, {
+          // Prevent FocusTrap from trying to focus the first element.
+          // We already do that in a more flexible, adaptive way in our Dialog composables.
+          initialFocus: false,
+          // if set to true, focus-trap prevents the default for the keyboard event, and we cannot handle it in our composables.
+          escapeDeactivates: false,
+        })
+      : { activate: undefined, deactivate: undefined }
 
   watch(
     [
@@ -45,7 +49,7 @@ export const useFocusOnShow = ({
     ] as const,
     ([dialog, visible, autoFocusOnShow, initialFocusElement]) => {
       if (!dialog || !visible) {
-        deactivateFocusTrap()
+        if (deactivateFocusTrap) deactivateFocusTrap()
         return
       }
       if (!dialog || !visible || !autoFocusOnShow) return
@@ -70,7 +74,7 @@ export const useFocusOnShow = ({
             }
           }
         }
-        activateFocusTrap()
+        if (activateFocusTrap) activateFocusTrap()
       })
     }
   )

src/composables/use-popover-content.ts

@@ -1,3 +1,5 @@
+import { ref } from '@nuxtjs/composition-api'
+
 import { PopoverContentProps, usePopper } from '~/composables/use-popper'
 
 import { useDialogContent } from '~/composables/use-dialog-content'
@@ -21,6 +23,7 @@ export function usePopoverContent({
     visibleRef: popoverPropsRefs.visible,
     autoFocusOnShowRef: popoverPropsRefs.autoFocusOnShow,
     autoFocusOnHideRef: popoverPropsRefs.autoFocusOnHide,
+    trapFocusRef: ref(false),
     triggerElementRef: popoverPropsRefs.triggerElement,
     hideOnClickOutsideRef: popoverPropsRefs.hideOnClickOutside,
     hideRef: popoverPropsRefs.hide,

test/playwright/e2e/homepage.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect } from '@playwright/test'
+import { test, expect, Page } from '@playwright/test'
 
 import { mockProviderApis } from '~~/test/playwright/utils/route'
 import {
@@ -24,3 +24,33 @@ for (const searchType of supportedSearchTypes) {
     await expect(page).toHaveURL(expectedUrl)
   })
 }
+
+const popoverIsVisible = async (page: Page) =>
+  await expect(page.locator('.popover-content')).toBeVisible()
+const popoverIsNotVisible = async (page: Page) =>
+  await expect(page.locator('.popover-content')).not.toBeVisible()
+const clickPopoverButton = async (page: Page) =>
+  await page.click('button[aria-label="All content"]')
+
+test('can close the search type popover by clicking outside', async ({
+  page,
+}) => {
+  await page.goto('/')
+  await clickPopoverButton(page)
+  await popoverIsVisible(page)
+
+  await page.mouse.click(1, 1)
+  await popoverIsNotVisible(page)
+})
+
+test('can close the search type popover by pressing Escape', async ({
+  page,
+}) => {
+  await page.goto('/')
+  await clickPopoverButton(page)
+  await popoverIsVisible(page)
+
+  await page.keyboard.press('Escape')
+
+  await popoverIsNotVisible(page)
+})