Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Fix popover not closing on Escape or click outside #2036

Merged
merged 2 commits into from
Dec 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/VModal/VInputModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
5 changes: 5 additions & 0 deletions src/components/VModal/VModalContent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/composables/use-dialog-content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
22 changes: 13 additions & 9 deletions src/composables/use-focus-on-show.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type Props = {
dialogRef: Ref<HTMLElement | null>
visibleRef: Ref<boolean>
autoFocusOnShowRef: Ref<boolean>
trapFocusRef: Ref<boolean>
initialFocusElementRef?: Ref<HTMLElement | null>
}

Expand All @@ -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(
[
Expand All @@ -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
Expand All @@ -70,7 +74,7 @@ export const useFocusOnShow = ({
}
}
}
activateFocusTrap()
if (activateFocusTrap) activateFocusTrap()
})
}
)
Expand Down
3 changes: 3 additions & 0 deletions src/composables/use-popover-content.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { ref } from '@nuxtjs/composition-api'

import { PopoverContentProps, usePopper } from '~/composables/use-popper'

import { useDialogContent } from '~/composables/use-dialog-content'
Expand All @@ -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,
Expand Down
32 changes: 31 additions & 1 deletion test/playwright/e2e/homepage.spec.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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)
})