fix: keep CSS classes when calling the "Update Style" API (#2709)

Previously, calling the "Update Style" API after adding the classes with
the CSS API removed the CSS classes from a given BPMN element.
This is now fixed by setting the classes in the model instead of
updating directly the state. The state is refreshed when calling the
"Update Style" API and the manually added classes were removed.

Basic integration tests have been added to check that the CSS classes
are present in the style property of the mxGraph model. They only check
a few use cases to help detecting regression They complement the
existing tests checking that the classes are correctly set in the SVG
nodes. The existing tests cover all use cases of the CSS API.

Author: tbouffard

src/component/mxgraph/GraphCellUpdater.ts

@@ -45,16 +45,14 @@ export default class GraphCellUpdater {
   }
 
   private updateAndRefreshCssClassesOfElement(elementId: string, cssClasses: string[]): void {
-    const mxCell = this.graph.getModel().getCell(elementId);
-    if (!mxCell) {
+    const cell = this.graph.getModel().getCell(elementId);
+    if (!cell) {
       return;
     }
-    const view = this.graph.getView();
-    const state = view.getState(mxCell);
-    state.style[BpmnStyleIdentifier.EXTRA_CSS_CLASSES] = cssClasses;
-    state.shape.redraw();
-    // Ensure that label classes are also updated. When there is no label, state.text is null
-    state.text?.redraw();
+
+    let cellStyle = cell.getStyle();
+    cellStyle = setStyle(cellStyle, BpmnStyleIdentifier.EXTRA_CSS_CLASSES, cssClasses.join(','));
+    this.graph.model.setStyle(cell, cellStyle);
   }
 
   addOverlays(bpmnElementId: string, overlays: Overlay | Overlay[]): void {

src/component/mxgraph/config/ShapeConfigurator.ts

@@ -150,11 +150,11 @@ export default class ShapeConfigurator {
         // 'this.state.style' = the style definition associated with the cell
         // 'this.state.cell.style' = the style applied to the cell: 1st element: style name = bpmn shape name
         const cell = this.state.cell;
-        // dialect = strictHtml is set means that current node holds an html label
+        // dialect = strictHtml is set means that current node holds an HTML label
         let allBpmnClassNames = computeAllBpmnClassNamesOfCell(cell, this.dialect === mxgraph.mxConstants.DIALECT_STRICTHTML);
         const extraCssClasses = this.state.style[BpmnStyleIdentifier.EXTRA_CSS_CLASSES];
-        if (extraCssClasses) {
-          allBpmnClassNames = allBpmnClassNames.concat(extraCssClasses);
+        if (typeof extraCssClasses == 'string') {
+          allBpmnClassNames = allBpmnClassNames.concat(extraCssClasses.split(','));
         }
 
         this.node.setAttribute('class', allBpmnClassNames.join(' '));

test/integration/helpers/model-expect.ts

@@ -144,6 +144,8 @@ type ExpectedModelElement = {
   stroke?: Stroke;
   verticalAlign?: string;
   opacity?: number;
+  // custom bpmn-visualization
+  extraCssClasses?: string[];
 };
 
 export interface ExpectedShapeModelElement extends ExpectedModelElement {

test/integration/matchers/matcher-utils.ts

@@ -20,6 +20,7 @@ import type { mxCell, mxGeometry, StyleMap } from 'mxgraph';
 import type { Opacity } from '@lib/component/registry';
 import type { MxGraphCustomOverlay, MxGraphCustomOverlayStyle } from '@lib/component/mxgraph/overlay/custom-overlay';
 import { getFontStyleValue as computeFontStyleValue } from '@lib/component/mxgraph/renderer/StyleComputer';
+import { BpmnStyleIdentifier } from '@lib/component/mxgraph/style';
 import { Font } from '@lib/model/bpmn/internal/Label';
 import MatcherContext = jest.MatcherContext;
 import CustomMatcherResult = jest.CustomMatcherResult;
@@ -45,6 +46,8 @@ export interface BpmnCellStyle extends StyleMap {
   endSize?: number;
   shape?: string;
   horizontal?: number;
+  // custom bpmn-visualization
+  extraCssClasses?: string[];
 }
 
 export interface ExpectedCell {
@@ -142,6 +145,8 @@ export function buildExpectedCellStyleWithCommonAttributes(expectedModelElt: Exp
     fontColor: font?.color ?? 'Black',
     fontStyle: getFontStyleValue(font),
     fontOpacity: expectedModelElt.font?.opacity,
+    // custom bpmn-visualization
+    extraCssClasses: expectedModelElt.extraCssClasses,
   };
 }
 
@@ -165,9 +170,10 @@ export function buildReceivedViewStateStyle(cell: mxCell, bv = bpmnVisualization
  * It returns the style + properties resolved from the referenced styleNames (generally at the beginning of the "cell.style" string) as computed by mxStylesheet.prototype.getCellStyle.
  *
  * @param cell The Cell to consider for the computation of the resolved style.
+ * @param bv The instance of BpmnVisualization under test
  */
-function buildReceivedResolvedModelCellStyle(cell: mxCell): BpmnCellStyle {
-  return toBpmnStyle(bpmnVisualization.graph.getCellStyle(cell), cell.edge);
+export function buildReceivedResolvedModelCellStyle(cell: mxCell, bv = bpmnVisualization): BpmnCellStyle {
+  return toBpmnStyle(bv.graph.getCellStyle(cell), cell.edge);
 }
 
 function toBpmnStyle(rawStyle: StyleMap, isEdge: boolean): BpmnCellStyle {
@@ -184,6 +190,8 @@ function toBpmnStyle(rawStyle: StyleMap, isEdge: boolean): BpmnCellStyle {
     fontColor: rawStyle.fontColor,
     fontStyle: rawStyle.fontStyle,
     fontOpacity: rawStyle.textOpacity,
+    // custom bpmn-visualization
+    extraCssClasses: rawStyle[BpmnStyleIdentifier.EXTRA_CSS_CLASSES]?.split(','),
   };
 
   if (isEdge) {

test/integration/mxGraph.model.css.api.test.ts

@@ -0,0 +1,46 @@
+/*
+Copyright 2023 Bonitasoft S.A.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { bpmnVisualization } from './helpers/model-expect';
+import { readFileSync } from '@test/shared/file-helper';
+
+// Most of the test are done in dom.css.classes.test.ts
+// The tests here check that the style of the cell in the mxGraph model includes the CSS classes
+describe('mxGraph model - CSS API', () => {
+  beforeEach(() => {
+    bpmnVisualization.load(readFileSync('../fixtures/bpmn/registry/1-pool-3-lanes-message-start-end-intermediate-events.bpmn'));
+  });
+
+  test('Add CSS classes on Shape', () => {
+    bpmnVisualization.bpmnElementsRegistry.addCssClasses('userTask_2_2', ['class#1', 'class#2']);
+    expect('userTask_2_2').toBeUserTask({
+      extraCssClasses: ['class#1', 'class#2'],
+      // not under test
+      parentId: 'lane_02',
+      label: 'User Task 2.2',
+    });
+  });
+
+  test('Add CSS classes on Edge', () => {
+    bpmnVisualization.bpmnElementsRegistry.addCssClasses('sequenceFlow_lane_3_elt_3', ['class-1', 'class-2', 'class-3']);
+    expect('sequenceFlow_lane_3_elt_3').toBeSequenceFlow({
+      extraCssClasses: ['class-1', 'class-2', 'class-3'],
+      // not under test
+      parentId: 'lane_03',
+      verticalAlign: 'bottom',
+    });
+  });
+});

test/integration/mxGraph.model.style.api.test.ts

@@ -18,11 +18,12 @@ import { initializeBpmnVisualizationWithContainerId } from './helpers/bpmn-visua
 import { HtmlElementLookup } from './helpers/html-utils';
 import type { ExpectedShapeModelElement } from './helpers/model-expect';
 import { bpmnVisualization } from './helpers/model-expect';
-import { buildReceivedViewStateStyle } from './matchers/matcher-utils';
+import { buildReceivedResolvedModelCellStyle, buildReceivedViewStateStyle } from './matchers/matcher-utils';
 import { buildExpectedShapeCellStyle } from './matchers/toBeShape';
 import { readFileSync } from '@test/shared/file-helper';
 import { ShapeBpmnElementKind, ShapeBpmnEventDefinitionKind } from '@lib/model/bpmn/internal';
 import type { EdgeStyleUpdate, Fill, Font, Stroke, StyleUpdate } from '@lib/component/registry';
+import type { mxCell } from 'mxgraph';
 
 describe('mxGraph model - update style', () => {
   describe('Shapes', () => {
@@ -580,25 +581,26 @@ describe('mxGraph model - update style', () => {
     const bv = initializeBpmnVisualizationWithContainerId('bpmn-container-style-css-cross-tests');
     const htmlElementLookup = new HtmlElementLookup(bv);
 
-    // we cannot reuse the model expect functions here. They are using the shared bpmnVisualization that we cannot use here.
-    // So use the minimal expect function. We only need to check a part of the data, the rest is already checked in details in other tests.
-    const checkViewStateStyle = (bpmnElementId: string, expectedModel: ExpectedShapeModelElement): void => {
+    const getCell = (bpmnElementId: string): mxCell => {
       const graph = bv.graph;
       const cell = graph.model.getCell(bpmnElementId);
       if (!cell) {
         throw new Error(`Unable to find cell in the model with id ${bpmnElementId}`);
       }
+      return cell;
+    };
 
-      const receivedViewStateStyle = buildReceivedViewStateStyle(cell, bv);
-      expect(receivedViewStateStyle).toEqual(buildExpectedShapeCellStyle(expectedModel));
+    // we cannot reuse the model expect functions here. They are using the shared bpmnVisualization that we cannot use here.
+    // So use the minimal expect function. We only need to check a part of the data, the rest is already checked in details in other tests.
+    const checkViewStateStyle = (bpmnElementId: string, expectedModel: ExpectedShapeModelElement): void => {
+      expect(buildReceivedViewStateStyle(getCell(bpmnElementId), bv)).toEqual(buildExpectedShapeCellStyle(expectedModel));
+    };
+
+    const checkModelStyle = (bpmnElementId: string, expectedModel: ExpectedShapeModelElement): void => {
+      expect(buildReceivedResolvedModelCellStyle(getCell(bpmnElementId), bv)).toEqual(buildExpectedShapeCellStyle(expectedModel));
     };
 
-    it.each(
-      // We have a bug when the CSS classes are applied first, they are dropped after the call of the updateStyle method
-      // See https://github.com/process-analytics/bpmn-visualization-js/issues/2561
-      // When fixed, it.each should use [true, false]
-      [true],
-    )('Apply style update first %s', (isStyleUpdateAppliedFirst: boolean) => {
+    it.each([true, false])('Apply style update first %s', (isStyleUpdateAppliedFirst: boolean) => {
       bv.load(readFileSync('../fixtures/bpmn/registry/1-pool-3-lanes-message-start-end-intermediate-events.bpmn'));
 
       const bpmnElementId = 'endEvent_message_1';
@@ -612,11 +614,14 @@ describe('mxGraph model - update style', () => {
         bv.bpmnElementsRegistry.updateStyle(bpmnElementId, { stroke: { color: strokeColor } });
       }
 
-      checkViewStateStyle(bpmnElementId, {
+      const expectedModel = {
+        extraCssClasses: ['class-1', 'class-2'],
         kind: ShapeBpmnElementKind.EVENT_END,
         stroke: { color: strokeColor },
         verticalAlign: 'top', // when events have a label
-      });
+      };
+      checkModelStyle(bpmnElementId, expectedModel);
+      checkViewStateStyle(bpmnElementId, expectedModel);
       htmlElementLookup.expectEndEvent(bpmnElementId, ShapeBpmnEventDefinitionKind.MESSAGE, { label: 'message end 2', additionalClasses: ['class-1', 'class-2'] });
     });
   });