Skip to content

Commit

Permalink
refactor: directly override svgCanvas2D.getTextCss (#3046)
Browse files Browse the repository at this point in the history 
Previously, this was done by modifying the `mxSvgCanvas2D` prototype.
The new implementation limits side effects on applications that use
`mxGraph` directly, and doesn't modify code the lib doesn't own.

The `mxGraph` implementation overriding is required to ensure that the
label pointer-events is set to 'none'. New integration tests have been
added to validate the old behavior and ensure that the refactoring has
preserved it. In particular, more tests check the labels of a greater
number of BPMN element types, and the value of the "pointer-events"
property of labels is explicitly checked.

The overriding implementation has been simplified to reduce the
copy/paste of `mxGraph` code. Instead, it now calls the standard
`mxGraph` implementation with specific settings.
  • Loading branch information
@tbouffard
tbouffard authored Mar 4, 2024
1 parent b1e667f commit 1c613c8
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 62 deletions.
49 changes: 1 addition & 48 deletions src/component/mxgraph/config/ShapeConfigurator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
import type { mxShape } from 'mxgraph';

import { ShapeBpmnElementKind } from '../../../model/bpmn/internal';
import { mxCellRenderer, mxConstants, mxSvgCanvas2D } from '../initializer';
import { mxCellRenderer } from '../initializer';
import {
BusinessRuleTaskShape,
CallActivityShape,
Expand Down Expand Up @@ -81,53 +81,6 @@ const registerShapes = (): void => {
*/
export default class ShapeConfigurator {
configureShapes(): void {
this.initMxSvgCanvasPrototype();
registerShapes();
}

private initMxSvgCanvasPrototype(): void {
// getTextCss is only used when creating foreignObject for label, so there is no impact on svg text that we use for Overlays.
// Analysis done for mxgraph@4.1.1, still apply to mxgraph@4.2.2
mxSvgCanvas2D.prototype.getTextCss = function () {
const s = this.state;
const lh = mxConstants.ABSOLUTE_LINE_HEIGHT ? s.fontSize * mxConstants.LINE_HEIGHT + 'px' : mxConstants.LINE_HEIGHT * this.lineHeightCorrection;
let css =
'display: inline-block; font-size: ' +
s.fontSize +
'px; ' +
'font-family: ' +
s.fontFamily +
'; color: ' +
s.fontColor +
'; line-height: ' +
lh +
// START Fix for issue #920 (https://github.com/process-analytics/bpmn-visualization-js/issues/920)
// This cannot be generalized for all mxgraph use cases. For instance, in an editor mode, we should be able to edit the text by clicking on it.
// Setting to 'none' prevent to capture click.
'; pointer-events: none' +
// (this.pointerEvents ? this.pointerEventsValue : 'none') +
// END OF Fix for issue #920
'; ';

if ((s.fontStyle & mxConstants.FONT_BOLD) == mxConstants.FONT_BOLD) {
css += 'font-weight: bold; ';
}
if ((s.fontStyle & mxConstants.FONT_ITALIC) == mxConstants.FONT_ITALIC) {
css += 'font-style: italic; ';
}

const deco = [];
if ((s.fontStyle & mxConstants.FONT_UNDERLINE) == mxConstants.FONT_UNDERLINE) {
deco.push('underline');
}
if ((s.fontStyle & mxConstants.FONT_STRIKETHROUGH) == mxConstants.FONT_STRIKETHROUGH) {
deco.push('line-through');
}
if (deco.length > 0) {
css += 'text-decoration: ' + deco.join(' ') + '; ';
}

return css;
};
}
}
21 changes: 20 additions & 1 deletion src/component/mxgraph/shape/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ export const overrideCreateSvgCanvas = function (shape: mxShape): void {
// The following is copied from the mxgraph mxShape implementation then converted to TypeScript and enriched for bpmn-visualization
// It is needed for adding the custom attributes that permits identification of the BPMN elements in the DOM
shape.createSvgCanvas = function () {
const canvas = new mxSvgCanvas2D(this.node, false);
// START bpmn-visualization CUSTOMIZATION
// use custom canvas implementation
const canvas = new SvgCanvas2D(this.node, false);
// END bpmn-visualization CUSTOMIZATION
canvas.strokeTolerance = this.pointerEvents ? this.svgStrokeTolerance : 0;
canvas.pointerEventsValue = this.svgPointerEvents;
const off = this.getSvgScreenOffset();
Expand Down Expand Up @@ -65,3 +68,19 @@ export const overrideCreateSvgCanvas = function (shape: mxShape): void {
return canvas;
};
};

class SvgCanvas2D extends mxSvgCanvas2D {
// getTextCss is only used when creating foreignObject for label, so there is no impact on svg text that we use for Overlays.
// Analysis done for mxgraph@4.1.1, still apply to mxgraph@4.2.2
override getTextCss(): string {
const originalPointerEvents = this.pointerEvents;
// Fix for issue https://github.com/process-analytics/bpmn-visualization-js/issues/920
// This sets the "pointer-events" style property to "none" to avoid capturing the click.
// This cannot be generalized for all mxgraph use cases. For instance, in an editor mode, we should be able to edit the text by clicking on it.
this.pointerEvents = false;

const textCss = super.getTextCss();
this.pointerEvents = originalPointerEvents;
return textCss;
}
}
14 changes: 7 additions & 7 deletions test/integration/dom.container.div.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@ describe.each`
it('DOM should contains BPMN elements when loading simple-start-task-end.bpmn', () => {
bpmnVisualization.load(readFileSync('../fixtures/bpmn/simple-start-task-end.bpmn'));

htmlElementLookup.expectStartEvent('StartEvent_1', ShapeBpmnEventDefinitionKind.NONE);
htmlElementLookup.expectTask('Activity_1');
htmlElementLookup.expectEndEvent('EndEvent_1', ShapeBpmnEventDefinitionKind.NONE);
htmlElementLookup.expectStartEvent('StartEvent_1', ShapeBpmnEventDefinitionKind.NONE, { label: 'Start Event 1' });
htmlElementLookup.expectTask('Activity_1', { label: 'Task 1' });
htmlElementLookup.expectEndEvent('EndEvent_1', ShapeBpmnEventDefinitionKind.NONE, { label: 'End Event 1' });
});

it('DOM should contains BPMN elements when loading model-complete-semantic.bpmn', () => {
bpmnVisualization.load(readFileSync('../fixtures/bpmn/model-complete-semantic.bpmn'));

htmlElementLookup.expectPool('participant_1_id');
htmlElementLookup.expectLane('lane_4_1_id');
htmlElementLookup.expectPool('participant_1_id', { label: 'Pool 1' });
htmlElementLookup.expectLane('lane_4_1_id', { label: 'Lane with child lanes' });

htmlElementLookup.expectStartEvent('start_event_signal_id', ShapeBpmnEventDefinitionKind.SIGNAL);
htmlElementLookup.expectIntermediateThrowEvent('intermediate_throw_event_message_id', ShapeBpmnEventDefinitionKind.MESSAGE);
htmlElementLookup.expectStartEvent('start_event_signal_id', ShapeBpmnEventDefinitionKind.SIGNAL, { label: 'Signal Start Event' });
htmlElementLookup.expectIntermediateThrowEvent('intermediate_throw_event_message_id', ShapeBpmnEventDefinitionKind.MESSAGE, { label: 'Throw Message Intermediate Event' });
});
});
15 changes: 9 additions & 6 deletions test/integration/helpers/html-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ export class HtmlElementLookup {
this.expectEventType(bpmnId, 'bpmn-start-event', bpmnEventDefinition, checks);
}

expectIntermediateThrowEvent(bpmnId: string, bpmnEventDefinition: ShapeBpmnEventDefinitionKind): void {
this.expectEventType(bpmnId, 'bpmn-intermediate-throw-event', bpmnEventDefinition);
expectIntermediateThrowEvent(bpmnId: string, bpmnEventDefinition: ShapeBpmnEventDefinitionKind, checks?: RequestedChecks): void {
this.expectEventType(bpmnId, 'bpmn-intermediate-throw-event', bpmnEventDefinition, checks);
}

expectEndEvent(bpmnId: string, bpmnEventDefinition: ShapeBpmnEventDefinitionKind, checks?: RequestedChecks): void {
Expand All @@ -74,8 +74,8 @@ export class HtmlElementLookup {
this.expectElement(bpmnId, expectSvgTask, ['bpmn-type-activity', 'bpmn-type-task', bpmnClass], checks);
}

expectTask(bpmnId: string): void {
this.expectTaskType(bpmnId, 'bpmn-task');
expectTask(bpmnId: string, checks?: RequestedChecks): void {
this.expectTaskType(bpmnId, 'bpmn-task', checks);
}

expectServiceTask(bpmnId: string, checks?: RequestedChecks): void {
Expand All @@ -94,8 +94,8 @@ export class HtmlElementLookup {
this.expectElement(bpmnId, expectSvgLane, ['bpmn-type-container', 'bpmn-lane'], checks);
}

expectPool(bpmnId: string): void {
this.expectElement(bpmnId, expectSvgPool, ['bpmn-type-container', 'bpmn-pool']);
expectPool(bpmnId: string, checks?: RequestedChecks): void {
this.expectElement(bpmnId, expectSvgPool, ['bpmn-type-container', 'bpmn-pool'], checks);
}

// ===========================================================================
Expand Down Expand Up @@ -169,6 +169,9 @@ export class HtmlElementLookup {

const labelLastDivElement = this.querySelector<HTMLElement>(this.bpmnQuerySelectors.labelLastDiv(bpmnId));
expect(labelLastDivElement.innerHTML).toEqual(label);
// Validate fix for https://github.com/process-analytics/bpmn-visualization-js/issues/920
expect(labelLastDivElement.style.pointerEvents).toBe('none');

const labelSvgGroup = this.querySelector<HTMLElement>(this.bpmnQuerySelectors.labelSvgGroup(bpmnId));
expectClassAttribute(labelSvgGroup, computeClassValue(bpmnClasses, ['bpmn-label', ...(additionalClasses ?? [])]));
}
Expand Down

0 comments on commit 1c613c8

Please sign in to comment.