-
Notifications
You must be signed in to change notification settings - Fork 78
fix!: update multi-select-combo-box to synchronize on change #9631
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.
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
Changes from 1 commit
c7266b1
5040694
99c8fff
94ef8a9
6b72649
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /* | ||
| * Copyright 2000-2026 Vaadin Ltd. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| * use this file except in compliance with the License. You may obtain a copy of | ||
| * the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
| package com.vaadin.flow.component.combobox.test; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| import com.vaadin.flow.component.UI; | ||
| import com.vaadin.flow.component.combobox.MultiSelectComboBox; | ||
| import com.vaadin.flow.component.html.Div; | ||
| import com.vaadin.flow.component.html.NativeButton; | ||
| import com.vaadin.flow.router.Route; | ||
| import com.vaadin.flow.shared.communication.PushMode; | ||
|
|
||
| /** | ||
| * Reproduces the scenario from | ||
| * <a href="https://github.com/vaadin/flow-components/issues/9611">#9611</a>: | ||
| * with push enabled, two separate push frames around a server-side | ||
| * {@code setValue} must not relay a spurious client-initiated value change with | ||
| * an empty value back to the server. | ||
| */ | ||
| @Route("vaadin-multi-select-combo-box/push-value-change") | ||
| public class MultiSelectComboBoxPushValueChangePage extends Div { | ||
| public MultiSelectComboBoxPushValueChangePage() { | ||
| UI.getCurrent().getPushConfiguration().setPushMode(PushMode.AUTOMATIC); | ||
|
|
||
| Div log = new Div(); | ||
| log.setId("value-change-log"); | ||
|
|
||
| NativeButton runScenario = new NativeButton("Run #9611 scenario", | ||
| event -> { | ||
| MultiSelectComboBox<String> comboBox = new MultiSelectComboBox<>(); | ||
| comboBox.setItems(List.of("1", "2", "3")); | ||
|
|
||
| comboBox.addValueChangeListener(e -> { | ||
| Div entry = new Div(); | ||
| entry.setText("isFromClient=%s old=%s new=%s value=%s" | ||
| .formatted(e.isFromClient(), e.getOldValue(), | ||
| e.getValue(), comboBox.getValue())); | ||
| log.add(entry); | ||
| }); | ||
|
|
||
| add(comboBox); | ||
|
|
||
| UI ui = UI.getCurrent(); | ||
| ui.push(); | ||
| comboBox.setValue(Set.of("1")); | ||
| ui.push(); | ||
| }); | ||
| runScenario.setId("run-scenario"); | ||
|
|
||
| add(runScenario, log); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /* | ||
| * Copyright 2000-2026 Vaadin Ltd. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| * use this file except in compliance with the License. You may obtain a copy of | ||
| * the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
| package com.vaadin.flow.component.combobox.test; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import com.vaadin.flow.component.combobox.MultiSelectComboBox; | ||
| import com.vaadin.flow.component.html.Div; | ||
| import com.vaadin.flow.component.html.Span; | ||
| import com.vaadin.flow.router.Route; | ||
|
|
||
| /** | ||
| * View used to verify that genuine user value commits (dropdown selection and | ||
| * deselection, clear-button click, chip removal, Esc-clear) still propagate the | ||
| * value to the server after the value sync was switched from the | ||
| * {@code selected-items-changed} property notification to the {@code change} | ||
| * event (see | ||
| * <a href="https://github.com/vaadin/flow-components/issues/9611">#9611</a>). | ||
| */ | ||
| @Route("vaadin-multi-select-combo-box/user-value-change") | ||
| public class MultiSelectComboBoxUserValueChangePage extends Div { | ||
| public MultiSelectComboBoxUserValueChangePage() { | ||
| MultiSelectComboBox<String> comboBox = new MultiSelectComboBox<>( | ||
| "Items"); | ||
| comboBox.setItems(List.of("Item 1", "Item 2", "Item 3")); | ||
| comboBox.setWidth("500px"); | ||
| // Clear button must be visible for the clear-button and Esc-to-clear | ||
| // user paths to clear the selection. | ||
| comboBox.setClearButtonVisible(true); | ||
|
|
||
| Span eventValue = new Span(); | ||
| eventValue.setId("event-value"); | ||
| Span eventOrigin = new Span(); | ||
| eventOrigin.setId("event-origin"); | ||
| Span eventCount = new Span("0"); | ||
| eventCount.setId("event-count"); | ||
|
|
||
| comboBox.addValueChangeListener(e -> { | ||
|
vursen marked this conversation as resolved.
Outdated
|
||
| eventValue.setText(String.join(",", e.getValue())); | ||
| eventOrigin.setText(e.isFromClient() ? "client" : "server"); | ||
| eventCount.setText( | ||
| String.valueOf(Integer.parseInt(eventCount.getText()) + 1)); | ||
| }); | ||
|
|
||
| add(comboBox); | ||
| add(new Div(new Span("Event value: "), eventValue)); | ||
| add(new Div(new Span("Event origin: "), eventOrigin)); | ||
| add(new Div(new Span("Event count: "), eventCount)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| /* | ||
| * Copyright 2000-2026 Vaadin Ltd. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| * use this file except in compliance with the License. You may obtain a copy of | ||
| * the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
| package com.vaadin.flow.component.combobox.test; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.junit.Assert; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
|
|
||
| import com.vaadin.flow.testutil.TestPath; | ||
| import com.vaadin.testbench.TestBenchElement; | ||
| import com.vaadin.tests.AbstractComponentIT; | ||
|
|
||
| /** | ||
| * Regression test for | ||
| * <a href="https://github.com/vaadin/flow-components/issues/9611">#9611</a>: | ||
| * with {@code @Push} enabled, a server-side {@code setValue} surrounded by two | ||
| * push frames must not produce a spurious client-initiated value change with an | ||
| * empty value. | ||
| */ | ||
| @TestPath("vaadin-multi-select-combo-box/push-value-change") | ||
| public class MultiSelectComboBoxPushValueChangeIT extends AbstractComponentIT { | ||
| private TestBenchElement runScenario; | ||
| private TestBenchElement log; | ||
|
|
||
| @Before | ||
| public void init() { | ||
| open(); | ||
| runScenario = $("button").waitForFirst(); | ||
|
Check warning on line 43 in vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/MultiSelectComboBoxPushValueChangeIT.java
|
||
| log = $("div").id("value-change-log"); | ||
| } | ||
|
|
||
| @Test | ||
| public void setValueWithPushFrames_noSpuriousEmptyClientValueChange() { | ||
| runScenario.click(); | ||
|
|
||
| // Wait for the value change triggered by the server-side setValue | ||
| waitUntil(driver -> !log.getText().isEmpty()); | ||
|
|
||
| List<String> entries = getLogEntries(); | ||
|
|
||
| // No client-initiated value change with an empty value must occur | ||
| for (String entry : entries) { | ||
| boolean isFromClient = entry.contains("isFromClient=true"); | ||
| boolean isEmptyValue = entry.contains("value=[]"); | ||
| Assert.assertFalse( | ||
| "Spurious client-initiated value change with empty value: " | ||
| + entry, | ||
| isFromClient && isEmptyValue); | ||
| } | ||
|
|
||
| // The final server value must remain ["1"] | ||
| String last = entries.get(entries.size() - 1); | ||
| Assert.assertTrue( | ||
| "Expected final value to remain [1], but log was: " + entries, | ||
| last.contains("value=[1]")); | ||
| } | ||
|
|
||
| private List<String> getLogEntries() { | ||
| return log.$("div").all().stream().map(TestBenchElement::getText) | ||
| .toList(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| /* | ||
| * Copyright 2000-2026 Vaadin Ltd. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| * use this file except in compliance with the License. You may obtain a copy of | ||
| * the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
| package com.vaadin.flow.component.combobox.test; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.junit.Assert; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
| import org.openqa.selenium.Keys; | ||
|
|
||
| import com.vaadin.flow.component.combobox.testbench.MultiSelectComboBoxElement; | ||
| import com.vaadin.flow.testutil.TestPath; | ||
| import com.vaadin.testbench.TestBenchElement; | ||
| import com.vaadin.tests.AbstractComponentIT; | ||
|
|
||
| /** | ||
| * Verifies that genuine user value commits still propagate to the server after | ||
| * the value sync was switched from the {@code selected-items-changed} property | ||
| * notification to the {@code change} event (see | ||
| * <a href="https://github.com/vaadin/flow-components/issues/9611">#9611</a>). | ||
| */ | ||
| @TestPath("vaadin-multi-select-combo-box/user-value-change") | ||
| public class MultiSelectComboBoxUserValueChangeIT extends AbstractComponentIT { | ||
|
web-padawan marked this conversation as resolved.
Outdated
|
||
| private MultiSelectComboBoxElement comboBox; | ||
| private TestBenchElement eventValue; | ||
| private TestBenchElement eventOrigin; | ||
|
|
||
| @Before | ||
| public void init() { | ||
| open(); | ||
| comboBox = $(MultiSelectComboBoxElement.class).waitForFirst(); | ||
|
Check warning on line 45 in vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/MultiSelectComboBoxUserValueChangeIT.java
|
||
| eventValue = $("span").id("event-value"); | ||
| eventOrigin = $("span").id("event-origin"); | ||
| } | ||
|
|
||
| @Test | ||
| public void selectItem_valuePropagatedToServer() { | ||
| clickItem("Item 1"); | ||
|
|
||
| waitUntil(driver -> "Item 1".equals(eventValue.getText())); | ||
| Assert.assertEquals("client", eventOrigin.getText()); | ||
| } | ||
|
|
||
| @Test | ||
| public void deselectItem_valuePropagatedToServer() { | ||
| clickItem("Item 1"); | ||
| waitUntil(driver -> "Item 1".equals(eventValue.getText())); | ||
|
|
||
| clickItem("Item 1"); | ||
| waitUntil(driver -> eventValue.getText().isEmpty()); | ||
| Assert.assertEquals("client", eventOrigin.getText()); | ||
| } | ||
|
|
||
| @Test | ||
| public void clickClearButton_valuePropagatedToServer() { | ||
| clickItem("Item 1"); | ||
| waitUntil(driver -> "Item 1".equals(eventValue.getText())); | ||
|
|
||
| comboBox.$("[part~='clear-button']").first().click(); | ||
|
Check warning on line 73 in vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/MultiSelectComboBoxUserValueChangeIT.java
|
||
|
|
||
| waitUntil(driver -> eventValue.getText().isEmpty()); | ||
| Assert.assertEquals("client", eventOrigin.getText()); | ||
| } | ||
|
|
||
| @Test | ||
| public void removeChip_valuePropagatedToServer() { | ||
| clickItem("Item 1"); | ||
| waitUntil(driver -> "Item 1".equals(eventValue.getText())); | ||
|
|
||
| comboBox.$("vaadin-multi-select-combo-box-chip").get(1) | ||
| .$("[part~='remove-button']").first().click(); | ||
|
Check warning on line 85 in vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/MultiSelectComboBoxUserValueChangeIT.java
|
||
|
|
||
| waitUntil(driver -> eventValue.getText().isEmpty()); | ||
| Assert.assertEquals("client", eventOrigin.getText()); | ||
| } | ||
|
|
||
| @Test | ||
| public void pressEscapeToClear_valuePropagatedToServer() { | ||
| clickItem("Item 1"); | ||
| waitUntil(driver -> "Item 1".equals(eventValue.getText())); | ||
|
|
||
| comboBox.sendKeys(Keys.ESCAPE); | ||
|
|
||
| waitUntil(driver -> eventValue.getText().isEmpty()); | ||
| Assert.assertEquals("client", eventOrigin.getText()); | ||
| } | ||
|
|
||
| private void clickItem(String label) { | ||
| comboBox.openPopup(); | ||
| comboBox.waitForLoadingFinished(); | ||
| List<TestBenchElement> items = comboBox | ||
| .$("vaadin-multi-select-combo-box-item").all(); | ||
| TestBenchElement item = items.stream() | ||
| .filter(el -> label.equals(el.getText())).findFirst() | ||
| .orElseThrow(() -> new AssertionError( | ||
| "Item not found in dropdown: " + label)); | ||
| item.click(); | ||
| comboBox.closePopup(); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.