From 324d4c1eec70f905eb7120bb065c4cbefd3d5b54 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Mon, 2 Oct 2023 10:02:49 +0200 Subject: [PATCH] fix(API): prevent a reset/remove when passing an empty string (#2900) --- src/component/mxgraph/style/style-updater.ts | 2 +- src/component/registry/css-registry.ts | 2 +- test/integration/mxGraph.model.css.api.test.ts | 13 ++++++------- test/integration/mxGraph.model.style.api.test.ts | 6 +++--- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/component/mxgraph/style/style-updater.ts b/src/component/mxgraph/style/style-updater.ts index af6988b44d..9671660e8a 100644 --- a/src/component/mxgraph/style/style-updater.ts +++ b/src/component/mxgraph/style/style-updater.ts @@ -84,7 +84,7 @@ export class StyleUpdater { resetStyle(bpmnElementIds: string | string[]): void { this.graph.batchUpdate(() => { - if (bpmnElementIds) { + if (bpmnElementIds || bpmnElementIds == '') { for (const id of withCellIdsOfMessageFlowIcons(bpmnElementIds)) { this.styleManager.resetStyleIfIsStored(id); } diff --git a/src/component/registry/css-registry.ts b/src/component/registry/css-registry.ts index d0756fbbe9..51ad0ab850 100644 --- a/src/component/registry/css-registry.ts +++ b/src/component/registry/css-registry.ts @@ -43,7 +43,7 @@ export class CssClassesRegistryImpl implements CssClassesRegistry { } removeAllCssClasses(bpmnElementIds?: string | string[]): void { - if (bpmnElementIds) { + if (bpmnElementIds || bpmnElementIds == '') { for (const bpmnElementId of ensureIsArray(bpmnElementIds)) { const isChanged = this.cssClassesCache.removeAllClassNames(bpmnElementId); this.updateCellIfChanged(isChanged, bpmnElementId); diff --git a/test/integration/mxGraph.model.css.api.test.ts b/test/integration/mxGraph.model.css.api.test.ts index 7fe287188e..89e6828e50 100644 --- a/test/integration/mxGraph.model.css.api.test.ts +++ b/test/integration/mxGraph.model.css.api.test.ts @@ -48,14 +48,13 @@ describe('mxGraph model - CSS API', () => { }); describe('Remove CSS classes - special cases', () => { - const emptyArray: string[] = []; - it.each([null, undefined, emptyArray])('Remove CSS classes with parameter: %s', (bpmnElementIds: string) => { + it.each([null, undefined, '', []])('Remove CSS classes with parameter: %s', (bpmnElementIds: string | string[]) => { // ensure we pass an empty array if (bpmnElementIds) { // eslint-disable-next-line jest/no-conditional-expect -- here we only validate the test parameter - expect(emptyArray).toBeArray(); + expect(bpmnElementIds).toBeArray(); // eslint-disable-next-line jest/no-conditional-expect - expect(emptyArray).toHaveLength(0); + expect(bpmnElementIds).toHaveLength(0); } bpmnElementsRegistry.addCssClasses(['userTask_2_2', 'sequenceFlow_lane_3_elt_3'], ['class1', 'class2']); @@ -79,7 +78,7 @@ describe('mxGraph model - CSS API', () => { }); describe('Remove all CSS classes - special cases', () => { - it.each([null, undefined])('Remove all CSS classes with nullish parameter: %s', (nullishResetParameter: string) => { + it.each([null, undefined])('Remove all CSS classes with a nullish parameter: %s', (nullishResetParameter: string) => { bpmnElementsRegistry.addCssClasses(['userTask_2_2', 'sequenceFlow_lane_3_elt_3'], ['class1', 'class2']); bpmnElementsRegistry.removeAllCssClasses(nullishResetParameter); @@ -98,11 +97,11 @@ describe('mxGraph model - CSS API', () => { }); }); - it('Remove all CSS classes with an empty array', () => { + it.each(['', []])('Remove all CSS classes with an empty parameter: %s', (emptyParameter: string | string[]) => { bpmnElementsRegistry.addCssClasses(['userTask_2_2', 'sequenceFlow_lane_3_elt_3'], ['class#1', 'class#2']); // should have no effect - bpmnElementsRegistry.removeAllCssClasses([]); + bpmnElementsRegistry.removeAllCssClasses(emptyParameter); expect('userTask_2_2').toBeUserTask({ extraCssClasses: ['class#1', 'class#2'], diff --git a/test/integration/mxGraph.model.style.api.test.ts b/test/integration/mxGraph.model.style.api.test.ts index 1ad73cf9d4..a0848418dc 100644 --- a/test/integration/mxGraph.model.style.api.test.ts +++ b/test/integration/mxGraph.model.style.api.test.ts @@ -1078,7 +1078,7 @@ describe('mxGraph model - reset style', () => { describe('Reset style - special cases', () => { const bpmnElementsRegistry = bpmnVisualization.bpmnElementsRegistry; - it.each([null, undefined])('Reset style with nullish parameter: %s', (nullishResetParameter: string) => { + it.each([null, undefined])('Reset style with a nullish parameter: %s', (nullishResetParameter: string) => { // Apply custom style const customStyle: StyleUpdate = { font: { color: 'blue' }, @@ -1102,7 +1102,7 @@ describe('mxGraph model - reset style', () => { }); }); - it('Reset style with an empty array', () => { + it.each(['', []])('Reset style with an empty parameter: %s', (emptyParameter: string | string[]) => { // Apply custom style const customStyle: StyleUpdate = { opacity: 25, @@ -1110,7 +1110,7 @@ describe('mxGraph model - reset style', () => { bpmnElementsRegistry.updateStyle(['startEvent_lane_1', 'sequenceFlow_lane_1_elt_1'], customStyle); // Reset style. It should have no effect. - bpmnElementsRegistry.resetStyle([]); + bpmnElementsRegistry.resetStyle(emptyParameter); // Check that the style has been reset to default values for each element expect('startEvent_lane_1').toBeStartEvent({