Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(API): prevent a reset/remove when passing an empty string #2900

Merged
merged 2 commits into from
Oct 2, 2023
Merged

fix(API): prevent a reset/remove when passing an empty string #2900

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50fe18b
fix(API): do not reset/remove when passing an empty string
tbouffard Sep 29, 2023
ad2dfb5
Merge branch 'master' into fix/check_empty_string_parameter
tbouffard Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/component/mxgraph/style/style-updater.ts
View file
Open in desktop
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 == '') {
Copy link
Member Author

@tbouffard tbouffard Oct 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: we may introduce a function to manage this check
It could improve readability while eliminating duplication (which is not really mandatory here).

for (const id of withCellIdsOfMessageFlowIcons(bpmnElementIds)) {
this.styleManager.resetStyleIfIsStored(id);
}
Expand Down
2 changes: 1 addition & 1 deletion src/component/registry/css-registry.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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