From 5fd682d83af45788ac48d09f1377587d773c7fa7 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Wed, 9 Nov 2022 09:50:58 -0500 Subject: [PATCH] fixes #990 --- docs/components/dialog.md | 30 +++++------------------- docs/resources/changelog.md | 1 + src/components/dropdown/dropdown.test.ts | 23 ++++++++++++++++++ src/components/dropdown/dropdown.ts | 14 +++++++---- 4 files changed, 40 insertions(+), 28 deletions(-) diff --git a/docs/components/dialog.md b/docs/components/dialog.md index 311551735..3a91200e9 100644 --- a/docs/components/dialog.md +++ b/docs/components/dialog.md @@ -5,8 +5,12 @@ [component-header:sl-dialog] ```html preview - - Lorem ipsum dolor sit amet, consectetur adipiscing elit. + + + Option 1 + Option 2 + Option 3 + Close @@ -22,28 +26,6 @@ ``` -```jsx react -import { useState } from 'react'; -import { SlButton, SlDialog } from '@shoelace-style/shoelace/dist/react'; - -const App = () => { - const [open, setOpen] = useState(false); - - return ( - <> - setOpen(false)}> - Lorem ipsum dolor sit amet, consectetur adipiscing elit. - setOpen(false)}> - Close - - - - setOpen(true)}>Open Dialog - - ); -}; -``` - ## UX Tips - Use a dialog when you immediately require the user's attention, e.g. confirming a destructive action. diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index 28759b17b..71f94086f 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -19,6 +19,7 @@ _During the beta period, these restrictions may be relaxed in the event of a mis - Fixed a bug in `` where the inner border disappeared on focus [#980](https://github.com/shoelace-style/shoelace/pull/980) - Fixed a bug that caused prefix/suffix animations in `` to wobble [#996](https://github.com/shoelace-style/shoelace/issues/996) - Fixed a bug in `` that prevented color from being set on the host element [#999](https://github.com/shoelace-style/shoelace/issues/999) +- Fixed a bug in `` where the `keydown` event erroneously propagated to ancestors when pressing Escape [#990](https://github.com/shoelace-style/shoelace/issues/990) - Improved `` to improve padding and render relative to the current font size - Updated Lit to 2.4.1 - Updated TypeScript to 4.8.4 diff --git a/src/components/dropdown/dropdown.test.ts b/src/components/dropdown/dropdown.test.ts index 29c6b6660..ea40ff95b 100644 --- a/src/components/dropdown/dropdown.test.ts +++ b/src/components/dropdown/dropdown.test.ts @@ -284,4 +284,27 @@ describe('', () => { expect(el.open).to.be.false; }); + + it('should close and stop propagating the keydown event when Escape is pressed and the dropdown is open ', async () => { + const el = await fixture(html` + + Toggle + + Dropdown Item 1 + Dropdown Item 2 + Dropdown Item 3 + + + `); + const firstMenuItem = el.querySelector('sl-menu-item')!; + const hideHandler = sinon.spy(); + + document.body.addEventListener('keydown', hideHandler); + firstMenuItem.focus(); + await sendKeys({ press: 'Escape' }); + await el.updateComplete; + + expect(el.open).to.be.false; + expect(hideHandler).to.not.have.been.called; + }); }); diff --git a/src/components/dropdown/dropdown.ts b/src/components/dropdown/dropdown.ts index 17ff2bd2a..1baa2a3d1 100644 --- a/src/components/dropdown/dropdown.ts +++ b/src/components/dropdown/dropdown.ts @@ -100,6 +100,7 @@ export default class SlDropdown extends ShoelaceElement { super.connectedCallback(); this.handleMenuItemActivate = this.handleMenuItemActivate.bind(this); this.handlePanelSelect = this.handlePanelSelect.bind(this); + this.handleKeyDown = this.handleKeyDown.bind(this); this.handleDocumentKeyDown = this.handleDocumentKeyDown.bind(this); this.handleDocumentMouseDown = this.handleDocumentMouseDown.bind(this); @@ -139,14 +140,17 @@ export default class SlDropdown extends ShoelaceElement { | undefined; } - handleDocumentKeyDown(event: KeyboardEvent) { - // Close when escape is pressed - if (event.key === 'Escape') { + handleKeyDown(event: KeyboardEvent) { + // Close when escape is pressed inside an open dropdown. We need to listen on the panel itself and stop propagation + // in case any ancestors are also listening for this key. + if (this.open && event.key === 'Escape') { + event.stopPropagation(); this.hide(); this.focusOnTrigger(); - return; } + } + handleDocumentKeyDown(event: KeyboardEvent) { // Handle tabbing if (event.key === 'Tab') { // Tabbing within an open menu should close the dropdown and refocus the trigger @@ -341,6 +345,7 @@ export default class SlDropdown extends ShoelaceElement { addOpenListeners() { this.panel.addEventListener('sl-activate', this.handleMenuItemActivate); this.panel.addEventListener('sl-select', this.handlePanelSelect); + this.panel.addEventListener('keydown', this.handleKeyDown); document.addEventListener('keydown', this.handleDocumentKeyDown); document.addEventListener('mousedown', this.handleDocumentMouseDown); } @@ -349,6 +354,7 @@ export default class SlDropdown extends ShoelaceElement { if (this.panel) { this.panel.removeEventListener('sl-activate', this.handleMenuItemActivate); this.panel.removeEventListener('sl-select', this.handlePanelSelect); + this.panel.removeEventListener('keydown', this.handleKeyDown); } document.removeEventListener('keydown', this.handleDocumentKeyDown); document.removeEventListener('mousedown', this.handleDocumentMouseDown);