Add delete action for multi element context #939
Conversation
Buckwich
changed the title
Buckwich
force-pushed
the
4554-multielement-context-pad
branch
3 times, most recently
from
253b990 to
d89bcf4
Compare
Buckwich
force-pushed
the
4554-multielement-context-pad
branch
from
d89bcf4 to
be76f0c
Compare
bpmn-io-tasks
bot
added
needs review
| }; | ||
|
|
||
| ContextPadProvider.prototype._isDeleteAllowed = function(elements) { | ||
| var baseAllowed = this._rules.allowed('elements.delete', { |
There was a problem hiding this comment.
[question] Why is it called baseAllowed? I know that this is the way it's called in bpmn-js, but we could reflect on this.
There was a problem hiding this comment.
I just reused it, but i'll reword it to make it clear that it could be an array
There was a problem hiding this comment.
renamed it to allowedOrAllowedElements or do you have a better suggestion?
| @@ -166,6 +166,60 @@ describe('features - context-pad', function() { | |||
| }) | |||
| ); | |||
|
|
|||
| describe('multi remove', function() { | |||
There was a problem hiding this comment.
Code style convention which should but is not part of eslint ruleset. One empty line after describe, two empty lines between test cases.
| describe('multi remove', function() { | |
| describe('multi remove', function() { | |
There was a problem hiding this comment.
do you have a link to this style convention? I was looking for it but couldn't find it yet
There was a problem hiding this comment.
This is how we've been writing the code for years (e.g. https://github.com/bpmn-io/bpmn-js/blob/develop/test/spec/features/drilldown/DrilldownSpec.js), but it's unfortunately undocumented.
| expect(deleteAction([ shape1, shape2 ])).to.exist; | ||
| } | ||
| )); | ||
|
|
There was a problem hiding this comment.
Code style convention (see previous comment).
|
The contribution looks good! The code is clean, and there are only a few suggestions/questions to address. |
Buckwich
force-pushed
the
4554-multielement-context-pad
branch
from
be76f0c to
c3960ee
Compare
Related to camunda/camunda-modeler#4554
Proposed Changes
Adds ContextPad actions for multi selections. The only action added for now is the delete action. Selecting multiple shapes and deleting with keyboard also works.
recording-dmn-multi-delete.mov
Checklist
To ensure you provided everything we need to look at your PR:
@bpmn-io/srtoolCloses {LINK_TO_ISSUE}orRelated to {LINK_TO_ISSUE}