From f3d3de56f9421cb5ccea927c540c859518d56337 Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Thu, 8 Dec 2022 09:07:19 +0300 Subject: [PATCH 1/2] Fix popover not closing on Esc and click outside --- src/components/VModal/VInputModal.vue | 1 + src/components/VModal/VModalContent.vue | 5 +++++ src/composables/use-dialog-content.ts | 1 + src/composables/use-focus-on-show.ts | 22 +++++++++++++--------- src/composables/use-popover-content.ts | 3 +++ 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/components/VModal/VInputModal.vue b/src/components/VModal/VInputModal.vue index 88b0451671..8e5509d6b9 100644 --- a/src/components/VModal/VInputModal.vue +++ b/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'], diff --git a/src/components/VModal/VModalContent.vue b/src/components/VModal/VModalContent.vue index 2f5c5fd8d7..6f428cef1b 100644 --- a/src/components/VModal/VModalContent.vue +++ b/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, 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, diff --git a/src/composables/use-dialog-content.ts b/src/composables/use-dialog-content.ts index 4f8212948e..2c89f11e30 100644 --- a/src/composables/use-dialog-content.ts +++ b/src/composables/use-dialog-content.ts @@ -11,6 +11,7 @@ type Props = { visibleRef: Ref autoFocusOnShowRef: Ref autoFocusOnHideRef: Ref + trapFocusRef: Ref triggerElementRef: Ref hideOnClickOutsideRef: Ref hideOnEscRef: Ref diff --git a/src/composables/use-focus-on-show.ts b/src/composables/use-focus-on-show.ts index c046dc7f16..7e53d4f1f7 100644 --- a/src/composables/use-focus-on-show.ts +++ b/src/composables/use-focus-on-show.ts @@ -15,6 +15,7 @@ type Props = { dialogRef: Ref visibleRef: Ref autoFocusOnShowRef: Ref + trapFocusRef: Ref initialFocusElementRef?: Ref } @@ -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() }) } ) diff --git a/src/composables/use-popover-content.ts b/src/composables/use-popover-content.ts index c2fc40f852..aa75497e10 100644 --- a/src/composables/use-popover-content.ts +++ b/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, From aebbf3059a3b1449ddc12afa0ab5e62379420420 Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Thu, 8 Dec 2022 13:55:26 +0300 Subject: [PATCH 2/2] Add popover e2e test --- test/playwright/e2e/homepage.spec.ts | 32 +++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/test/playwright/e2e/homepage.spec.ts b/test/playwright/e2e/homepage.spec.ts index 2a368e0344..4c4523c20a 100644 --- a/test/playwright/e2e/homepage.spec.ts +++ b/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) +})