Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pro 3247 keyboard shortcuts conflicts #3953

Closed

Conversation

falkodev
Copy link
Contributor

@falkodev falkodev commented Dec 7, 2022

Summary

If multiple keyboard shortcuts using the same combination of keys are registered, the system must display a warning inside the keyboard shortcut list modal about the conflict. This should not prevents other shortcuts from working, nor triggers multiples action using a single key combination.

Only the last identical keyboard shortcut registered must be available.

This matters only if they are available in the same context.

What are the specific steps to test this change?

  1. npm link apostrophe on testbed
  2. Conflicts will be notified once logged in
  3. Run unit tests

What kind of change does this PR introduce?

(Check at least one)

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Build-related changes
  • Other

Make sure the PR fulfills these requirements:

  • It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
  • The changelog is updated
  • Related documentation has been updated
  • Related tests have been updated

haroun and others added 30 commits November 4, 2022 20:33
…odule

* feature/keyboard-shortcut: (45 commits)
  remove tiptap breaking change and duplicates warnings (#3892)
  eslint
  fix various errata in the update menu
  Try better
  Changelog
  Manage busy wizard status to control actions disabled state
  Add `null` to select fields with a function name as choices (#3924)
  date
  remove TODO
  release 3.31.0
  undo weird change
  aposPlaceholder: true supersedes all field values
  fix advisory message
  do not try to "convert" while aposPlaceholder is still in effect; do not clone unnecessarily; always respect placeholders regardless of field values until aposPlaceholder is cleared
  add widget placeholders feature (#3910)
  release 3.30.0
  changelog, webpack context
  update vue-loader
  refactor addFileGroup method
  response to comments
  ...
…cut-component' into pro-3248-apos-command-menu-shortcut-component
…cut-component' into pro-3248-apos-command-menu-shortcut-component
…b.com:apostrophecms/apostrophe into pro-3248-apos-command-menu-shortcut-component

* 'pro-3248-apos-command-menu-shortcut-component' of github.com:apostrophecms/apostrophe:
  Disable shortcut commands when focus in rich-text editor and inputs
haroun and others added 9 commits December 5, 2022 14:18
…d-menu-shortcut-component

* pro-3244-command-menu-module: (56 commits)
  explicit command or group name in test
  compile errors from multiple modules test
  get all errors at once
  fast parking
  chore: prettify a bit & iterate over `Object.keys` instead of `for ... in`
  release 3.33.0
  protect against any possibility of dcad-1191 manifesting in A3
  update changelog
  for easier extension of the rich-text checking and adding all `defaultOptions` to the `activeOptions`
  Publish batch icon (#3944)
  dashed lines magic
  kill the foreign hover on mouseleave, also defensive driving
  Add "publish" batch feature to manager modal (#3939)
  Fixes based on comments
  add more info about PR contribution
  typos
  changelog
  working solution
  better link
  shiny new documentation link for email
  ...
@linear
Copy link

linear bot commented Dec 7, 2022

PRO-3247 Handling keyboard shortcuts conflicts

If multiple keyboard shortcuts using the same combination of keys are registered, the system must display a warning inside the keyboard shortcut list modal about the conflict. This should not prevents other shortcuts from working, nor triggers multiples action using a single key combination.

Only the last identical keyboard shortcut registered must be available.

This matters only if they are available in the same context.

Acceptance criteria

  • Keyboard shortcuts are still working in case of conflicts.
  • Integration tests exists.

@falkodev falkodev requested a review from haroun December 7, 2022 12:49
haroun and others added 16 commits December 7, 2022 22:00
…d-menu-shortcut-component

* pro-3244-command-menu-module:
  rename fields into commands
  test visible groups and modals
  add modals
…odule

* feature/keyboard-shortcut:
  fix lint error in vue component
  cleanup
  DRY
  trailing comma
  nested areas work properly with initialModal: false
…d-menu-shortcut-component

* pro-3244-command-menu-module:
  fix lint error in vue component
  cleanup
  DRY
  trailing comma
  nested areas work properly with initialModal: false
…-3247-keyboard-shortcuts-conflicts

* pro-3248-apos-command-menu-shortcut-component:
  check commands after filtering
  translate command menu
  remove comment
  remove todo statement
  use variable in CSS
  fix i18n lint error
  add permission
  fix lint error in vue component
  rename fields into commands
  test visible groups and modals
  add modals
  cleanup
  DRY
  trailing comma
  nested areas work properly with initialModal: false
…cuts-conflicts

* feature/keyboard-shortcut:
  handle node 16+ better
  put 14 back on the list doh
  use npm 8 with node 14, because tiptap
Base automatically changed from pro-3248-apos-command-menu-shortcut-component to feature/keyboard-shortcut December 20, 2022 19:33
@falkodev falkodev closed this Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants