Skip to content

Commit d23c2ac

Browse files
readingdancerCopilotnielslyngsoe
authored
Fixes the Checkbox, Dropdown and Select list when the models change the UI updates. (#19487)
* Fix CheckboxList UI not updating when values are set programmatically * WIP * Added unit tests for the new functionality in the checkbox list element. As requested by Copilot, here are some unit tests to ensure this addition passes all of the possible edge cases mentioned. * Small change based on CoPilot feedback Removed a check that was redundant and removed a unit test that was also not needed for the current PR and fixed one of the other tests. * Fixing code quality issues highlighted in the unit tests * Fix CheckboxList UI not updating when values are set programmatically * WIP * Standardizes property editor UI state management Introduces a utility for managing the state of property editor UI elements when their values are set programmatically. This ensures that UI components like dropdowns, checkbox lists, and selects correctly reflect the selected values, especially when these values are updated via code rather than direct user interaction. The changes include: - A mixin to simplify state updates - A helper function to ensure values are handled as arrays - Consistent state updating logic across components. * Update src/Umbraco.Web.UI.Client/src/packages/property-editors/select/property-editor-ui-select.element.ts Co-authored-by: Copilot <[email protected]> * Removed the hard coded label * Fixed the short-circuit issue raised by co-pilot * Fixing more co-pilot suggestions Also cleaned up the test files based on the JSDocs suggestions. * Update src/Umbraco.Web.UI.Client/src/packages/property-editors/dropdown/property-editor-ui-dropdown.element.ts Co-authored-by: Copilot <[email protected]> * Refactors checkbox and dropdown tests Refactors checkbox-list and dropdown property editor UI tests to share common test utilities, reducing code duplication and improving maintainability. Uses Sets for faster selection lookup in `updateItemsState` function. * Fixing CodeScene suggestion based on "String Heavy Function Arguments" * Fix for an issue that was stopping the Bellissima build. * Improves property editor UI state updates Ensures UI updates in checkbox list, dropdown and select property editors only occur when necessary. Avoids unnecessary re-renders by comparing the updated state with the current state, and only triggering an update if there are actual changes. This improves performance and prevents potential issues caused by excessive re-rendering. * Changes based on feedback from @nielslyngsoe * removing unnecessary call to requestUpdate --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Niels Lyngsø <[email protected]> Co-authored-by: Niels Lyngsø <[email protected]>
1 parent a0aff9d commit d23c2ac

8 files changed

+831
-14
lines changed

‎src/Umbraco.Web.UI.Client/src/packages/property-editors/checkbox-list/property-editor-ui-checkbox-list.element.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { ensureArray, updateItemsSelectedState } from '../utils/property-editor-ui-state-manager.js';
2+
import './components/input-checkbox-list/input-checkbox-list.element.js';
13
import type {
24
UmbCheckboxListItem,
35
UmbInputCheckboxListElement,
@@ -11,8 +13,6 @@ import type {
1113
UmbPropertyEditorUiElement,
1214
} from '@umbraco-cms/backoffice/property-editor';
1315

14-
import './components/input-checkbox-list/input-checkbox-list.element.js';
15-
1616
/**
1717
* @element umb-property-editor-ui-checkbox-list
1818
*/
@@ -28,8 +28,11 @@ export class UmbPropertyEditorUICheckboxListElement
2828

2929
@property({ type: Array })
3030
public override set value(value: Array<string> | string | undefined) {
31-
this.#selection = Array.isArray(value) ? value : value ? [value] : [];
31+
this.#selection = ensureArray(value);
32+
// Update the checked state of existing list items when value changes
33+
this.#updateCheckedState();
3234
}
35+
3336
public override get value(): Array<string> | undefined {
3437
return this.#selection;
3538
}
@@ -89,6 +92,21 @@ export class UmbPropertyEditorUICheckboxListElement
8992
this.dispatchEvent(new UmbChangeEvent());
9093
}
9194

95+
/**
96+
* Updates the checked state of all list items based on current selection.
97+
* This fixes the issue where UI doesn't update when values are set programmatically.
98+
*/
99+
#updateCheckedState() {
100+
// Only update if we have list items loaded
101+
if (this._list.length > 0) {
102+
// Update state only if changes are needed
103+
const updatedList = updateItemsSelectedState(this._list, this.#selection, 'checked');
104+
if (updatedList !== this._list) {
105+
this._list = updatedList;
106+
}
107+
}
108+
}
109+
92110
override render() {
93111
return html`
94112
<umb-input-checkbox-list

‎src/Umbraco.Web.UI.Client/src/packages/property-editors/checkbox-list/property-editor-ui-checkbox-list.test.ts

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,46 @@
11
import { UmbPropertyEditorUICheckboxListElement } from './property-editor-ui-checkbox-list.element.js';
22
import { expect, fixture, html } from '@open-wc/testing';
33
import { type UmbTestRunnerWindow, defaultA11yConfig } from '@umbraco-cms/internal/test-utils';
4+
import {
5+
setupBasicStringConfig,
6+
setupObjectConfig,
7+
setupEmptyConfig,
8+
getCheckboxListElement,
9+
getCheckboxSelection,
10+
verifyMultiSelectValueAndDOM,
11+
MULTI_SELECT_TEST_DATA
12+
} from '../utils/property-editor-test-utils.js';
413

514
describe('UmbPropertyEditorUICheckboxListElement', () => {
615
let element: UmbPropertyEditorUICheckboxListElement;
716

817
beforeEach(async () => {
9-
element = await fixture(html` <umb-property-editor-ui-checkbox-list></umb-property-editor-ui-checkbox-list> `);
18+
element = await fixture(html`<umb-property-editor-ui-checkbox-list></umb-property-editor-ui-checkbox-list>`);
1019
});
1120

21+
// Local helper function to get checked values from DOM (specific to this component)
22+
function getLocalCheckedValues() {
23+
const checkboxListInput = getCheckboxListElement(element);
24+
const checkboxElements = checkboxListInput?.shadowRoot?.querySelectorAll('uui-checkbox') || [];
25+
const checkedValues: string[] = [];
26+
27+
checkboxElements.forEach((checkbox: Element) => {
28+
const uuiCheckbox = checkbox as any;
29+
if (uuiCheckbox.checked) {
30+
checkedValues.push(uuiCheckbox.value);
31+
}
32+
});
33+
34+
return checkedValues;
35+
}
36+
37+
// Local helper function to verify both selection and DOM state
38+
function verifyLocalSelectionAndDOM(expectedSelection: string[], expectedChecked: string[]) {
39+
const checkboxListInput = getCheckboxListElement(element) as { selection?: string[] } | null;
40+
expect(checkboxListInput?.selection).to.deep.equal(expectedSelection);
41+
expect(getLocalCheckedValues().sort()).to.deep.equal(expectedChecked.sort());
42+
}
43+
1244
it('is defined with its own instance', () => {
1345
expect(element).to.be.instanceOf(UmbPropertyEditorUICheckboxListElement);
1446
});
@@ -18,4 +50,106 @@ describe('UmbPropertyEditorUICheckboxListElement', () => {
1850
await expect(element).shadowDom.to.be.accessible(defaultA11yConfig);
1951
});
2052
}
53+
54+
describe('programmatic value setting', () => {
55+
beforeEach(async () => {
56+
setupBasicStringConfig(element);
57+
await element.updateComplete;
58+
});
59+
60+
it('should update UI immediately when value is set programmatically with array', async () => {
61+
element.value = ['Red', 'Blue'];
62+
await element.updateComplete;
63+
64+
expect(getCheckboxListElement(element)).to.exist;
65+
verifyLocalSelectionAndDOM(['Red', 'Blue'], ['Red', 'Blue']);
66+
});
67+
68+
it('should update UI immediately when value is set to empty array', async () => {
69+
// First set some values
70+
element.value = ['Red', 'Green'];
71+
await element.updateComplete;
72+
73+
// Then clear them
74+
element.value = [];
75+
await element.updateComplete;
76+
77+
verifyLocalSelectionAndDOM([], []);
78+
});
79+
80+
it('should update UI immediately when value is set to single string', async () => {
81+
element.value = 'Green';
82+
await element.updateComplete;
83+
84+
verifyLocalSelectionAndDOM(['Green'], ['Green']);
85+
});
86+
87+
it('should handle undefined value gracefully', async () => {
88+
element.value = undefined;
89+
await element.updateComplete;
90+
91+
verifyLocalSelectionAndDOM([], []);
92+
});
93+
94+
it('should handle invalid values gracefully', async () => {
95+
// Set value with invalid option that doesn't exist in the configured list ['Red', 'Green', 'Blue']
96+
element.value = ['Red', 'InvalidColor', 'Blue'];
97+
await element.updateComplete;
98+
99+
// Should preserve all values in selection but only check valid ones in DOM
100+
verifyLocalSelectionAndDOM(['Red', 'InvalidColor', 'Blue'], ['Red', 'Blue']);
101+
});
102+
103+
it('should maintain value consistency between getter and setter', async () => {
104+
const testValue = ['Red', 'Green'];
105+
element.value = testValue;
106+
await element.updateComplete;
107+
108+
expect(element.value).to.deep.equal(testValue);
109+
verifyLocalSelectionAndDOM(testValue, testValue);
110+
});
111+
112+
it('should update multiple times correctly', async () => {
113+
for (const update of MULTI_SELECT_TEST_DATA) {
114+
element.value = update.value;
115+
await element.updateComplete;
116+
verifyLocalSelectionAndDOM(update.expected, update.expected);
117+
}
118+
});
119+
});
120+
121+
describe('configuration handling', () => {
122+
it('should handle string array configuration', async () => {
123+
setupBasicStringConfig(element, ['Option1', 'Option2', 'Option3']);
124+
125+
element.value = ['Option1', 'Option3'];
126+
await element.updateComplete;
127+
128+
verifyLocalSelectionAndDOM(['Option1', 'Option3'], ['Option1', 'Option3']);
129+
});
130+
131+
it('should handle object array configuration', async () => {
132+
setupObjectConfig(element);
133+
134+
element.value = ['red', 'blue'];
135+
await element.updateComplete;
136+
137+
verifyLocalSelectionAndDOM(['red', 'blue'], ['red', 'blue']);
138+
});
139+
140+
it('should handle empty configuration gracefully', async () => {
141+
setupEmptyConfig(element);
142+
143+
element.value = ['test'];
144+
await element.updateComplete;
145+
146+
// Should not throw error
147+
expect(element.value).to.deep.equal(['test']);
148+
149+
// Should have no uui-checkboxes since configuration is empty
150+
const checkboxListInput = getCheckboxListElement(element);
151+
const checkboxElements = checkboxListInput?.shadowRoot?.querySelectorAll('uui-checkbox') || [];
152+
expect(checkboxElements).to.have.length(0);
153+
});
154+
});
21155
});

‎src/Umbraco.Web.UI.Client/src/packages/property-editors/dropdown/property-editor-ui-dropdown.element.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { ensureArray, updateItemsSelectedState } from '../utils/property-editor-ui-state-manager.js';
12
import { css, customElement, html, map, nothing, property, state, when } from '@umbraco-cms/backoffice/external/lit';
23
import { UmbChangeEvent } from '@umbraco-cms/backoffice/event';
34
import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element';
@@ -30,7 +31,9 @@ export class UmbPropertyEditorUIDropdownElement
3031

3132
@property({ type: Array })
3233
public override set value(value: Array<string> | string | undefined) {
33-
this.#selection = this.#ensureValueIsArray(value);
34+
this.#selection = ensureArray(value);
35+
// Update the selected state of existing options when value changes
36+
this.#updateSelectedState();
3437
}
3538
public override get value(): Array<string> | undefined {
3639
return this.#selection;
@@ -97,10 +100,6 @@ export class UmbPropertyEditorUIDropdownElement
97100
}
98101
}
99102

100-
#ensureValueIsArray(value: Array<string> | string | null | undefined): Array<string> {
101-
return Array.isArray(value) ? value : value ? [value] : [];
102-
}
103-
104103
#onChange(event: CustomEvent & { target: UmbInputDropdownListElement }) {
105104
const value = event.target.value as string;
106105
this.#setValue(value ? [value] : []);
@@ -114,12 +113,27 @@ export class UmbPropertyEditorUIDropdownElement
114113

115114
#setValue(value: Array<string> | string | null | undefined) {
116115
if (!value) return;
117-
const selection = this.#ensureValueIsArray(value);
116+
const selection = ensureArray(value);
118117
this._options.forEach((item) => (item.selected = selection.includes(item.value)));
119118
this.value = value;
120119
this.dispatchEvent(new UmbChangeEvent());
121120
}
122121

122+
/**
123+
* Updates the selected state of all options based on current selection.
124+
* This fixes the issue where UI doesn't update when values are set programmatically.
125+
*/
126+
#updateSelectedState() {
127+
// Only update if we have options loaded
128+
if (this._options.length > 0) {
129+
// Update state only if changes are needed
130+
const updatedOptions = updateItemsSelectedState(this._options, this.#selection, 'selected');
131+
if (updatedOptions !== this._options) {
132+
this._options = updatedOptions;
133+
}
134+
}
135+
}
136+
123137
override render() {
124138
return html`
125139
${when(

0 commit comments

Comments
 (0)