Skip to content

Commit

Permalink
fix(PreviewSupport): do not copy original markers
Browse files Browse the repository at this point in the history 
related to bpmn-io/bpmn-js#2179
  • Loading branch information
@marstamm
marstamm committed May 29, 2024
1 parent d1caba8 commit 1a88d3f
Show file tree
Hide file tree
Showing 6 changed files with 353 additions and 38 deletions.
39 changes: 19 additions & 20 deletions lib/features/preview-support/PreviewSupport.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { query as domQuery } from 'min-dom';

import { getVisual } from '../../util/GraphicsUtil';

import Ids from '../../util/IdGenerator';

/**
* @typedef {import('../../core/Types').ElementLike} Element
* @typedef {import('../../core/Types').ShapeLike} Shape
Expand All @@ -20,6 +22,8 @@ import { getVisual } from '../../util/GraphicsUtil';
* @typedef {import('../../draw/Styles').default} Styles
*/

const cloneIds = new Ids('ps');

var MARKER_TYPES = [
'marker-start',
'marker-mid',
Expand Down Expand Up @@ -50,8 +54,6 @@ export default function PreviewSupport(elementRegistry, eventBus, canvas, styles
this._elementRegistry = elementRegistry;
this._canvas = canvas;
this._styles = styles;

this._clonedMarkers = {};
}

PreviewSupport.$inject = [
Expand Down Expand Up @@ -175,35 +177,32 @@ PreviewSupport.prototype._cloneMarkers = function(gfx, className = 'djs-dragger'
* @param {string} [className="djs-dragger"]
*/
PreviewSupport.prototype._cloneMarker = function(parentGfx, gfx, marker, markerType, className = 'djs-dragger') {
var markerId = marker.id + '-' + className;

var clonedMarker = this._clonedMarkers[ markerId ];

parentGfx = parentGfx || this._canvas._svg;
// Add a random suffix to the marker ID in case the same marker is previewed multiple times
var clonedMarkerId = [ marker.id, className, cloneIds.next() ].join('-');

if (!clonedMarker) {
clonedMarker = svgClone(marker);
// reuse marker if it was part of original gfx
var copiedMarker = domQuery('marker#' + marker.id, parentGfx);

var clonedMarkerId = markerId + '-clone';

clonedMarker.id = clonedMarkerId;
parentGfx = parentGfx || this._canvas._svg;

svgClasses(clonedMarker).add(className);
var clonedMarker = copiedMarker || svgClone(marker);

this._clonedMarkers[ markerId ] = clonedMarker;
clonedMarker.id = clonedMarkerId;

var defs = domQuery(':scope > defs', parentGfx);
svgClasses(clonedMarker).add(className);

if (!defs) {
defs = svgCreate('defs');
var defs = domQuery(':scope > defs', parentGfx);

svgAppend(parentGfx, defs);
}
if (!defs) {
defs = svgCreate('defs');

svgAppend(defs, clonedMarker);
svgAppend(parentGfx, defs);
}

var reference = idToReference(this._clonedMarkers[ markerId ].id);
svgAppend(defs, clonedMarker);

var reference = idToReference(clonedMarker.id);

svgAttr(gfx, markerType, reference);
};
Expand Down
8 changes: 4 additions & 4 deletions test/spec/features/complex-preview/ComplexPreviewSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ describe('features/complex-preview', function() {
// then
expect(domQueryAll('marker.djs-dragging', canvas.getContainer())).to.have.length(2);

expect(domQuery('marker#marker-start-djs-dragging-clone', canvas.getContainer())).to.exist;
expect(domQuery('marker#marker-end-djs-dragging-clone', canvas.getContainer())).to.exist;
expect(domQuery('marker[id^="marker-start-djs-dragging-ps"]', canvas.getContainer())).to.exist;
expect(domQuery('marker[id^="marker-end-djs-dragging-ps"]', canvas.getContainer())).to.exist;
}));


Expand Down Expand Up @@ -251,8 +251,8 @@ describe('features/complex-preview', function() {
expect(layer).to.exist;

expect(layer.childNodes).to.have.length(0);
expect(domQuery('marker#marker-start-djs-dragging-clone', canvas.getContainer())).not.to.exist;
expect(domQuery('marker#marker-end-djs-dragging-clone', canvas.getContainer())).not.to.exist;
expect(domQuery('marker[id^="marker-start-djs-dragging-ps"]', canvas.getContainer())).not.to.exist;
expect(domQuery('marker[id^="marker-end-djs-dragging-ps"]', canvas.getContainer())).not.to.exist;
}));

});
Expand Down
199 changes: 186 additions & 13 deletions test/spec/features/move/MovePreviewSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ import moveModule from 'lib/features/move';
import attachSupportModule from 'lib/features/attach-support';
import rulesModule from './rules';
import rendererModule from '../preview-support/renderer';
import nestedRendererModule from '../preview-support/nested-renderer';

import {
query as domQuery,
queryAll as domQueryAll
} from 'min-dom';

import { isArray } from 'min-dash';

import {
classes as svgClasses
} from 'tiny-svg';
Expand Down Expand Up @@ -584,27 +587,172 @@ describe('features/move - MovePreview', function() {
// then
var container = canvas.getContainer();

var clonedMarkers = domQueryAll('marker.djs-dragger', container);
// var clonedMarkers = domQueryAll('marker.djs-dragger', container);
// expect(clonedMarkers).to.have.length(6);

expect(clonedMarkers).to.have.length(3);
var markerStartClones = [ ...domQueryAll('marker[id^="marker-start-djs-dragger-ps"]', container) ],
markerMidClones = [ ...domQueryAll('marker[id^="marker-mid-djs-dragger-ps"]', container) ],
markerEndClones = [ ...domQueryAll('marker[id^="marker-end-djs-dragger-ps"]', container) ];

var markerStartClone = domQuery('marker#marker-start-djs-dragger-clone', container),
markerMidClone = domQuery('marker#marker-mid-djs-dragger-clone', container),
markerEndClone = domQuery('marker#marker-end-djs-dragger-clone', container);
expect(markerStartClones).to.have.length(4);
expect(markerMidClones).to.exist.length(4);
expect(markerEndClones).to.exist.length(4);

expect(markerStartClone).to.exist;
expect(markerMidClone).to.exist;
expect(markerEndClone).to.exist;
var markerStartCloneIds = markerStartClones.map(marker => marker.id),
markerMidCloneIds = markerMidClones.map(marker => marker.id),
markerEndCloneIds = markerEndClones.map(marker => marker.id);

var connection1Path = domQuery('[data-element-id="connection1"] path', dragGroup);

expect(idToReferenceFormatOptions(markerStartClone.id)).to.deep.include(connection1Path.style.markerStart);
expect(idToReferenceFormatOptions(markerEndClone.id)).to.deep.include(connection1Path.style.markerEnd);
expect(idToReferenceFormatOptions(markerStartCloneIds)).to.deep.include(connection1Path.style.markerStart);
expect(idToReferenceFormatOptions(markerEndCloneIds)).to.deep.include(connection1Path.style.markerEnd);

var connection2Path = domQuery('[data-element-id="connection2"] path', dragGroup);

expect(idToReferenceFormatOptions(markerStartClone.id)).to.deep.include(connection2Path.style.markerStart);
expect(idToReferenceFormatOptions(markerMidClone.id)).to.deep.include(connection2Path.style.markerMid);
expect(idToReferenceFormatOptions(markerStartCloneIds)).to.deep.include(connection2Path.style.markerStart);
expect(idToReferenceFormatOptions(markerMidCloneIds)).to.deep.include(connection2Path.style.markerMid);

var connection3Path = domQuery('[data-element-id="connection3"] path', dragGroup);

expect(idToReferenceFormatOptions(markerMidCloneIds)).to.deep.include(connection3Path.style.markerMid);
expect(idToReferenceFormatOptions(markerEndCloneIds)).to.deep.include(connection3Path.style.markerEnd);
}));


it('should remove markers on cleanup', inject(function(canvas, dragging, move, selection) {

// when
selection.select([ shape1, shape2, shape3 ]);

move.start(canvasEvent({ x: 0, y: 0 }), shape2);

dragging.move(canvasEvent({ x: 100, y: 50 }));

dragging.end();

// then
var clonedMarkers = domQueryAll('marker.djs-dragger-marker', canvas.getContainer());

expect(clonedMarkers).to.have.length(0);
}));

});


describe('nested markers', function() {

beforeEach(bootstrapDiagram({
modules: [
attachSupportModule,
modelingModule,
moveModule,
nestedRendererModule,
rulesModule
]
}));

beforeEach(inject(function(dragging) {
dragging.setOptions({ manual: true });
}));


var rootShape, shape1, shape2, shape3, connection1, connection2, connection3;

beforeEach(inject(function(elementFactory, canvas) {

rootShape = elementFactory.createRoot({
id: 'root'
});

canvas.setRootElement(rootShape);

shape1 = elementFactory.createShape({
id: 'shape1',
x: 100, y: 100, width: 100, height: 100
});

canvas.addShape(shape1, rootShape);

shape2 = elementFactory.createShape({
id: 'shape2',
x: 400, y: 300, width: 100, height: 100
});

canvas.addShape(shape2, rootShape);

shape3 = elementFactory.createShape({
id: 'shape3',
x: 100, y: 300, width: 100, height: 100
});

canvas.addShape(shape3, rootShape);

connection1 = elementFactory.createConnection({
id: 'connection1',
waypoints: [ { x: 200, y: 150 }, { x: 450, y: 150 }, { x: 450, y: 300 } ],
source: shape1,
target: shape2,
marker: {
start: true,
end: true
}
});

canvas.addConnection(connection1, rootShape);

connection2 = elementFactory.createConnection({
id: 'connection2',
waypoints: [ { x: 450, y: 400 }, { x: 450, y: 450 }, { x: 150, y: 450 }, { x: 150, y: 400 } ],
source: shape1,
target: shape2,
marker: {
start: true,
mid: true
}
});

canvas.addConnection(connection2, rootShape);

connection3 = elementFactory.createConnection({
id: 'connection3',
waypoints: [ { x: 150, y: 300 }, { x: 150, y: 200 } ],
source: shape1,
target: shape2,
marker: {
start: true,
mid: true,
end: true
}
});

canvas.addConnection(connection3, rootShape);
}));


it('should clone markers', inject(function(canvas, dragging, move, selection) {

// when
selection.select([ shape1, shape2, shape3 ]);

move.start(canvasEvent({ x: 0, y: 0 }), shape2);

dragging.move(canvasEvent({ x: 100, y: 50 }));

var dragGroup = dragging.context().data.context.dragGroup;

// then
var container = canvas.getContainer();

// var clonedMarkers = domQueryAll('marker.djs-dragger', container);
// expect(clonedMarkers).to.have.length(7);

var markerStartClone = domQuery('marker[id^="marker-start-connection3-djs-dragger-ps"]', container),
markerMidClone = domQuery('marker[id^="marker-mid-connection3-djs-dragger-ps"]', container),
markerEndClone = domQuery('marker[id^="marker-end-connection3-djs-dragger-ps"]', container);

expect(markerStartClone).to.exist;
expect(markerMidClone).to.exist;
expect(markerEndClone).to.exist;

var connection3Path = domQuery('[data-element-id="connection3"] path', dragGroup);

Expand All @@ -613,6 +761,28 @@ describe('features/move - MovePreview', function() {
}));


it('should NOT copy marker IDs', inject(function(canvas, dragging, move, selection) {

// when
selection.select([ shape1, shape2, shape3 ]);

move.start(canvasEvent({ x: 0, y: 0 }), shape2);

dragging.move(canvasEvent({ x: 100, y: 50 }));

// then
var container = canvas.getContainer();

var markerStart = domQueryAll('marker#marker-start-connection3', container),
markerMid = domQueryAll('marker#marker-mid-connection3', container),
markerEnd = domQueryAll('marker#marker-end-connection3', container);

expect(markerStart).to.have.length(1);
expect(markerMid).to.have.length(1);
expect(markerEnd).to.have.length(1);
}));


it('should remove markers on cleanup', inject(function(canvas, dragging, move, selection) {

// when
Expand All @@ -631,7 +801,6 @@ describe('features/move - MovePreview', function() {
}));

});

});

// helpers //////////
Expand Down Expand Up @@ -660,6 +829,10 @@ function idToReference(id, quoteSymbol) {
* @return {string[]}
*/
function idToReferenceFormatOptions(id) {
if (isArray(id)) {
return id.flatMap(id => idToReferenceFormatOptions(id));
}

return [ '', '\'', '"' ].map(function(quoteSymbol) {
return idToReference(id, quoteSymbol);
});
Expand Down
Loading

0 comments on commit 1a88d3f

Please sign in to comment.