From 44dff6ba72445716ab8c51ca6e3227120e25e8bf Mon Sep 17 00:00:00 2001 From: Wes Cossick Date: Thu, 19 Dec 2019 14:39:51 -0600 Subject: [PATCH 1/6] Listen for clicks outside of the menu and close the dropdown --- src/use-dropdown-menu.ts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/use-dropdown-menu.ts b/src/use-dropdown-menu.ts index f58c2d8..fa93447 100644 --- a/src/use-dropdown-menu.ts +++ b/src/use-dropdown-menu.ts @@ -39,6 +39,41 @@ export default function useDropdownMenu(itemCount: number) { if (isOpen) { moveFocus(0); } + + + // This function is designed to handle every click + const handleEveryClick = (event: MouseEvent) => { + // Ignore if the menu isn't open + if (!isOpen) { + return; + } + + // Make this happen asynchronously + setTimeout(() => { + // Type guard + if (!(event.target instanceof Element)) { + return; + } + + + // Ignore if we're clicking inside the menu + if (event.target.closest('[role="menu"]')) { + return; + } + + + // Hide dropdown + setIsOpen(false); + }, 10); + }; + + + // Add listener + document.addEventListener('click', handleEveryClick); + + + // Return function to remove listener + return () => document.removeEventListener('click', handleEveryClick); }, [isOpen]); // Create a handler function for the button's clicks and keyboard events From 20f8b27d8f1c67f5280ecd04d4d4209fb3807e67 Mon Sep 17 00:00:00 2001 From: Wes Cossick Date: Thu, 19 Dec 2019 14:40:19 -0600 Subject: [PATCH 2/6] Add some extra tests --- test/use-dropdown-menu.test.tsx | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/use-dropdown-menu.test.tsx b/test/use-dropdown-menu.test.tsx index fb84078..47ea6a2 100644 --- a/test/use-dropdown-menu.test.tsx +++ b/test/use-dropdown-menu.test.tsx @@ -119,3 +119,24 @@ it('moves the focus to the menu button after pressing escape while focused on a firstMenuItem.simulate('keydown', { key: 'Escape' }); expect(document.activeElement?.id).toBe('menu-button'); }); + +it('opens the menu after clicking the button', () => { + const component = mount(); + const button = component.find('#menu-button'); + const span = component.find('#is-open-indicator'); + + button.simulate('click'); + + expect(span.text()).toBe('true'); +}); + +it('closes the menu after clicking the button when the menu is open', () => { + const component = mount(); + const button = component.find('#menu-button'); + const span = component.find('#is-open-indicator'); + + button.simulate('click'); + button.simulate('click'); + + expect(span.text()).toBe('false'); +}); From fee85587d205137134eacc24f9711d05e2803bbd Mon Sep 17 00:00:00 2001 From: Wes Cossick Date: Thu, 19 Dec 2019 14:40:29 -0600 Subject: [PATCH 3/6] Test the new behavior and tighten other tests --- test/puppeteer/demo.test.ts | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/test/puppeteer/demo.test.ts b/test/puppeteer/demo.test.ts index 1e2a060..b91b84c 100644 --- a/test/puppeteer/demo.test.ts +++ b/test/puppeteer/demo.test.ts @@ -6,6 +6,8 @@ const { keyboard } = page; // Helper functions used in multiple tests const currentFocusID = () => page.evaluate(() => document.activeElement.id); +const menuOpen = () => page.waitForSelector('#menu', { visible: true }); +const menuClosed = () => page.waitForSelector('#menu', { hidden: true }); // Tests beforeEach(async () => { @@ -21,31 +23,46 @@ it('has the correct page title', async () => { it('focuses on the first menu item when the enter key is pressed', async () => { await page.focus('#menu-button'); await keyboard.down('Enter'); + await menuOpen(); expect(await currentFocusID()).toBe('menu-item-1'); }); it('focuses on the menu button after pressing escape', async () => { - await page.focus('#menu-button'); - await keyboard.down('Enter'); + await page.click('#menu-button'); + await menuOpen(); await keyboard.down('Escape'); + await menuClosed(); expect(await currentFocusID()).toBe('menu-button'); }); it('focuses on the next item in the tab order after pressing tab', async () => { - await page.focus('#menu-button'); - await keyboard.down('Enter'); + await page.click('#menu-button'); + await menuOpen(); await keyboard.down('Tab'); + await menuClosed(); expect(await currentFocusID()).toBe('first-footer-link'); }); it('focuses on the previous item in the tab order after pressing shift-tab', async () => { - await page.focus('#menu-button'); - await keyboard.down('Enter'); + await page.click('#menu-button'); + await menuOpen(); await keyboard.down('Shift'); await keyboard.down('Tab'); + await menuClosed(); expect(await currentFocusID()).toBe('menu-button'); }); + +it('closes the menu if you click outside of it', async () => { + await page.click('#menu-button'); + await menuOpen(); + await page.click('body'); + await menuClosed(); + + // menuClosed() will time out if it doesn't actually close + + expect(true).toBe(true); +}); From 369c4d1d6e3703a6529534157b32273b1903dbb5 Mon Sep 17 00:00:00 2001 From: Wes Cossick Date: Thu, 19 Dec 2019 14:47:51 -0600 Subject: [PATCH 4/6] More thorough testing of new behavior --- test/puppeteer/demo.test.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/test/puppeteer/demo.test.ts b/test/puppeteer/demo.test.ts index b91b84c..7877003 100644 --- a/test/puppeteer/demo.test.ts +++ b/test/puppeteer/demo.test.ts @@ -31,6 +31,7 @@ it('focuses on the first menu item when the enter key is pressed', async () => { it('focuses on the menu button after pressing escape', async () => { await page.click('#menu-button'); await menuOpen(); + await keyboard.down('Escape'); await menuClosed(); @@ -40,6 +41,7 @@ it('focuses on the menu button after pressing escape', async () => { it('focuses on the next item in the tab order after pressing tab', async () => { await page.click('#menu-button'); await menuOpen(); + await keyboard.down('Tab'); await menuClosed(); @@ -49,6 +51,7 @@ it('focuses on the next item in the tab order after pressing tab', async () => { it('focuses on the previous item in the tab order after pressing shift-tab', async () => { await page.click('#menu-button'); await menuOpen(); + await keyboard.down('Shift'); await keyboard.down('Tab'); await menuClosed(); @@ -59,10 +62,24 @@ it('focuses on the previous item in the tab order after pressing shift-tab', asy it('closes the menu if you click outside of it', async () => { await page.click('#menu-button'); await menuOpen(); + await page.click('body'); - await menuClosed(); + await menuClosed(); // times out if menu doesn't close + + expect(true).toBe(true); +}); + +it('leaves the menu open if you click inside of it', async () => { + await page.click('#menu-button'); + await menuOpen(); + + await page.click('#menu-item-1'); + await new Promise(resolve => setTimeout(resolve, 1000)); // visibility: hidden is delayed via CSS + await menuOpen(); // times out if menu closes - // menuClosed() will time out if it doesn't actually close + await page.click('#menu'); + await new Promise(resolve => setTimeout(resolve, 1000)); // visibility: hidden is delayed via CSS + await menuOpen(); // times out if menu closes expect(true).toBe(true); }); From e0860ad7f4e5f11f2f058ba9fc0b2a65a5121170 Mon Sep 17 00:00:00 2001 From: Wes Cossick Date: Thu, 19 Dec 2019 14:49:27 -0600 Subject: [PATCH 5/6] Note the new behavior in the README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 26024f3..eaf2b20 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![Greenkeeper badge](https://badges.greenkeeper.io/sparksuite/react-accessible-dropdown-menu-hook.svg)](https://greenkeeper.io/) -This Hook handles all the accessibility logic when building a dropdown menu, dropdown button, etc., and leaves the design completely up to you. [View the demo.](http://sparksuite.github.io/react-accessible-dropdown-menu-hook) +This Hook handles all the accessibility logic when building a dropdown menu, dropdown button, etc., and leaves the design completely up to you. It also handles the logic for closing the menu when you click outside of it. [View the demo.](http://sparksuite.github.io/react-accessible-dropdown-menu-hook) ## Getting started From 1176a5a8768fa5268c3933bfedeac998d75ea6a1 Mon Sep 17 00:00:00 2001 From: Wes Cossick Date: Thu, 19 Dec 2019 14:49:52 -0600 Subject: [PATCH 6/6] Format the code --- src/use-dropdown-menu.ts | 15 +++++---------- test/puppeteer/demo.test.ts | 8 ++++---- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/use-dropdown-menu.ts b/src/use-dropdown-menu.ts index fa93447..98cb9cb 100644 --- a/src/use-dropdown-menu.ts +++ b/src/use-dropdown-menu.ts @@ -40,38 +40,33 @@ export default function useDropdownMenu(itemCount: number) { moveFocus(0); } - // This function is designed to handle every click const handleEveryClick = (event: MouseEvent) => { // Ignore if the menu isn't open if (!isOpen) { return; } - + // Make this happen asynchronously setTimeout(() => { // Type guard if (!(event.target instanceof Element)) { return; } - - + // Ignore if we're clicking inside the menu if (event.target.closest('[role="menu"]')) { return; } - - + // Hide dropdown setIsOpen(false); }, 10); }; - - + // Add listener document.addEventListener('click', handleEveryClick); - - + // Return function to remove listener return () => document.removeEventListener('click', handleEveryClick); }, [isOpen]); diff --git a/test/puppeteer/demo.test.ts b/test/puppeteer/demo.test.ts index 7877003..f39089f 100644 --- a/test/puppeteer/demo.test.ts +++ b/test/puppeteer/demo.test.ts @@ -31,7 +31,7 @@ it('focuses on the first menu item when the enter key is pressed', async () => { it('focuses on the menu button after pressing escape', async () => { await page.click('#menu-button'); await menuOpen(); - + await keyboard.down('Escape'); await menuClosed(); @@ -41,7 +41,7 @@ it('focuses on the menu button after pressing escape', async () => { it('focuses on the next item in the tab order after pressing tab', async () => { await page.click('#menu-button'); await menuOpen(); - + await keyboard.down('Tab'); await menuClosed(); @@ -51,7 +51,7 @@ it('focuses on the next item in the tab order after pressing tab', async () => { it('focuses on the previous item in the tab order after pressing shift-tab', async () => { await page.click('#menu-button'); await menuOpen(); - + await keyboard.down('Shift'); await keyboard.down('Tab'); await menuClosed(); @@ -62,7 +62,7 @@ it('focuses on the previous item in the tab order after pressing shift-tab', asy it('closes the menu if you click outside of it', async () => { await page.click('#menu-button'); await menuOpen(); - + await page.click('body'); await menuClosed(); // times out if menu doesn't close