wagtail icon indicating copy to clipboard operation
wagtail copied to clipboard

Refactor existing non-Draftail keyboard shortcuts to be easier to build upon

Open lb- opened this issue 3 years ago • 3 comments

Builds on #8957 - Modals (dialogs) dispatching an event when they open & close.

Moves the existing, non-Draftail, keyboard shortcuts to one central include with the goal of making it easier to register new shortcuts and providing a modal that shows all available shortcuts.

Changed behaviour

  • When a keyboard shortcut is triggered for the save / preview pane, it will also focus on the button (more suitable for accessibility), this makes no real difference for the save shortcut.
  • They keyboard shortcuts will not activate when a modal is open, this causes confusion and is unhelpful to the user. We can revisit this if we find a suitable use case for this.

How to test

  • Load up any edit page and confirm you can use cmd+s on Mac or ctrl+s on Windows to make the save page trigger.
  • Load up any edit page and confirm you can use cmd+p on Mac or cmd+p on Windows to toggle the Preview side pain
  • Load up any edit page and confirm you can use command+option+m on Mac or ctrl+alt+m on Windows while on a specific field to show the comment field (note: comments must be toggled on).

Details

  • Move Mousetrap package to npm package and not static asset.
  • Adopt a data attribute driven keyboard shortcut declaration approach, along with a way to register shortcuts via dispatching an event. Not documented intentionally as we may change this before we finalise this approach in future work.
  • Pull out keyboard shortcut JS to generic include file
  • Sdd unit tests for some basic JS behaviour
  • [X] Do the tests still pass?[^1]
  • [X] Does the code comply with the style guide?
  • relates to https://github.com/wagtail/wagtail/issues/3949
  • A refinement on #8900 -> but without changing any existing keyboard shortcuts

lb- avatar Aug 07 '22 04:08 lb-

Rework of #8900 with a bit of a clearer build upon #8957 & without changing the current preview shortcut.

Turns out - finding a shortcut is hard - will need to discuss further with the core team and maybe on a separate discussion thread.

lb- avatar Aug 07 '22 04:08 lb-

I do not think the failing CI is from this PR

   /usr/bin/docker pull mysql:8.0.23
  Error response from daemon: Head "https://registry-1.docker.io/v2/library/mysql/manifests/8.0.23": Get "https://auth.docker.io/token?account=githubactions&scope=repository%3Alibrary%2Fmysql%3Apull&service=registry.docker.io": EOF
  Warning: Docker pull failed with exit code 1, back off 7.364 seconds before retry.
  /usr/bin/docker pull mysql:8.0.23
  8.0.23: Pulling from library/mysql
  Get "https://registry-1.docker.io/v2/library/mysql/manifests/sha256:355617769102e9d2ebb7d5879263a12d230badb7271c91748b2c7b0ac6971083": EOF
  Warning: Docker pull failed with exit code 1, back off 1.135 seconds before retry.
  /usr/bin/docker pull mysql:8.0.23
  8.0.23: Pulling from library/mysql
  Get "https://registry-1.docker.io/v2/library/mysql/manifests/sha256:355617769102e9d2ebb7d5879263a12d230badb7271c91748b2c7b0ac6971083": EOF
  Error: Docker pull failed with exit code 1

lb- avatar Aug 07 '22 04:08 lb-

Manage this branch in Squash

Test this branch here: https://lb-cleanupkeyboard-shortcuts-2u0fw.squash.io

squash-labs[bot] avatar Sep 06 '22 09:09 squash-labs[bot]

Closing this - does not seem to be getting traction or a big priority right now.

Plus we will probably revisit this implementation completely now that RFC78 (Stimulus) has been approved.

lb- avatar Nov 06 '22 10:11 lb-