Skip to content

Commit

Permalink
fix(API): prevent a reset/remove when passing an empty string (#2900)
Browse files Browse the repository at this point in the history
  • Loading branch information
tbouffard authored Oct 2, 2023
1 parent 4070159 commit 324d4c1
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/component/mxgraph/style/style-updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/component/registry/css-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class CssClassesRegistryImpl implements CssClassesRegistry {
}

removeAllCssClasses(bpmnElementIds?: string | string[]): void {
if (bpmnElementIds) {
if (bpmnElementIds || bpmnElementIds == '') {
for (const bpmnElementId of ensureIsArray<string>(bpmnElementIds)) {
const isChanged = this.cssClassesCache.removeAllClassNames(bpmnElementId);
this.updateCellIfChanged(isChanged, bpmnElementId);
Expand Down
13 changes: 6 additions & 7 deletions test/integration/mxGraph.model.css.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand All @@ -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);
Expand All @@ -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'],
Expand Down
6 changes: 3 additions & 3 deletions test/integration/mxGraph.model.style.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand All @@ -1102,15 +1102,15 @@ 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,
};
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({
Expand Down

0 comments on commit 324d4c1

Please sign in to comment.