From 5eeb98d39d2ac4e8db028aaa1969f8d2aac395a2 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Wed, 9 Nov 2022 11:34:50 -0500 Subject: [PATCH] fixes #925 --- docs/resources/changelog.md | 2 ++ src/components/dialog/dialog.test.ts | 13 +++++++++++++ src/components/dialog/dialog.ts | 17 ++++++++++++++--- src/components/drawer/drawer.test.ts | 13 +++++++++++++ src/components/drawer/drawer.ts | 17 ++++++++++++++--- src/internal/modal.ts | 6 +++--- 6 files changed, 59 insertions(+), 9 deletions(-) diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index 71f94086f..c3a785eb8 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -20,6 +20,8 @@ _During the beta period, these restrictions may be relaxed in the event of a mis - 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) +- Fixed a bug that prevented arrow keys from scrolling content within `` and `` [#925](https://github.com/shoelace-style/shoelace/issues/925) +- Fixed a bug that prevented Escape from closing `` and `` in some cases - 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/dialog/dialog.test.ts b/src/components/dialog/dialog.test.ts index f430f08f1..58d5f82ab 100644 --- a/src/components/dialog/dialog.test.ts +++ b/src/components/dialog/dialog.test.ts @@ -1,5 +1,6 @@ // cspell:dictionaries lorem-ipsum import { expect, fixture, html, waitUntil } from '@open-wc/testing'; +import { sendKeys } from '@web/test-runner-commands'; import sinon from 'sinon'; import type SlDialog from './dialog'; @@ -132,4 +133,16 @@ describe('', () => { expect(initialFocusHandler).to.have.been.calledOnce; expect(document.activeElement).to.equal(input); }); + + it('should close when pressing Escape', async () => { + const el = await fixture(html` `); + const hideHandler = sinon.spy(); + + el.addEventListener('sl-hide', hideHandler); + + await sendKeys({ press: 'Escape' }); + await waitUntil(() => hideHandler.calledOnce); + + expect(el.open).to.be.false; + }); }); diff --git a/src/components/dialog/dialog.ts b/src/components/dialog/dialog.ts index 6d214ba7c..dc94ce284 100644 --- a/src/components/dialog/dialog.ts +++ b/src/components/dialog/dialog.ts @@ -90,6 +90,7 @@ export default class SlDialog extends ShoelaceElement { connectedCallback() { super.connectedCallback(); + this.handleDocumentKeyDown = this.handleDocumentKeyDown.bind(this); this.modal = new Modal(this); } @@ -97,6 +98,7 @@ export default class SlDialog extends ShoelaceElement { this.dialog.hidden = !this.open; if (this.open) { + this.addOpenListeners(); this.modal.activate(); lockBodyScrolling(this); } @@ -142,8 +144,16 @@ export default class SlDialog extends ShoelaceElement { this.hide(); } - handleKeyDown(event: KeyboardEvent) { - if (event.key === 'Escape') { + addOpenListeners() { + document.addEventListener('keydown', this.handleDocumentKeyDown); + } + + removeOpenListeners() { + document.removeEventListener('keydown', this.handleDocumentKeyDown); + } + + handleDocumentKeyDown(event: KeyboardEvent) { + if (this.open && event.key === 'Escape') { event.stopPropagation(); this.requestClose('keyboard'); } @@ -154,6 +164,7 @@ export default class SlDialog extends ShoelaceElement { if (this.open) { // Show this.emit('sl-show'); + this.addOpenListeners(); this.originalTrigger = document.activeElement as HTMLElement; this.modal.activate(); @@ -203,6 +214,7 @@ export default class SlDialog extends ShoelaceElement { } else { // Hide this.emit('sl-hide'); + this.removeOpenListeners(); this.modal.deactivate(); await Promise.all([stopAnimations(this.dialog), stopAnimations(this.overlay)]); @@ -249,7 +261,6 @@ export default class SlDialog extends ShoelaceElement { 'dialog--open': this.open, 'dialog--has-footer': this.hasSlotController.test('footer') })} - @keydown=${this.handleKeyDown} >
this.requestClose('overlay')} tabindex="-1">
diff --git a/src/components/drawer/drawer.test.ts b/src/components/drawer/drawer.test.ts index ad7ba4a53..d285cbc9e 100644 --- a/src/components/drawer/drawer.test.ts +++ b/src/components/drawer/drawer.test.ts @@ -1,5 +1,6 @@ // cspell:dictionaries lorem-ipsum import { expect, fixture, html, waitUntil } from '@open-wc/testing'; +import { sendKeys } from '@web/test-runner-commands'; import sinon from 'sinon'; import type SlDrawer from './drawer'; @@ -132,4 +133,16 @@ describe('', () => { expect(initialFocusHandler).to.have.been.calledOnce; expect(document.activeElement).to.equal(input); }); + + it('should close when pressing Escape', async () => { + const el = await fixture(html` `); + const hideHandler = sinon.spy(); + + el.addEventListener('sl-hide', hideHandler); + + await sendKeys({ press: 'Escape' }); + await waitUntil(() => hideHandler.calledOnce); + + expect(el.open).to.be.false; + }); }); diff --git a/src/components/drawer/drawer.ts b/src/components/drawer/drawer.ts index 2666bfe69..05fbeea8c 100644 --- a/src/components/drawer/drawer.ts +++ b/src/components/drawer/drawer.ts @@ -107,6 +107,7 @@ export default class SlDrawer extends ShoelaceElement { connectedCallback() { super.connectedCallback(); + this.handleDocumentKeyDown = this.handleDocumentKeyDown.bind(this); this.modal = new Modal(this); } @@ -114,6 +115,7 @@ export default class SlDrawer extends ShoelaceElement { this.drawer.hidden = !this.open; if (this.open && !this.contained) { + this.addOpenListeners(); this.modal.activate(); lockBodyScrolling(this); } @@ -159,8 +161,16 @@ export default class SlDrawer extends ShoelaceElement { this.hide(); } - handleKeyDown(event: KeyboardEvent) { - if (event.key === 'Escape') { + addOpenListeners() { + document.addEventListener('keydown', this.handleDocumentKeyDown); + } + + removeOpenListeners() { + document.removeEventListener('keydown', this.handleDocumentKeyDown); + } + + handleDocumentKeyDown(event: KeyboardEvent) { + if (this.open && event.key === 'Escape') { event.stopPropagation(); this.requestClose('keyboard'); } @@ -171,6 +181,7 @@ export default class SlDrawer extends ShoelaceElement { if (this.open) { // Show this.emit('sl-show'); + this.addOpenListeners(); this.originalTrigger = document.activeElement as HTMLElement; // Lock body scrolling only if the drawer isn't contained @@ -225,6 +236,7 @@ export default class SlDrawer extends ShoelaceElement { } else { // Hide this.emit('sl-hide'); + this.removeOpenListeners(); this.modal.deactivate(); unlockBodyScrolling(this); @@ -279,7 +291,6 @@ export default class SlDrawer extends ShoelaceElement { 'drawer--rtl': this.localize.dir() === 'rtl', 'drawer--has-footer': this.hasSlotController.test('footer') })} - @keydown=${this.handleKeyDown} >
this.requestClose('overlay')} tabindex="-1">
diff --git a/src/internal/modal.ts b/src/internal/modal.ts index b4686e8ce..c00373530 100644 --- a/src/internal/modal.ts +++ b/src/internal/modal.ts @@ -52,10 +52,10 @@ export default class Modal { handleKeyDown(event: KeyboardEvent) { if (event.key === 'Tab' && event.shiftKey) { this.tabDirection = 'backward'; - } - // Ensure focus remains trapped after they key is pressed - requestAnimationFrame(() => this.checkFocus()); + // Ensure focus remains trapped after the key is pressed + requestAnimationFrame(() => this.checkFocus()); + } } handleKeyUp() {