fix(ui5-toolbar-select): display not updating on programmatic option selection

[NakataCode]

Problem

Since v2.15.0, changing the selected option programmatically (setting ToolbarSelectOption.selected = true/false) correctly updated the outer ui5-toolbar-select-option elements but left the inner ui5-select display frozen on the initially selected option.

Two root causes

1. ToolbarSelectOption had a custom getter reading hasAttribute("selected"). The framework sets the DOM attribute before calling the property setter, so change detection always saw old === new and skipped invalidation — meaning ToolbarSelect never re-rendered on programmatic changes.

2. The template passed value={this.value} to the inner <Select>, feeding _valueStorage on every render. Once set, Select._applySelectionByValue locks the display to the stored string and ignores the selected attribute on inner options entirely.

Solution

• ToolbarSelectOption — revert to plain @property, removing the broken getter/setter. Framework change detection now works correctly.

• ToolbarSelectTemplate — remove value={this.value} from inner <Select>. Selection is driven by selected on inner <Option> elements as rendered by the template.

• ToolbarSelect:

  • value setter: forwards directly to the inner select when rendered, including empty string to clear selection; stages in _value pre-render only.
  • value getter: read directly from the selected option, no _valueStorage dependency.
  • onBeforeRendering: enforce single-selection (last selected wins), guarded to avoid unnecessary writes that could trigger re-render loops.
  • onAfterRendering: deferred apply for explicit value= API only, clears _value unconditionally after applying to prevent stale overrides.
  • _syncOptions: clear _value on user interaction so a prior explicit value= assignment does not override the user's choice.

Fixes: #12619

[sap-ui5-webcomponents-release]

🚀 Deployed on https://pr-13751--ui5-webcomponents-preview.netlify.app

[PetyaMarkovaBogdanova]

Findings

High: explicit clearing via value = "" no longer updates the inner select once rendered
ToolbarSelect.ts:152 returns early for falsy values and never forwards empty string to the rendered inner Select.
Impact: after the component is mounted, setting value to empty string can leave the old visual selection in place, which is a behavioral regression for programmatic API use.

High: stale _value can override later programmatic selected changes
In ToolbarSelect.ts:164, getter prioritizes _value when present.
In ToolbarSelect.ts:220, _value is cleared only when select.value !== _value.
If initial value matches current select.value on first render, _value stays set. A later selected change on options can then be overwritten in ToolbarSelect.ts:221, forcing old _value back into the inner Select. This can reintroduce a display/source mismatch in mixed usage flows.

Medium: regression tests do not cover the two risky value lifecycle paths above
The added test in ToolbarSelect.cy.tsx:348 validates multi-selected normalization and inner option sync, but there is no coverage for:

setting value to empty string after render
initializing with value, then changing selected programmatically later (stale _value path)
Open Questions / Assumptions

Should value = "" be treated as a real API command to clear selection post-render, or only as init-time absence?
Is _value intended to be strictly pre-render staging only? If yes, it should be cleared deterministically after first render pass, not conditionally.
Change Summary

The core direction is good and addresses the reported root causes: removing custom selected accessors in ToolbarSelectOption and dropping value forwarding in the template are both correct. The remaining risk is concentrated in ToolbarSelect value staging/clearing logic, which currently introduces two high-probability edge regressions.

[PetyaMarkovaBogdanova]

Standards
Hard violations

1. DOM mutation via this.select.value = — ToolbarSelect.ts:159 and ToolbarSelect.ts:221
   Rule: "querySelector is allowed only for calling methods like .focus(). Always modify child component state through the template."
   The value setter and onAfterRendering both write directly to the result of a querySelector. Removing value={this.value} from the template was correct, but it was replaced with imperative mutation — a different instance of the same documented anti-pattern.

2. option.selected = shouldBeSelected in onBeforeRendering — ToolbarSelect.ts:211
   Rule: "Never change public properties programmatically — only in response to user interaction."
   This runs in a lifecycle method, not an interaction handler. It also exhibits Feature Envy: ToolbarSelect is walking children and mutating their public state to enforce single-selection logic that belongs in the option or the template.

3. Tag selectors in tests — ToolbarSelect.cy.tsx:629 and ToolbarSelect.cy.tsx:656
   Rule: "Always use attribute selectors."
   doc.querySelector("ui5-toolbar-select") and document.querySelectorAll("ui5-toolbar-select-option") should be [ui5-toolbar-select] / [ui5-toolbar-select-option].

4. _value is an undecorated plain field — ToolbarSelect.ts:180
   Rule: "Use noAttribute: true for private/internal properties not used in CSS selectors."
   Without @Property({ noAttribute: true }), _value is not reactive. Re-renders triggered by it are incidental to other reactive changes — a fragile implicit dependency.

Code smells (judgement calls)
Mysterious Name — _value is a staging buffer for a not-yet-rendered value; _pendingValue or _stagedValue would communicate the lifecycle semantics.
Duplicated Logic — the pattern this.select.value = x; this._value = "" appears in both the setter's live-select branch and in onAfterRendering; the two paths could share a helper.
Message Chain — selectedOption?.textContent || this.select?.value || "" in the getter navigates three nullable authorities with no single source of truth declared.
Spec
Missing / partial

1. value = "" is silently lost when staged pre-render (your High 1)
   Spec: "value setter: forwards directly to the inner select when rendered, including empty string to clear selection; stages in _value pre-render only."
   onAfterRendering guards on if (this._value && this.select). Because "" is falsy, a pre-render value = "" is staged but then unconditionally skipped. The test covers the post-render case only; the pre-render staging path for empty string is never applied.

2. Getter still depends on _value (your High 2)
   Spec: "value getter: read directly from the selected option, no _valueStorage dependency."
   The getter at ToolbarSelect.ts:168 returns _value first when truthy. During the window between a value= write and onAfterRendering, the getter returns the staged string rather than the actual selection — the same class of stale-read the spec set out to eliminate.

3. Missing regression coverage (your Medium)
   The new tests validate multi-option normalization and the post-render value = "" case, but there is no test for: (a) value="" set before first render, and (b) initializing with value="X" then changing option.selected programmatically before _value is cleared.

Scope creep — not in spec
New ToolbarSelectOption.value property (@SInCE 2.25.0) — not mentioned in the spec; adds public API surface.
selectedToolbarOption field on ToolbarSelectChangeEventDetail — event detail extended with a new typed field, also @SInCE 2.25.0; not in the bug spec.
Implemented but looks wrong
value getter reads textContent instead of option.value — ToolbarSelect.ts:172
The getter returns selectedOption?.textContent, but the same diff introduces a ToolbarSelectOption.value property. If a consumer sets option.value = "foo" (the new API) and that option is selected, ToolbarSelect.value returns the label text, not "foo" — the two new features directly contradict each other.

onBeforeRendering writes to child selected can schedule a re-render loop — ToolbarSelect.ts:211
The spec says: "guarded to avoid unnecessary writes that could trigger re-render loops." The guard (if (option.selected !== shouldBeSelected)) reduces writes but does not eliminate the risk when multiple options need correcting simultaneously, since each write invalidates the option (a tracked slot), which can re-queue the parent.

Summary: Standards — 4 hard violations, 3 smells; worst is imperative DOM mutation replacing the template binding the fix was meant to restore. Spec — 2 high gaps (empty-string staging and getter _value priority), 2 scope-creep additions, 1 internal contradiction between the new option.value property and the getter's textContent fallback.