Skip to content

fix!: update multi-select-combo-box to synchronize on change#9631

Merged
web-padawan merged 5 commits into
mainfrom
fix/9611-mscb-spurious-value-change
Jul 6, 2026
Merged

fix!: update multi-select-combo-box to synchronize on change#9631
web-padawan merged 5 commits into
mainfrom
fix/9611-mscb-spurious-value-change

Conversation

@web-padawan

@web-padawan web-padawan commented Jun 29, 2026

Copy link
Copy Markdown
Member

Description

Fixes #9611

With @Push and two push frames around a server-side setValue, MultiSelectComboBox could reset its value to empty because the selected-items-changed notification also fires for programmatic changes and gets echoed back as a spurious client value change.

  • Synchronized the value on the web component change event instead of selected-items-changed, so only genuine user gestures are treated as client changes
  • Updated TestBench helpers to dispatch change (and deselectAll to use clear())
  • Added integration tests for the push scenario and user value commits

Warning

Client-side changes to the web component selectedItems property no longer synchronize to the server unless a change event is dispatched. Server-side setValue and end-user interactions are unaffected.

Type of change

  • Behavior altering fix

🤖 Generated with Claude Code

@web-padawan web-padawan changed the title fix: prevent spurious value-change in multi-select-combo-box refactor: update multi-select-combo-box to synchronize on change Jul 6, 2026
@web-padawan web-padawan changed the title refactor: update multi-select-combo-box to synchronize on change fix!: update multi-select-combo-box to synchronize on change Jul 6, 2026
@web-padawan web-padawan force-pushed the fix/9611-mscb-spurious-value-change branch 2 times, most recently from 37d4ab4 to 31673d6 Compare July 6, 2026 07:59
@web-padawan web-padawan marked this pull request as ready for review July 6, 2026 08:00
Synchronize the value on the web component `change` event instead of the
`selected-items-changed` property notification. That notification also fires
for programmatic (server-set) changes, which - with @Push and two push frames
around setValue - could echo a stale empty value back as a spurious
client-initiated value change. The `change` event fires only on genuine user
value commits, so only real user gestures are treated as client changes.

BREAKING CHANGE: client-side changes to the web component `selectedItems`
property no longer synchronize to the server unless a `change` event is
dispatched. Normal server-side setValue and end-user interactions are
unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@web-padawan web-padawan force-pushed the fix/9611-mscb-spurious-value-change branch from 31673d6 to c7266b1 Compare July 6, 2026 08:03
web-padawan and others added 2 commits July 6, 2026 12:03
- Add unit test asserting the value synchronizes on the change event
- Reduce integration tests to cover value propagation via the TestBench
  element helpers
- Use Java text blocks for the TestBench element scripts

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@web-padawan web-padawan force-pushed the fix/9611-mscb-spurious-value-change branch from 7ab1cb0 to 99c8fff Compare July 6, 2026 09:19
@web-padawan web-padawan requested a review from vursen July 6, 2026 09:23
TestBenchElement.getText() already waits for the pending server round trip
via waitForVaadin(), so the extra explicit wait was redundant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…BoxValueChangeIT

The two IT classes covered mostly the same client value change scenarios.
Move the one scenario that was missing (deselectAll) into the existing
class and remove the duplicate.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

@web-padawan web-padawan added this pull request to the merge queue Jul 6, 2026
Merged via the queue into main with commit c9676c1 Jul 6, 2026
18 checks passed
@web-padawan web-padawan deleted the fix/9611-mscb-spurious-value-change branch July 6, 2026 15:01
@vaadin-bot

Copy link
Copy Markdown
Collaborator

Hi @web-padawan and @web-padawan, when i performed cherry-pick to this commit to 25.1, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick c9676c1
error: could not apply c9676c1... fix!: update multi-select-combo-box to synchronize on change (#9631)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

sissbruecker pushed a commit that referenced this pull request Jul 6, 2026
…(CP: 25.2) (#9683)

This PR cherry-picks changes from the original PR #9631 to branch 25.2.
---
#### Original PR description
> ## Description
> 
> Fixes #9611
> 
> With `@Push` and two push frames around a server-side `setValue`,
MultiSelectComboBox could reset its value to empty because the
`selected-items-changed` notification also fires for programmatic
changes and gets echoed back as a spurious client value change.
> 
> - Synchronized the value on the web component `change` event instead
of `selected-items-changed`, so only genuine user gestures are treated
as client changes
> - Updated TestBench helpers to dispatch `change` (and `deselectAll` to
use `clear()`)
> - Added integration tests for the push scenario and user value commits
> 
> > [!WARNING]
> > Client-side changes to the web component `selectedItems` property no
longer synchronize to the server unless a `change` event is dispatched.
Server-side `setValue` and end-user interactions are unaffected.
> 
> ## Type of change
> 
> - Behavior altering fix
> 
> ---
> 
> 🤖 Generated with Claude Code
>

Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>
sissbruecker pushed a commit that referenced this pull request Jul 7, 2026
…1) (#9686)

This PR cherry-picks changes from the original PR #9631 to branch 25.1.

---

> ## Description
>
> Fixes #9611
>
> With `@Push` and two push frames around a server-side `setValue`,
MultiSelectComboBox could reset its value to empty because the
`selected-items-changed` notification also fires for programmatic
changes and gets echoed back as a spurious client value change.
>
> - Synchronized the value on the web component `change` event instead
of `selected-items-changed`, so only genuine user gestures are treated
as client changes
> - Updated TestBench helpers to dispatch `change` (and `deselectAll` to
use `clear()`)
> - Added integration tests for the push scenario and user value commits
>
> > [!WARNING]
> > Client-side changes to the web component `selectedItems` property no
longer synchronize to the server unless a `change` event is dispatched.
Server-side `setValue` and end-user interactions are unaffected.
>
> ## Type of change
>
> - Behavior altering fix
>
> ---
>
> 🤖 Generated with Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MultiSelectComboBox: Spurious client-initiated value-change event resets value when ui.push() is called before setValue()

3 participants