Skip to content

Block Editors: avoid discard changes for no changes (Fixes #20680) #20941

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

Merged
nielslyngsoe merged 3 commits into from
Nov 24, 2025
Merged

Block Editors: avoid discard changes for no changes (Fixes #20680) #20941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
08a8ad3
ensure Block List only updates if it has an update
nielslyngsoe Oct 2, 2025
ddf44b9
ensures RTE and Grid Block Editor ony updates value if there is a change
nielslyngsoe Oct 2, 2025
4c5cd24
Added tests for regression issues
andr317c Nov 24, 2025
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
11 changes: 9 additions & 2 deletions ...ck/block-grid/property-editors/block-grid-editor/property-editor-ui-block-grid.element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
UmbPropertyEditorUiElement,
UmbPropertyEditorConfigCollection,
} from '@umbraco-cms/backoffice/property-editor';
import { observeMultiple } from '@umbraco-cms/backoffice/observable-api';
import { jsonStringComparison, observeMultiple } from '@umbraco-cms/backoffice/observable-api';
import { UMB_PROPERTY_CONTEXT, UMB_PROPERTY_DATASET_CONTEXT } from '@umbraco-cms/backoffice/property';
import { UmbFormControlMixin, UmbValidationContext } from '@umbraco-cms/backoffice/validation';
import type { UmbBlockTypeGroup } from '@umbraco-cms/backoffice/block-type';
Expand Down Expand Up @@ -181,15 +181,22 @@
]).pipe(debounceTime(20)),
([layouts, contents, settings, exposes]) => {
if (layouts.length === 0) {
if (this.value === undefined) {
return;
}
super.value = undefined;
} else {
super.value = {
const newValue = {
...super.value,
layout: { [UMB_BLOCK_GRID_PROPERTY_EDITOR_SCHEMA_ALIAS]: layouts },
contentData: contents,
settingsData: settings,
expose: exposes,
};
if (jsonStringComparison(this.value, newValue)) {
return;
}
super.value = newValue;

Check warning on line 199 in src/Umbraco.Web.UI.Client/src/packages/block/block-grid/property-editors/block-grid-editor/property-editor-ui-block-grid.element.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (release/16.4)

❌ Getting worse: Complex Method 

UmbPropertyEditorUIBlockGridElement.constructor increases in cyclomatic complexity from 20 to 22, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check warning on line 199 in src/Umbraco.Web.UI.Client/src/packages/block/block-grid/property-editors/block-grid-editor/property-editor-ui-block-grid.element.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (release/16.4)

❌ New issue: Bumpy Road Ahead 

UmbPropertyEditorUIBlockGridElement.constructor has 3 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
}

// If we don't have a value set from the outside or an internal value, we don't want to set the value.
Expand Down
11 changes: 9 additions & 2 deletions ...ck/block-list/property-editors/block-list-editor/property-editor-ui-block-list.element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
UmbFormControlMixin,
UmbValidationContext,
} from '@umbraco-cms/backoffice/validation';
import { observeMultiple } from '@umbraco-cms/backoffice/observable-api';
import { jsonStringComparison, observeMultiple } from '@umbraco-cms/backoffice/observable-api';
import { debounceTime } from '@umbraco-cms/backoffice/external/rxjs';
import { UMB_CONTENT_WORKSPACE_CONTEXT } from '@umbraco-cms/backoffice/content';

Expand Down Expand Up @@ -339,15 +339,22 @@ export class UmbPropertyEditorUIBlockListElement
]).pipe(debounceTime(20)),
([layouts, contents, settings, exposes]) => {
if (layouts.length === 0) {
if (this.value === undefined) {
return;
}
super.value = undefined;
} else {
super.value = {
const newValue = {
...super.value,
layout: { [UMB_BLOCK_LIST_PROPERTY_EDITOR_SCHEMA_ALIAS]: layouts },
contentData: contents,
settingsData: settings,
expose: exposes,
};
if (jsonStringComparison(this.value, newValue)) {
return;
}
super.value = newValue;
}

// If we don't have a value set from the outside or an internal value, we don't want to set the value.
Expand Down
17 changes: 14 additions & 3 deletions src/Umbraco.Web.UI.Client/src/packages/rte/components/rte-base.element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { UmbPropertyEditorRteValueType } from '../types.js';
import { UMB_BLOCK_RTE_PROPERTY_EDITOR_SCHEMA_ALIAS } from '../constants.js';
import { observeMultiple } from '@umbraco-cms/backoffice/observable-api';
import { jsonStringComparison, observeMultiple } from '@umbraco-cms/backoffice/observable-api';
import { property, state } from '@umbraco-cms/backoffice/external/lit';
import { UmbBlockRteEntriesContext, UmbBlockRteManagerContext } from '@umbraco-cms/backoffice/block-rte';
import { UmbChangeEvent } from '@umbraco-cms/backoffice/event';
Expand Down Expand Up @@ -243,9 +243,12 @@ export abstract class UmbPropertyEditorUiRteElementBase
([layouts, contents, settings, exposes]) => {
if (layouts.length === 0) {
if (super.value?.markup === undefined) {
if (this.value === undefined) {
return;
}
super.value = undefined;
} else {
super.value = {
const newValue = {
...super.value,
blocks: {
layout: {},
Expand All @@ -254,9 +257,13 @@ export abstract class UmbPropertyEditorUiRteElementBase
expose: [],
},
};
if (jsonStringComparison(this.value, newValue)) {
return;
}
super.value = newValue;
}
} else {
super.value = {
const newValue = {
markup: this._markup,
blocks: {
layout: { [UMB_BLOCK_RTE_PROPERTY_EDITOR_SCHEMA_ALIAS]: layouts },
Expand All @@ -265,6 +272,10 @@ export abstract class UmbPropertyEditorUiRteElementBase
expose: exposes,
},
};
if (jsonStringComparison(this.value, newValue)) {
return;
}
super.value = newValue;
}

// If we don't have a value set from the outside or an internal value, we don't want to set the value.
Expand Down
19 changes: 19 additions & 0 deletions ...o.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/ContentWithBlockGrid.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,22 @@ test('can add a variant block element with invariant RTE Tiptap in the content',
await umbracoApi.documentType.ensureNameNotExists(customElementTypeName);
await umbracoApi.language.ensureNameNotExists('Danish');
});

// Tests regression issue: https://github.com/umbraco/Umbraco-CMS/issues/20680
test('can move away from a content node with a block grid after making no changes without seeing discard unsaved changes', {tag: '@smoke'}, async ({umbracoApi, umbracoUi}) => {
// Arrange
const customDataTypeId = await umbracoApi.dataType.createBlockGridWithPermissions(customDataTypeName, elementTypeId, true, true);
const documentTypeId = await umbracoApi.documentType.createDocumentTypeWithPropertyEditor(documentTypeName, customDataTypeName, customDataTypeId);
await umbracoApi.document.createDefaultDocument(contentName, documentTypeId);
await umbracoUi.goToBackOffice();
await umbracoUi.content.goToSection(ConstantHelper.sections.content);
await umbracoUi.content.goToContentWithName(contentName);

// Act
await umbracoUi.documentType.goToSection(ConstantHelper.sections.settings);

// Assert
// We do this to make sure that there is no discard changes button visible, if the discard changes was visible, we would not be able to go to the document type
await umbracoUi.documentType.goToDocumentType(documentTypeName);
});

18 changes: 18 additions & 0 deletions ...o.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/ContentWithBlockList.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,3 +364,21 @@ test('can add a variant block element with invariant RTE Tiptap in the content',
await umbracoApi.documentType.ensureNameNotExists(customElementTypeName);
await umbracoApi.language.ensureNameNotExists('Danish');
});

// Tests regression issue: https://github.com/umbraco/Umbraco-CMS/issues/20680
test('can move away from a content node with a block list after making no changes without seeing discard unsaved changes', {tag: '@smoke'}, async ({umbracoApi, umbracoUi}) => {
// Arrange
const customDataTypeId = await umbracoApi.dataType.createBlockListDataTypeWithABlock(customDataTypeName, elementTypeId);
const documentTypeId = await umbracoApi.documentType.createDocumentTypeWithPropertyEditor(documentTypeName, customDataTypeName, customDataTypeId);
await umbracoApi.document.createDefaultDocument(contentName, documentTypeId);
await umbracoUi.goToBackOffice();
await umbracoUi.content.goToSection(ConstantHelper.sections.content);
await umbracoUi.content.goToContentWithName(contentName);

// Act
await umbracoUi.documentType.goToSection(ConstantHelper.sections.settings);

// Assert
// We do this to make sure that there is no discard changes button visible, if the discard changes was visible, we would not be able to go to the document type
await umbracoUi.documentType.goToDocumentType(documentTypeName);
});
Loading