fixes #925

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 `<sl-input>` to wobble [#996](https://github.com/shoelace-style/shoelace/issues/996)
 - Fixed a bug in `<sl-icon>` that prevented color from being set on the host element [#999](https://github.com/shoelace-style/shoelace/issues/999)
 - Fixed a bug in `<sl-dropdown>` where the `keydown` event erroneously propagated to ancestors when pressing <kbd>Escape</kbd> [#990](https://github.com/shoelace-style/shoelace/issues/990)
+- Fixed a bug that prevented arrow keys from scrolling content within `<sl-dialog>` and `<sl-drawer>` [#925](https://github.com/shoelace-style/shoelace/issues/925)
+- Fixed a bug that prevented <kbd>Escape</kbd> from closing `<sl-dialog>` and `<sl-drawer>` in some cases
 - Improved `<sl-badge>` to improve padding and render relative to the current font size
 - Updated Lit to 2.4.1
 - Updated TypeScript to 4.8.4

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('<sl-dialog>', () => {
     expect(initialFocusHandler).to.have.been.calledOnce;
     expect(document.activeElement).to.equal(input);
   });
+
+  it('should close when pressing Escape', async () => {
+    const el = await fixture<SlDialog>(html` <sl-dialog open></sl-dialog> `);
+    const hideHandler = sinon.spy();
+
+    el.addEventListener('sl-hide', hideHandler);
+
+    await sendKeys({ press: 'Escape' });
+    await waitUntil(() => hideHandler.calledOnce);
+
+    expect(el.open).to.be.false;
+  });
 });

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}
       >
         <div part="overlay" class="dialog__overlay" @click=${() => this.requestClose('overlay')} tabindex="-1"></div>
 

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('<sl-drawer>', () => {
     expect(initialFocusHandler).to.have.been.calledOnce;
     expect(document.activeElement).to.equal(input);
   });
+
+  it('should close when pressing Escape', async () => {
+    const el = await fixture<SlDrawer>(html` <sl-drawer open></sl-drawer> `);
+    const hideHandler = sinon.spy();
+
+    el.addEventListener('sl-hide', hideHandler);
+
+    await sendKeys({ press: 'Escape' });
+    await waitUntil(() => hideHandler.calledOnce);
+
+    expect(el.open).to.be.false;
+  });
 });

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}
       >
         <div part="overlay" class="drawer__overlay" @click=${() => this.requestClose('overlay')} tabindex="-1"></div>
 

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() {