Skip to content

Commit 620759f

Browse files
committed
fix(ui5-toolbar-select): sync value when option selected property changes programmatically
1 parent b6ba0f0 commit 620759f

2 files changed

Lines changed: 89 additions & 10 deletions

File tree

packages/main/cypress/specs/ToolbarSelect.cy.tsx

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import Toolbar from "../../src/Toolbar.js";
22
import ToolbarSelect from "../../src/ToolbarSelect.js";
33
import ToolbarSelectOption from "../../src/ToolbarSelectOption.js";
4+
import Button from "../../src/Button.js";
45

56
describe("Toolbar general interaction", () => {
67
it("Should render the select with the correct attributes", () => {
@@ -264,10 +265,10 @@ describe("Toolbar general interaction", () => {
264265
cy.mount(
265266
<Toolbar id="otb_d">
266267
<ToolbarSelect style="width: 201px;" id="toolbar-select">
267-
<ToolbarSelectOption>1</ToolbarSelectOption>
268-
<ToolbarSelectOption selected>2</ToolbarSelectOption>
269-
<ToolbarSelectOption>3</ToolbarSelectOption>
270-
</ToolbarSelect>
268+
<ToolbarSelectOption>1</ToolbarSelectOption>
269+
<ToolbarSelectOption selected>2</ToolbarSelectOption>
270+
<ToolbarSelectOption>3</ToolbarSelectOption>
271+
</ToolbarSelect>
271272
</Toolbar>
272273
);
273274
cy.viewport(220, 1080); // Set a small viewport width to trigger overflow
@@ -282,4 +283,47 @@ describe("Toolbar general interaction", () => {
282283
// Verify the toolbar-select is rendered inside the popover
283284
cy.get("ui5-toolbar-select").should("be.visible");
284285
});
286+
287+
it("Should update ToolbarSelect value when option selected property changes via button click", () => {
288+
cy.mount(
289+
<>
290+
<Toolbar>
291+
<ToolbarSelect id="testSelect">
292+
<ToolbarSelectOption id="opt1" selected>Option 1</ToolbarSelectOption>
293+
<ToolbarSelectOption id="opt2">Option 2</ToolbarSelectOption>
294+
</ToolbarSelect>
295+
</Toolbar>
296+
<Button id="btnNext">Next Step</Button>
297+
</>
298+
);
299+
300+
// Wait for component to render
301+
cy.wait(500);
302+
303+
// Initial check - Option 1 should be selected
304+
cy.get("[ui5-toolbar]")
305+
.find("[ui5-toolbar-select]")
306+
.should("have.prop", "value", "Option 1");
307+
308+
// Set up button click handler
309+
cy.get("[ui5-button]").then($btn => {
310+
$btn.get(0).addEventListener("click", () => {
311+
const opt1 = document.getElementById("opt1") as ToolbarSelectOption;
312+
const opt2 = document.getElementById("opt2") as ToolbarSelectOption;
313+
opt1.selected = false;
314+
opt2.selected = true;
315+
});
316+
});
317+
318+
// Click the button
319+
cy.get("[ui5-button]").realClick();
320+
321+
// Wait for update
322+
cy.wait(200);
323+
324+
// Verify the ToolbarSelect value property updated
325+
cy.get("[ui5-toolbar]")
326+
.find("[ui5-toolbar-select]")
327+
.should("have.prop", "value", "Option 2");
328+
});
285329
});

packages/main/src/ToolbarSelect.ts

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import property from "@ui5/webcomponents-base/dist/decorators/property.js";
44
import slot from "@ui5/webcomponents-base/dist/decorators/slot.js";
55
import event from "@ui5/webcomponents-base/dist/decorators/event-strict.js";
66
import type ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js";
7+
import type { ChangeInfo } from "@ui5/webcomponents-base/dist/UI5Element.js";
78
import ToolbarSelectCss from "./generated/themes/ToolbarSelect.css.js";
89
import type Select from "./Select.js";
910

@@ -91,6 +92,7 @@ class ToolbarSelect extends ToolbarItem {
9192
@slot({
9293
"default": true,
9394
type: HTMLElement,
95+
invalidateOnChildChange: true,
9496
})
9597
options!: Array<ToolbarSelectOption>;
9698

@@ -146,14 +148,19 @@ class ToolbarSelect extends ToolbarItem {
146148
*/
147149
@property()
148150
set value(newValue: string) {
149-
if (this.select && this.select.value !== newValue) {
150-
this.select.value = newValue;
151-
}
152151
this._value = newValue;
152+
const selectElement = this.select;
153+
if (selectElement && selectElement.value !== newValue) {
154+
selectElement.value = newValue;
155+
}
153156
}
154157

155158
get value(): string | undefined {
156-
return this.select ? this.select.value : this._value;
159+
// Always return _value if it's set, as it represents the source of truth
160+
if (this._value !== undefined && this._value !== "") {
161+
return this._value;
162+
}
163+
return this.select ? this.select.value : undefined;
157164
}
158165

159166
get select(): Select | null {
@@ -163,6 +170,30 @@ class ToolbarSelect extends ToolbarItem {
163170
// Internal value storage, in case the composite select is not rendered on the the assignment happens
164171
_value: string = "";
165172

173+
onInvalidation(changeInfo: ChangeInfo) {
174+
// When a child ToolbarSelectOption's selected property changes, update the value
175+
if (changeInfo.reason === "childchange") {
176+
const selectedOption = this.options.find(option => option.selected);
177+
if (selectedOption) {
178+
const newValue = selectedOption.textContent || "";
179+
// Update both internal value and the select component's value
180+
this._value = newValue;
181+
// Cache the select reference to avoid multiple DOM queries
182+
const selectElement = this.select;
183+
if (selectElement && selectElement.value !== newValue) {
184+
selectElement.value = newValue;
185+
}
186+
} else {
187+
// If no option is selected, clear the value
188+
this._value = "";
189+
const selectElement = this.select;
190+
if (selectElement) {
191+
selectElement.value = "";
192+
}
193+
}
194+
}
195+
}
196+
166197
onClick(e: Event): void {
167198
e.stopImmediatePropagation();
168199
const prevented = !this.fireDecoratorEvent("click", { targetRef: e.target as HTMLElement });
@@ -189,12 +220,16 @@ class ToolbarSelect extends ToolbarItem {
189220

190221
onChange(e: CustomEvent<SelectChangeEventDetail>): void {
191222
e.stopImmediatePropagation();
223+
224+
// Update internal value BEFORE firing the change event
225+
// so that when event listeners read this.value, they get the updated value
226+
this._value = e.detail.selectedOption?.textContent || "";
227+
this._syncOptions(e.detail.selectedOption);
228+
192229
const prevented = !this.fireDecoratorEvent("change", { ...e.detail, targetRef: e.target as HTMLElement });
193230
if (!prevented) {
194231
this.fireDecoratorEvent("close-overflow");
195232
}
196-
197-
this._syncOptions(e.detail.selectedOption);
198233
}
199234

200235
_syncOptions(selectedOption: HTMLElement): void {

0 commit comments

Comments
 (0)