ensure that anchor links are still clickable

This is an issue in the Vue version (it just works in the React version)
but I added tests for them anyway.

While this solution "works" I am not 100% happy with it. Let me explain
what's happening here and why I am not that happy about it:

- For starters, the Vue `nextTick` is apparently too fast. So what we do
  is when we get the pointer up event, we will close the menu and
  re-focus the button. We ran this code in a `nextTick` so that we can
  ensure that we close the menu *after* all the click events are
  finished. However because this is too fast, the menu is already closed
  and the anchor link is already unmounted and thus not clickable
  anymore. So instead we use a double requestAnimationFrame (to mimick a
  `nextFrame` as seen in the `disposables` from the React code). This
  works, but a bit messy, oh well.
- The next reason why I am not that happy is because I can't reproduce
  it in JSDOM (Jest tests). When you *click* a link in JSDOM it doesn't
  update the `window.location.hash` or `window.location.href`. To mimick
  that behaviour I put a `@click` event on the anchor to verify that we
  actually clicked it. However this already works, even before the
  "fix". So I left a TODO in there so that we can hopefully fix the
  test, so that we _can_ reproduce this behaviour.

Fixes: #14
This commit is contained in:
Robin Malfait
2020-09-28 11:47:49 +02:00
parent 93f315b066
commit e9fd05e06d
4 changed files with 65 additions and 29 deletions
@@ -593,11 +593,14 @@ describe('Keyboard interactions', () => {
it(
'should be possible to close the menu with Enter and invoke the active menu item',
suppressConsoleLogs(async () => {
const clickHandler = jest.fn()
render(
<Menu>
<Menu.Button>Trigger</Menu.Button>
<Menu.Items>
<Menu.Item as="a">Item A</Menu.Item>
<Menu.Item as="a" onClick={clickHandler}>
Item A
</Menu.Item>
<Menu.Item as="a">Item B</Menu.Item>
<Menu.Item as="a">Item C</Menu.Item>
</Menu.Items>
@@ -626,6 +629,9 @@ describe('Keyboard interactions', () => {
// Verify it is closed
assertMenuButton(getMenuButton(), { state: MenuButtonState.Closed })
assertMenu(getMenu(), { state: MenuState.Closed })
// Verify the "click" went through on the `a` tag
expect(clickHandler).toHaveBeenCalled()
})
)
})
@@ -2458,12 +2464,15 @@ describe('Mouse interactions', () => {
it(
'should be possible to click a menu item, which closes the menu',
suppressConsoleLogs(async () => {
const clickHandler = jest.fn()
render(
<Menu>
<Menu.Button>Trigger</Menu.Button>
<Menu.Items>
<Menu.Item as="a">alice</Menu.Item>
<Menu.Item as="a">bob</Menu.Item>
<Menu.Item as="a" onClick={clickHandler}>
bob
</Menu.Item>
<Menu.Item as="a">charlie</Menu.Item>
</Menu.Items>
</Menu>
@@ -2478,6 +2487,7 @@ describe('Mouse interactions', () => {
// We should be able to click the first item
await click(items[1])
assertMenu(getMenu(), { state: MenuState.Closed })
expect(clickHandler).toHaveBeenCalled()
})
)
@@ -562,8 +562,7 @@ function Item<TTag extends React.ElementType = typeof DEFAULT_ITEM_TAG>(
const handlePointerUp = React.useCallback(
(event: React.PointerEvent<HTMLElement>) => {
if (disabled) return
event.preventDefault()
if (disabled) return event.preventDefault()
dispatch({ type: ActionTypes.CloseMenu })
d.nextFrame(() => state.buttonRef.current?.focus())
},
@@ -1,4 +1,4 @@
import { defineComponent, h } from 'vue'
import { defineComponent, h, nextTick } from 'vue'
import { render } from '../../test-utils/vue-testing-library'
import { Menu, MenuButton, MenuItems, MenuItem } from './menu'
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
@@ -61,6 +61,13 @@ function getMenuItems(): HTMLElement[] {
return Array.from(document.querySelectorAll('[role="menuitem"]'))
}
beforeAll(() => {
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(nextTick as any)
jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(jest.fn())
})
afterAll(() => jest.restoreAllMocks())
describe('Safe guards', () => {
it.each([
['MenuButton', MenuButton],
@@ -771,16 +778,20 @@ describe('Keyboard interactions', () => {
})
it('should be possible to close the menu with Enter and invoke the active menu item', async () => {
renderTemplate(`
<Menu>
<MenuButton>Trigger</MenuButton>
<MenuItems>
<MenuItem as="a">Item A</MenuItem>
<MenuItem as="a">Item B</MenuItem>
<MenuItem as="a">Item C</MenuItem>
</MenuItems>
</Menu>
`)
const clickHandler = jest.fn()
renderTemplate({
template: `
<Menu>
<MenuButton>Trigger</MenuButton>
<MenuItems>
<MenuItem as="a" @click="clickHandler">Item A</MenuItem>
<MenuItem as="a">Item B</MenuItem>
<MenuItem as="a">Item C</MenuItem>
</MenuItems>
</Menu>
`,
setup: () => ({ clickHandler }),
})
assertMenuButton(getMenuButton(), {
state: MenuButtonState.Closed,
@@ -804,6 +815,9 @@ describe('Keyboard interactions', () => {
// Verify it is closed
assertMenuButton(getMenuButton(), { state: MenuButtonState.Closed })
assertMenu(getMenu(), { state: MenuState.Closed })
// Verify the "click" went through on the `a` tag
expect(clickHandler).toHaveBeenCalled()
})
})
@@ -2380,16 +2394,20 @@ describe('Mouse interactions', () => {
})
it('should be possible to click a menu item, which closes the menu', async () => {
renderTemplate(`
<Menu>
<MenuButton>Trigger</MenuButton>
<MenuItems>
<MenuItem as="a">alice</MenuItem>
<MenuItem as="a">bob</MenuItem>
<MenuItem as="a">charlie</MenuItem>
</MenuItems>
</Menu>
`)
const clickHandler = jest.fn()
renderTemplate({
template: `
<Menu>
<MenuButton>Trigger</MenuButton>
<MenuItems>
<MenuItem as="a">alice</MenuItem>
<MenuItem as="a" @click="clickHandler">bob</MenuItem>
<MenuItem as="a">charlie</MenuItem>
</MenuItems>
</Menu>
`,
setup: () => ({ clickHandler }),
})
// Open menu
await click(getMenuButton())
@@ -2400,6 +2418,7 @@ describe('Mouse interactions', () => {
// We should be able to click the first item
await click(items[1])
assertMenu(getMenu(), { state: MenuState.Closed })
expect(clickHandler).toHaveBeenCalled()
})
it('should be possible to click a menu item, which closes the menu and invokes the @click handler', async () => {
@@ -445,10 +445,18 @@ export const MenuItem = defineComponent({
}
function handlePointerUp(event: PointerEvent) {
if (disabled) return
event.preventDefault()
api.closeMenu()
nextTick(() => api.buttonRef.value?.focus())
if (disabled) return event.preventDefault()
// Turns out that we can't use nextTick here. Even if we do, the `handleClick` would *not* be
// called because the closeMenu() update is *too fast* and the tree gets unmounted before it
// bubbles up. So instead of nextTick, we use the good old double requestAnimationFrame to
// wait for a "nextFrame".
requestAnimationFrame(() => {
requestAnimationFrame(() => {
api.closeMenu()
api.buttonRef.value?.focus()
})
})
}
function handleClick(event: MouseEvent) {