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

Editor action refactoring #887

Merged
merged 10 commits into from
Oct 29, 2018
Merged

Editor action refactoring #887

merged 10 commits into from
Oct 29, 2018

Conversation

nikku
Copy link
Member

@nikku nikku commented Oct 26, 2018

This builds upon bpmn-io/diagram-js#295.

Refactors the editor actions to be independent from actual editor (or viewer) features, as discussed with @barmac.

Core features:

  • only bind to actions in EditorActions which are implemented via features
  • only register keyboard shortcuts for actions that are implemented via features
  • explicitly include modules in NavigatedViewer and Modeler
  • use key event exposed via keyEvent property

This superseeds #886, if merged.

@nikku nikku requested a review from barmac October 26, 2018 23:05
@ghost ghost assigned nikku Oct 26, 2018
@ghost ghost added the needs review Review pending label Oct 26, 2018
barmac and others added 10 commits October 29, 2018 15:24
With the new version of `diagram-js`, the keyboard listeners receive
a single parameter which is an object with `KeyboardEvent` event
as the only property. `BpmnKeyBindings` needed to change in order
to make use of new version of API.
`KeyboardEvent.keyCode` is deprecated, so the BpmnKeyBindings will
use non-deprecated `KeyboardEvent.key` instead.
* Treat features as optional dependencies and register action
  only if feature exists
* Explicitly add features to the Modeler and NavigatedViewer types
* only register keyboard bindings that are backed by actual
  editor actions
* rename BpmnKeyBindings -> BpmnKeyboardBindings for parity with
  diagram-js@latest
* sub-class and override _keyboardBindings_ service provided
  by diagram-js
* extend spec to verify default actions
* event got renamed to keyEvent in context passed to
  keyboard listeners
@barmac barmac force-pushed the editor-action-refactoring branch from 8d8d2a8 to eadef0d Compare October 29, 2018 14:35
@ghost ghost assigned barmac Oct 29, 2018
@barmac
Copy link
Member

barmac commented Oct 29, 2018

Added diagram-js@3.0.0 dependency

Copy link
Member

@barmac barmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@merge-me merge-me bot merged commit a94406e into master Oct 29, 2018
@delete-merged-branch delete-merged-branch bot deleted the editor-action-refactoring branch October 29, 2018 14:52
@ghost ghost removed the needs review Review pending label Oct 29, 2018
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.

2 participants