From fd7bab70bf59f618de06c2f7b6cadd84204c6d0a Mon Sep 17 00:00:00 2001 From: Marcus Herrmann Date: Sun, 2 Feb 2020 15:50:44 +0100 Subject: [PATCH 1/6] Start testing focus state on Dropdown --- packages/dropdown/tests/unit/Dropdown.spec.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/dropdown/tests/unit/Dropdown.spec.js b/packages/dropdown/tests/unit/Dropdown.spec.js index 5b204ca..45d350b 100644 --- a/packages/dropdown/tests/unit/Dropdown.spec.js +++ b/packages/dropdown/tests/unit/Dropdown.spec.js @@ -19,13 +19,16 @@ describe('Dropdown', () => { items: ' ' }, - localVue + localVue, + attachToDocument: true }) button = wrapper.find('button') }) describe('Events', () => { + let firstMenuItem + it('@click - open and close menu', () => { button.trigger('click') expect(wrapper.vm.$refs.menu).toBeDefined() @@ -48,6 +51,13 @@ describe('Dropdown', () => { button.trigger('keydown.up') expect(wrapper.vm.$refs.menu).toBeUndefined() }) + + it('@keydown.down - focuses first menu item', async () => { + button.trigger('keydown.down') + await wrapper.vm.$nextTick() + firstMenuItem = wrapper.findAll('[role="menuitem"]').at(0) + expect(firstMenuItem.element).toBe(document.activeElement) + }) }) describe('Menu toggle button', () => { From 2e2f4ef3565e765baf82a89c135b1281503707ac Mon Sep 17 00:00:00 2001 From: Oscar Date: Fri, 6 Mar 2020 20:19:57 +0100 Subject: [PATCH 2/6] fix(dropdown): close menu when focus moves outside --- packages/dropdown/src/index.vue | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/dropdown/src/index.vue b/packages/dropdown/src/index.vue index 4d528af..9a55077 100644 --- a/packages/dropdown/src/index.vue +++ b/packages/dropdown/src/index.vue @@ -66,11 +66,11 @@ export default { this.checkForAccessibleName() - document.addEventListener('keydown', this.handleGlobalKeyDown) + document.addEventListener('focusin', this.handleGlobalFocus) document.documentElement.addEventListener('click', this.handleGlobalClick) }, beforeDestroy() { - document.removeEventListener('keydown', this.handleGlobalKeyDown) + document.removeEventListener('focusin', this.handleGlobalFocus) document.documentElement.removeEventListener( 'click', this.handleGlobalClick @@ -99,10 +99,10 @@ export default { this.open() } }, - handleGlobalKeyDown(evt) { - if (evt.keyCode === 9 && isOutsidePath(evt, this.$el)) { - this.close(false) - } + handleGlobalFocus() { + if (this.$el.contains(document.activeElement)) return + + this.close(false) }, handleGlobalClick(evt) { if (isOutsidePath(evt, this.$el)) { From 1c06de659033d677f7d0de0b3005b5b42a191978 Mon Sep 17 00:00:00 2001 From: Oscar Date: Fri, 6 Mar 2020 20:43:08 +0100 Subject: [PATCH 3/6] fix(dropdown): query selector needs to get `menuitemcheckbox` and `menuitemradio` --- packages/dropdown/src/index.vue | 2 +- packages/dropdown/tests/unit/Dropdown.spec.js | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/dropdown/src/index.vue b/packages/dropdown/src/index.vue index 9a55077..7f2c804 100644 --- a/packages/dropdown/src/index.vue +++ b/packages/dropdown/src/index.vue @@ -117,7 +117,7 @@ export default { this.$nextTick().then(() => { this.items = Array.from( - this.$refs.menu.querySelectorAll('[role=menuitem]:not([disabled])') + this.$refs.menu.querySelectorAll('[role^="menuitem"]:not([disabled])') ) this.items.forEach(button => { diff --git a/packages/dropdown/tests/unit/Dropdown.spec.js b/packages/dropdown/tests/unit/Dropdown.spec.js index 45d350b..f279550 100644 --- a/packages/dropdown/tests/unit/Dropdown.spec.js +++ b/packages/dropdown/tests/unit/Dropdown.spec.js @@ -150,3 +150,59 @@ describe('Dropdown', () => { }) }) }) + +describe('Dropdown – menuitemcheckbox', () => { + let wrapper + let button + + beforeEach(() => { + wrapper = shallowMount(Dropdown, { + slots: { + 'button-text': 'Options', + items: ` + + ` + }, + localVue, + attachToDocument: true + }) + + button = wrapper.find('button') + }) + + it('detects `menuitemradio` button', async () => { + button.trigger('click') + + await wrapper.vm.$nextTick() + + expect(wrapper.vm.items).toHaveLength(2) + }) +}) + +describe('Dropdown – menuitemradio', () => { + let wrapper + let button + + beforeEach(() => { + wrapper = shallowMount(Dropdown, { + slots: { + 'button-text': 'Options', + items: ` + + ` + }, + localVue, + attachToDocument: true + }) + + button = wrapper.find('button') + }) + + it('detects `menuitemradio` button', async () => { + button.trigger('click') + + await wrapper.vm.$nextTick() + + expect(wrapper.vm.items).toHaveLength(2) + }) +}) From 7bb8e12b83f17fe08924ac908db6c6c72552c3b3 Mon Sep 17 00:00:00 2001 From: Oscar Date: Fri, 6 Mar 2020 20:44:00 +0100 Subject: [PATCH 4/6] feat(dropdown): test arrow management, add stories --- packages/dropdown/tests/dropdown.stories.js | 19 +++++- packages/dropdown/tests/unit/Dropdown.spec.js | 62 +++++++++++++++---- 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/packages/dropdown/tests/dropdown.stories.js b/packages/dropdown/tests/dropdown.stories.js index 61b446a..b6167b3 100644 --- a/packages/dropdown/tests/dropdown.stories.js +++ b/packages/dropdown/tests/dropdown.stories.js @@ -18,9 +18,26 @@ export const Basic = () => ({ ` }) +export const TabAway = () => ({ + components: { TournantDropdown }, + template: ` +
+

Above dropdown with a placeholder link.

+ + + ${items} + +

Some more content underneath the item.

+

And another paragraph with a link. +

+ ` +}) + export const Positioning = () => ({ components: { TournantDropdown }, - template: ` + template: ` diff --git a/packages/dropdown/tests/unit/Dropdown.spec.js b/packages/dropdown/tests/unit/Dropdown.spec.js index f279550..ae6c64b 100644 --- a/packages/dropdown/tests/unit/Dropdown.spec.js +++ b/packages/dropdown/tests/unit/Dropdown.spec.js @@ -1,23 +1,21 @@ -// global.console = { warn: jest.fn() } - import { shallowMount, createLocalVue } from '@vue/test-utils' import Dropdown from '@p/dropdown/src/index.vue' const localVue = createLocalVue() -localVue.directive('clickaway', {}) - describe('Dropdown', () => { let wrapper let button + let menu beforeEach(() => { wrapper = shallowMount(Dropdown, { slots: { 'button-text': 'Options', - items: - ' ' + items: ` + + ` }, localVue, attachToDocument: true @@ -29,35 +27,75 @@ describe('Dropdown', () => { describe('Events', () => { let firstMenuItem - it('@click - open and close menu', () => { + it('@click - open and close menu', async () => { button.trigger('click') + await wrapper.vm.$nextTick() expect(wrapper.vm.$refs.menu).toBeDefined() button.trigger('click') - + await wrapper.vm.$nextTick() expect(wrapper.vm.isVisible).toBeFalsy() }) - it('@keydown.down - open menu', () => { + it('@keydown.down - open menu', async () => { button.trigger('keydown.down') + await wrapper.vm.$nextTick() + expect(wrapper.vm.$refs.menu).toBeDefined() }) it('@keydown.down > @keydown.up - opens and closes the menu', async () => { await button.trigger('keydown.down') + await wrapper.vm.$nextTick() expect(wrapper.vm.$refs.menu).toBeDefined() button.trigger('keydown.up') + await wrapper.vm.$nextTick() expect(wrapper.vm.$refs.menu).toBeUndefined() }) it('@keydown.down - focuses first menu item', async () => { button.trigger('keydown.down') + await wrapper.vm.$nextTick() + + firstMenuItem = wrapper.findAll('[role="menuitem"]').at(0) + + expect(firstMenuItem.element).toBe(document.activeElement) + }) + + it('@keydown.down - loops through items', async () => { + // open + button.trigger('keydown.down') + + const { length } = wrapper.findAll('[role="menuitem"]') + + for (let i = 0; i < length; i++) { + button.trigger('keydown.down') + } + firstMenuItem = wrapper.findAll('[role="menuitem"]').at(0) + + await wrapper.vm.$nextTick() + expect(firstMenuItem.element).toBe(document.activeElement) }) + + it('@keydown.up - focusses last item if at the beginning', async () => { + // open + button.trigger('keydown.down') + + await wrapper.vm.$nextTick() + + menu = wrapper.find('[role="menu"]') + menu.trigger('keydown.up') + + const { length } = wrapper.findAll('[role="menuitem"]') + const lastItem = wrapper.findAll('[role="menuitem"]').at(length - 1) + + expect(lastItem.element).toBe(document.activeElement) + }) }) describe('Menu toggle button', () => { @@ -69,13 +107,13 @@ describe('Dropdown', () => { expect(button.attributes('aria-haspopup')).toBeTruthy() }) - it('changes its `aria-expanded` attribute', () => { + it('changes its `aria-expanded` attribute', async () => { button.trigger('click') - + await wrapper.vm.$nextTick() expect(button.attributes('aria-expanded')).toBe('true') button.trigger('click') - + await wrapper.vm.$nextTick() expect(button.attributes('aria-expanded')).toBe('false') }) }) From 5daf50c7baf5584c42b8be85e77643f5ee4ce113 Mon Sep 17 00:00:00 2001 From: Oscar Date: Fri, 6 Mar 2020 20:44:17 +0100 Subject: [PATCH 5/6] docs: update comments --- helper/isOutsidePath.js | 2 +- packages/dropdown/src/index.vue | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/helper/isOutsidePath.js b/helper/isOutsidePath.js index 88bbf48..98757f3 100644 --- a/helper/isOutsidePath.js +++ b/helper/isOutsidePath.js @@ -2,7 +2,7 @@ * Check if the target of an event is outside of an element * * @param {Event} evt - * @param {Node} element + * @param {HTMLElement} element * @returns */ const isOutsidePath = (evt, element) => { diff --git a/packages/dropdown/src/index.vue b/packages/dropdown/src/index.vue index 7f2c804..5bf461b 100644 --- a/packages/dropdown/src/index.vue +++ b/packages/dropdown/src/index.vue @@ -128,7 +128,7 @@ export default { }) }, close(setFocus = true) { - // Method will be called from the `clickaway` directive on every component instance + // Multiple instances might have added event listeners // Limit work and ensure correct handling of focus by having an additional check for visibility if (this.isVisible) { this.isVisible = false From 09d7b43b883a59e79b114b1f9c9dba429592d346 Mon Sep 17 00:00:00 2001 From: Oscar Date: Sat, 7 Mar 2020 11:37:34 +0100 Subject: [PATCH 6/6] feat(dropdown): test handleGlobalClick --- packages/dropdown/tests/unit/Dropdown.spec.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/dropdown/tests/unit/Dropdown.spec.js b/packages/dropdown/tests/unit/Dropdown.spec.js index ae6c64b..a8cb8d3 100644 --- a/packages/dropdown/tests/unit/Dropdown.spec.js +++ b/packages/dropdown/tests/unit/Dropdown.spec.js @@ -2,6 +2,15 @@ import { shallowMount, createLocalVue } from '@vue/test-utils' import Dropdown from '@p/dropdown/src/index.vue' +document.body.innerHTML = ` +
+ + + + test +
+` + const localVue = createLocalVue() describe('Dropdown', () => { @@ -96,6 +105,18 @@ describe('Dropdown', () => { expect(lastItem.element).toBe(document.activeElement) }) + + it('closes the menu if click happens outside of it', async () => { + button.trigger('keydown.down') + + await wrapper.vm.$nextTick() + + document.getElementById('test-link').click() + + await wrapper.vm.$nextTick() + + expect(wrapper.vm.$refs.menu).toBeUndefined() + }) }) describe('Menu toggle button', () => {