From 9c33844de8c7f0795f32c441c26803b111eb505f Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Wed, 4 May 2022 17:21:56 +0300 Subject: [PATCH] Fix mobile menu modals not opening for the second time (#1386) * Fix mobile menu modals not opening for the second time * Fix e2e tests * Set the default hideOnClickOutside for modal to false * Make e2e header button test work for mobile and desktop --- src/components/VHeader/VHeaderFilter.vue | 8 ++--- src/components/VModal/VModalContent.vue | 2 +- src/composables/use-dialog-content.js | 2 +- test/playwright/e2e/mobile-menu.spec.ts | 43 ++++++++++++++++++++++ test/playwright/utils/navigation.ts | 45 ++++++++++++++++++++---- 5 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 test/playwright/e2e/mobile-menu.spec.ts diff --git a/src/components/VHeader/VHeaderFilter.vue b/src/components/VHeader/VHeaderFilter.vue index cfce790d5e..7f037ad18f 100644 --- a/src/components/VHeader/VHeaderFilter.vue +++ b/src/components/VHeader/VHeaderFilter.vue @@ -105,11 +105,12 @@ export default { open() } } + const mobileOptions = { visible: visibleRef, 'trigger-element': computed(() => nodeRef?.value?.firstChild), hide: close, - 'aria-label': i18n.t('header.filter-button.simple'), + 'aria-label': i18n.t('header.filter-button.simple').toString(), mode: 'mobile', } @@ -118,10 +119,7 @@ export default { visible: visibleRef, } /** - * @type { import('@nuxtjs/composition-api').Ref<{ - * 'trigger-element'?: import('@nuxtjs/composition-api').ComputedRef, - * hide?: () => {}, visible: import('@nuxtjs/composition-api').Ref, - * 'aria-label': [string], to?: string, mode?: string }> } + * @type { import('@nuxtjs/composition-api').Ref<{'trigger-element'?: import('@nuxtjs/composition-api').ComputedRef, hide?: () => void, visible: import('@nuxtjs/composition-api').Ref, 'aria-label'?: string, to?: string, mode?: string, hideOnClickOutside?: boolean }> } */ const options = ref(mobileOptions) onMounted(() => { diff --git a/src/components/VModal/VModalContent.vue b/src/components/VModal/VModalContent.vue index 2dd70c2982..ffe1084cb6 100644 --- a/src/components/VModal/VModalContent.vue +++ b/src/components/VModal/VModalContent.vue @@ -92,7 +92,7 @@ const VModalContent = defineComponent({ }, hideOnClickOutside: { type: Boolean, - default: true, + default: false, }, autoFocusOnShow: { type: Boolean, diff --git a/src/composables/use-dialog-content.js b/src/composables/use-dialog-content.js index 3cc6647e65..9817422f5e 100644 --- a/src/composables/use-dialog-content.js +++ b/src/composables/use-dialog-content.js @@ -19,7 +19,7 @@ import { useFocusOnBlur } from '~/composables/use-focus-on-blur' /** @typedef {import('./types').ToRefs & { emit: import('@nuxtjs/composition-api').SetupContext['emit']}} Props */ /** - * @param {Props} props + * @param {Props} params */ export function useDialogContent({ emit, ...props }) { const focusOnBlur = useFocusOnBlur(props) diff --git a/test/playwright/e2e/mobile-menu.spec.ts b/test/playwright/e2e/mobile-menu.spec.ts new file mode 100644 index 0000000000..b679bcb065 --- /dev/null +++ b/test/playwright/e2e/mobile-menu.spec.ts @@ -0,0 +1,43 @@ +import { test, expect } from '@playwright/test' + +import { + closeMobileMenu, + openFilters, + openMobileMenu, +} from '~~/test/playwright/utils/navigation' + +const mockUaString = + 'Mozilla/5.0 (Android 7.0; Mobile; rv:54.0) Gecko/54.0 Firefox/54.0' +const mobileFixture = { + viewport: { width: 640, height: 700 }, + userAgent: mockUaString, +} +test.use(mobileFixture) + +test('Can open filters menu on mobile at least twice', async ({ page }) => { + await page.goto('/search/?q=cat') + + await openFilters(page) + await expect(page.locator(`input[type="checkbox"]`)).toHaveCount(11, { + timeout: 100, + }) + await closeMobileMenu(page) + await expect(page.locator(`input[type="checkbox"]`)).toHaveCount(0, { + timeout: 100, + }) + + await openFilters(page) + await expect(page.locator(`input[type="checkbox"]`)).toHaveCount(11, { + timeout: 100, + }) +}) + +test('Can open mobile menu at least twice', async ({ page }) => { + await page.goto('/search/?q=cat') + await openMobileMenu(page) + await expect(page.locator('button', { hasText: 'Close' })).toBeVisible() + await closeMobileMenu(page) + await expect(page.locator('button', { hasText: 'Close' })).not.toBeVisible() + await openMobileMenu(page) + await expect(page.locator('button', { hasText: 'Close' })).toBeVisible() +}) diff --git a/test/playwright/utils/navigation.ts b/test/playwright/utils/navigation.ts index 1e1c65c2b2..23930ac443 100644 --- a/test/playwright/utils/navigation.ts +++ b/test/playwright/utils/navigation.ts @@ -1,15 +1,46 @@ import { expect, Page } from '@playwright/test' -export const openFilters = async (page: Page) => { - const filterButtonSelector = '[aria-controls="filters"]' - const isPressed = async () => - await page.getAttribute(filterButtonSelector, 'aria-pressed') - if ((await isPressed()) !== 'true') { - await page.click(filterButtonSelector) - expect(await isPressed()).toEqual('true') +const buttonSelectors = { + filter: '[aria-controls="filters"]', + contentSwitcher: '[aria-controls="content-switcher-modal"]', +} + +const isButtonPressed = async (page: Page, buttonSelector: string) => { + const viewportSize = page.viewportSize() + if (!viewportSize) { + return false + } + const pageWidth = viewportSize.width + if (pageWidth > 640) { + return await page.getAttribute(buttonSelector, 'aria-pressed') + } else { + return (await page.locator('button', { hasText: 'Close' }).isVisible()) + ? 'true' + : 'false' + } +} + +const openMenu = async (page: Page, button: 'filter' | 'contentSwitcher') => { + const selector = buttonSelectors[button] + const expectedValue = 'true' + if ((await isButtonPressed(page, selector)) !== expectedValue) { + await page.click(selector) + expect(await isButtonPressed(page, selector)).toEqual(expectedValue) } } +export const openFilters = async (page: Page) => { + await openMenu(page, 'filter') +} + +export const openMobileMenu = async (page: Page) => { + await openMenu(page, 'contentSwitcher') +} + +export const closeMobileMenu = async (page: Page) => { + await page.click('text=Close') +} + export const assertCheckboxStatus = async ( page: Page, label: string,