fix: fix and test shortcut keys in modals (#1115)

src/routes/_components/dialog/components/MediaDialog.html

@@ -75,8 +75,8 @@
   </div>
 </ModalDialog>
 
-<Shortcut scope='mediaDialog' key="ArrowLeft" on:pressed="prev()" />
-<Shortcut scope='mediaDialog' key="ArrowRight" on:pressed="next()" />
+<Shortcut scope='modal-{id}' key="ArrowLeft" on:pressed="prev()" />
+<Shortcut scope='modal-{id}' key="ArrowRight" on:pressed="next()" />
 <style>
   :global(.media-modal-dialog) {
     max-width: calc(100vw);
@@ -201,9 +201,6 @@
   import { smoothScroll, hasNativeSmoothScroll } from '../../../_utils/smoothScroll'
   import { doubleRAF } from '../../../_utils/doubleRAF'
   import { store } from '../../../_store/store'
-  import {
-    pushShortcutScope,
-    popShortcutScope } from '../../../_utils/shortcuts'
 
   export default {
     oncreate () {
@@ -220,11 +217,9 @@
       } else {
         this.setupScroll()
       }
-      pushShortcutScope('mediaDialog')
     },
     ondestroy () {
       this.teardownScroll()
-      popShortcutScope('mediaDialog')
     },
     store: () => store,
     data: () => ({

src/routes/_components/dialog/components/ModalDialog.html

@@ -22,7 +22,7 @@
     <slot></slot>
   </div>
 </div>
-<Shortcut scope="modal" key="Backspace" on:pressed="closeDialog(id)"/>
+<Shortcut scope="modal-{id}" key="Backspace" on:pressed="closeDialog(id)"/>
 <style>
   :global(.modal-dialog[aria-hidden='true']) {
     display: none;
@@ -146,10 +146,10 @@
 
   export default {
     oncreate () {
+      let { id } = this.get()
       let dialogElement = this.refs.node.parentElement
       this._a11yDialog = new A11yDialog(dialogElement)
       this._a11yDialog.on('hide', () => {
-        let { id } = this.get()
         document.body.classList.toggle('modal-open', false)
         this.fire('close')
         this._a11yDialog.destroy()
@@ -158,10 +158,11 @@
       })
       on('showDialog', this, this.showDialog)
       on('closeDialog', this, this.closeDialog)
-      pushShortcutScope('modal')
+      pushShortcutScope(`modal-${id}`)
     },
     ondestroy () {
-      popShortcutScope('modal')
+      let { id } = this.get()
+      popShortcutScope(`modal-${id}`)
     },
     components: { Shortcut, SvgIcon },
     data: () => ({

src/routes/_components/dialog/components/ShortcutHelpDialog.html

@@ -9,7 +9,7 @@
 
   <ShortcutHelpInfo inDialog={true} />
 
-  <Shortcut scope='modal' key='h|?' on:pressed='close()'/>
+  <Shortcut scope='modal-{id}' key='h|?' on:pressed='close()'/>
 </ModalDialog>
 <style>
   h1 {

tests/spec/030-shortcuts-modal.js

@@ -0,0 +1,75 @@
+import {
+  composeButton, composeModalInput, composeModalPostPrivacyButton,
+  getMediaScrollLeft,
+  getNthStatusMediaButton, getNthStatusOptionsButton,
+  modalDialog, scrollToStatus, sleep, visibleModalDialog
+} from '../utils'
+import { loginAsFoobar } from '../roles'
+import { indexWhere } from '../../src/routes/_utils/arrays'
+import { homeTimeline } from '../fixtures'
+
+fixture`030-shortcuts-modal.js`
+  .page`http://localhost:4002`
+
+test('Backspace dismisses modal', async t => {
+  await loginAsFoobar(t)
+  await t
+    .click(getNthStatusOptionsButton(1))
+    .expect(modalDialog.hasAttribute('aria-hidden')).notOk()
+  await sleep(1000)
+  await t
+    .pressKey('backspace')
+    .expect(modalDialog.exists).false
+})
+
+test('Backspace dismisses media modal', async t => {
+  await loginAsFoobar(t)
+  let idx = indexWhere(homeTimeline, _ => (_.content || '').includes('2 kitten photos'))
+  await scrollToStatus(t, idx + 1)
+  await t
+    .click(getNthStatusMediaButton(idx + 1))
+    .expect(modalDialog.hasAttribute('aria-hidden')).notOk()
+  await sleep(1000)
+  await t
+    .pressKey('backspace')
+    .expect(modalDialog.exists).false
+})
+
+test('Left/right changes active media in modal', async t => {
+  await loginAsFoobar(t)
+  let idx = indexWhere(homeTimeline, _ => (_.content || '').includes('2 kitten photos'))
+  await scrollToStatus(t, idx + 1)
+  await t
+    .click(getNthStatusMediaButton(idx + 1))
+    .expect(modalDialog.hasAttribute('aria-hidden')).notOk()
+    .expect(getMediaScrollLeft()).eql(0)
+  await sleep(1000)
+  await t
+    .pressKey('right')
+    .expect(getMediaScrollLeft()).gt(0)
+  await sleep(1000)
+  await t
+    .pressKey('backspace')
+    .expect(modalDialog.exists).false
+})
+
+test('Backspace works correctly for nested modal', async t => {
+  await loginAsFoobar(t)
+  await scrollToStatus(t, 10)
+  await t
+    .click(composeButton)
+    .expect(modalDialog.hasAttribute('aria-hidden')).notOk()
+    .expect(composeModalInput.exists).ok()
+    .click(composeModalPostPrivacyButton)
+    .expect(visibleModalDialog.textContent).contains('Post privacy')
+  await sleep(1000)
+  await t
+    .pressKey('backspace')
+  await t
+    .expect(modalDialog.hasAttribute('aria-hidden')).notOk()
+    .expect(composeModalInput.exists).ok()
+  await sleep(1000)
+  await t
+    .pressKey('backspace')
+    .expect(modalDialog.exists).notOk()
+})

tests/utils.js

@@ -5,6 +5,7 @@ import * as blobUtils from './blobUtils'
 export const settingsButton = $('nav a[aria-label=Settings]')
 export const instanceInput = $('#instanceInput')
 export const modalDialog = $('.modal-dialog')
+export const visibleModalDialog = $('.modal-dialog:not([aria-hidden="true"])')
 export const modalDialogContents = $('.modal-dialog-contents')
 export const closeDialogButton = $('.close-dialog-button')
 export const notificationsNavButton = $('nav a[href="/notifications"]')
@@ -76,6 +77,8 @@ export const sleep = timeout => new Promise(resolve => setTimeout(resolve, timeo
 
 export const getUrl = exec(() => window.location.href)
 
+export const getMediaScrollLeft = exec(() => document.querySelector('.media-scroll').scrollLeft || 0)
+
 export const getActiveElementClassList = exec(() =>
   (document.activeElement && (document.activeElement.getAttribute('class') || '').split(/\s+/)) || []
 )
@@ -243,6 +246,10 @@ export function getNthStatusMedia (n) {
   return $(`${getNthStatusSelector(n)} .status-media`)
 }
 
+export function getNthStatusMediaButton (n) {
+  return $(`${getNthStatusSelector(n)} .status-media button`)
+}
+
 export function getNthStatusRelativeDate (n) {
   return $(`${getNthStatusSelector(n)} .status-relative-date`)
 }