From 923b42e5499375333805cdeb851f0048ae5b31e4 Mon Sep 17 00:00:00 2001 From: Aleksi Hietanen Date: Fri, 13 Jan 2017 08:36:51 +0200 Subject: [PATCH] Include old value in ValueChangeEvent (#8229) * Include old value in ValueChangeEvent --- .../main/java/com/vaadin/data/HasValue.java | 38 +++++++++++++------ .../event/selection/MultiSelectionEvent.java | 14 +++---- .../event/selection/SingleSelectionEvent.java | 12 ++++-- .../java/com/vaadin/ui/AbstractDateField.java | 2 +- .../java/com/vaadin/ui/AbstractField.java | 10 +++-- .../com/vaadin/ui/AbstractMultiSelect.java | 3 +- .../com/vaadin/ui/AbstractSingleSelect.java | 12 ++++-- .../src/main/java/com/vaadin/ui/ComboBox.java | 2 +- .../colorpicker/ColorPickerGrid.java | 4 +- .../colorpicker/ColorPickerHistory.java | 5 ++- .../colorpicker/ColorPickerPopup.java | 6 ++- .../colorpicker/ColorPickerPreview.java | 5 ++- .../grid/SingleSelectionModelImpl.java | 11 ++++-- .../data/GridAsSingleSelectInBinderTest.java | 4 ++ .../grid/GridSingleSelectionModelTest.java | 12 ++++-- .../tests/server/component/grid/GridTest.java | 1 + .../vaadin/ui/AbstractMultiSelectTest.java | 28 ++++++++++++++ .../vaadin/ui/AbstractSingleSelectTest.java | 24 +++++++++++- 18 files changed, 145 insertions(+), 48 deletions(-) diff --git a/server/src/main/java/com/vaadin/data/HasValue.java b/server/src/main/java/com/vaadin/data/HasValue.java index b5707605cd..962a3e9620 100644 --- a/server/src/main/java/com/vaadin/data/HasValue.java +++ b/server/src/main/java/com/vaadin/data/HasValue.java @@ -19,8 +19,9 @@ import java.io.Serializable; import java.lang.reflect.Method; import java.util.EventObject; import java.util.Objects; -import com.vaadin.server.Setter; + import com.vaadin.event.SerializableEventListener; +import com.vaadin.server.Setter; import com.vaadin.shared.Registration; import com.vaadin.ui.Component; import com.vaadin.util.ReflectTools; @@ -50,6 +51,9 @@ public interface HasValue extends Serializable { private final boolean userOriginated; private final Component component; + private final V oldValue; + private final V value; + /** * Creates a new {@code ValueChange} event containing the current value * of the given value-bearing source component. @@ -58,13 +62,15 @@ public interface HasValue extends Serializable { * the type of the source component * @param component * the source component bearing the value, not null + * @param oldValue + * the previous value held by the source of this event * @param userOriginated * {@code true} if this event originates from the client, * {@code false} otherwise. */ public > ValueChangeEvent( - COMPONENT component, boolean userOriginated) { - this(component, component, userOriginated); + COMPONENT component, V oldValue, boolean userOriginated) { + this(component, component, oldValue, userOriginated); } /** @@ -75,31 +81,39 @@ public interface HasValue extends Serializable { * the component, not null * @param hasValue * the HasValue instance bearing the value, not null + * @param oldValue + * the previous value held by the source of this event * @param userOriginated * {@code true} if this event originates from the client, * {@code false} otherwise. */ public ValueChangeEvent(Component component, HasValue hasValue, - boolean userOriginated) { + V oldValue, boolean userOriginated) { super(hasValue); this.userOriginated = userOriginated; this.component = component; + this.oldValue = oldValue; + value = hasValue.getValue(); } /** - * Returns the new value of the event source. - *

- * This a shorthand method for {@link HasValue#getValue()} for the event - * source {@link #getSource()}. Thus the value is always the most recent - * one, even if has been changed after the firing of this event. - * - * @see HasValue#getValue() + * Returns the value of the source before this value change event + * occurred. + * + * @return the value previously held by the source of this event + */ + public V getOldValue() { + return oldValue; + } + + /** + * Returns the new value that triggered this value change event. * * @return the new value */ public V getValue() { - return getSource().getValue(); + return value; } /** diff --git a/server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java b/server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java index 3c89a53f50..e00c60dca6 100644 --- a/server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java +++ b/server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java @@ -39,8 +39,6 @@ import com.vaadin.ui.MultiSelect; public class MultiSelectionEvent extends ValueChangeEvent> implements SelectionEvent { - private final Set oldSelection; - /** * Creates a new event. * @@ -54,8 +52,7 @@ public class MultiSelectionEvent extends ValueChangeEvent> */ public MultiSelectionEvent(AbstractMultiSelect source, Set oldSelection, boolean userOriginated) { - super(source, userOriginated); - this.oldSelection = oldSelection; + super(source, oldSelection, userOriginated); } /** @@ -73,8 +70,7 @@ public class MultiSelectionEvent extends ValueChangeEvent> */ public MultiSelectionEvent(Component component, MultiSelect source, Set oldSelection, boolean userOriginated) { - super(component, source, userOriginated); - this.oldSelection = oldSelection; + super(component, source, oldSelection, userOriginated); } /** @@ -98,7 +94,7 @@ public class MultiSelectionEvent extends ValueChangeEvent> * @return a set of items selected before the selection was changed */ public Set getOldSelection() { - return Collections.unmodifiableSet(oldSelection); + return Collections.unmodifiableSet(getOldValue()); } /** @@ -111,7 +107,7 @@ public class MultiSelectionEvent extends ValueChangeEvent> * @return the items that were removed from selection */ public Set getRemovedSelection() { - LinkedHashSet copy = new LinkedHashSet<>(oldSelection); + LinkedHashSet copy = new LinkedHashSet<>(getOldValue()); copy.removeAll(getNewSelection()); return copy; } @@ -127,7 +123,7 @@ public class MultiSelectionEvent extends ValueChangeEvent> */ public Set getAddedSelection() { LinkedHashSet copy = new LinkedHashSet<>(getValue()); - copy.removeAll(oldSelection); + copy.removeAll(getOldValue()); return copy; } diff --git a/server/src/main/java/com/vaadin/event/selection/SingleSelectionEvent.java b/server/src/main/java/com/vaadin/event/selection/SingleSelectionEvent.java index 89f30e1869..db78e0591b 100644 --- a/server/src/main/java/com/vaadin/event/selection/SingleSelectionEvent.java +++ b/server/src/main/java/com/vaadin/event/selection/SingleSelectionEvent.java @@ -42,13 +42,15 @@ public class SingleSelectionEvent extends ValueChangeEvent * * @param source * the listing that fired the event + * @param oldSelection + * the item that was previously selected * @param userOriginated * {@code true} if this event originates from the client, * {@code false} otherwise. */ - public SingleSelectionEvent(AbstractSingleSelect source, + public SingleSelectionEvent(AbstractSingleSelect source, T oldSelection, boolean userOriginated) { - super(source, userOriginated); + super(source, oldSelection, userOriginated); } /** @@ -58,13 +60,15 @@ public class SingleSelectionEvent extends ValueChangeEvent * the component where the event originated * @param source * the single select source + * @param oldSelection + * the item that was previously selected * @param userOriginated * {@code true} if this event originates from the client, * {@code false} otherwise. */ public SingleSelectionEvent(Component component, SingleSelect source, - boolean userOriginated) { - super(component, source, userOriginated); + T oldSelection, boolean userOriginated) { + super(component, source, oldSelection, userOriginated); } /** diff --git a/server/src/main/java/com/vaadin/ui/AbstractDateField.java b/server/src/main/java/com/vaadin/ui/AbstractDateField.java index 0a6cf8fc51..57e0b0ce5f 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractDateField.java +++ b/server/src/main/java/com/vaadin/ui/AbstractDateField.java @@ -297,7 +297,7 @@ public abstract class AbstractDateField extends AbstractComponent if (!isDifferentValue(value)) { return false; } + T oldValue = this.getValue(); doSetValue(value); if (!userOriginated) { markAsDirty(); } - fireEvent(createValueChange(userOriginated)); + fireEvent(createValueChange(oldValue, userOriginated)); return true; } @@ -178,13 +179,16 @@ public abstract class AbstractField extends AbstractComponent /** * Returns a new value change event instance. * + * @param oldValue + * the value of this field before this value change event * @param userOriginated * {@code true} if this event originates from the client, * {@code false} otherwise. * @return the new event */ - protected ValueChangeEvent createValueChange(boolean userOriginated) { - return new ValueChangeEvent<>(this, userOriginated); + protected ValueChangeEvent createValueChange(T oldValue, + boolean userOriginated) { + return new ValueChangeEvent<>(this, oldValue, userOriginated); } @Override diff --git a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java index ce79da25a1..e104b1c4e6 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java @@ -230,7 +230,8 @@ public abstract class AbstractMultiSelect extends AbstractListing public Registration addValueChangeListener( HasValue.ValueChangeListener> listener) { return addSelectionListener(event -> listener.valueChange( - new ValueChangeEvent<>(this, event.isUserOriginated()))); + new ValueChangeEvent<>(this, event.getOldValue(), + event.isUserOriginated()))); } /** diff --git a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java index f98d15bf10..967b84e6b6 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java @@ -154,7 +154,8 @@ public abstract class AbstractSingleSelect extends AbstractListing public Registration addValueChangeListener( HasValue.ValueChangeListener listener) { return addSelectionListener(event -> listener.valueChange( - new ValueChangeEvent<>(this, event.isUserOriginated()))); + new ValueChangeEvent<>(this, event.getOldValue(), + event.isUserOriginated()))); } @Override @@ -227,6 +228,7 @@ public abstract class AbstractSingleSelect extends AbstractListing return; } + T oldSelection = getSelectedItem().orElse(getEmptyValue()); doSetSelectedKey(key); // Update diffstate so that a change will be sent to the client if the @@ -234,7 +236,8 @@ public abstract class AbstractSingleSelect extends AbstractListing updateDiffstate("selectedItemKey", key == null ? Json.createNull() : Json.create(key)); - fireEvent(new SingleSelectionEvent<>(AbstractSingleSelect.this, true)); + fireEvent(new SingleSelectionEvent<>(AbstractSingleSelect.this, + oldSelection, true)); } /** @@ -253,8 +256,11 @@ public abstract class AbstractSingleSelect extends AbstractListing return; } + T oldSelection = getSelectedItem().orElse(getEmptyValue()); doSetSelectedKey(key); - fireEvent(new SingleSelectionEvent<>(AbstractSingleSelect.this, false)); + + fireEvent(new SingleSelectionEvent<>(AbstractSingleSelect.this, + oldSelection, false)); } /** diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java index f742a22adc..226269d72e 100644 --- a/server/src/main/java/com/vaadin/ui/ComboBox.java +++ b/server/src/main/java/com/vaadin/ui/ComboBox.java @@ -552,7 +552,7 @@ public class ComboBox extends AbstractSingleSelect HasValue.ValueChangeListener listener) { return addSelectionListener(event -> { listener.valueChange(new ValueChangeEvent<>(event.getComponent(), - this, event.isUserOriginated())); + this, event.getOldValue(), event.isUserOriginated())); }); } diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java index 95229a2d6a..6ab1ac1ccb 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java @@ -38,9 +38,11 @@ public class ColorPickerGrid extends AbstractField { @Override public void select(int x, int y) { + Color oldValue = colorGrid[x][y]; ColorPickerGrid.this.x = x; ColorPickerGrid.this.y = y; - fireEvent(new ValueChangeEvent<>(ColorPickerGrid.this, true)); + fireEvent(new ValueChangeEvent<>(ColorPickerGrid.this, oldValue, + true)); } @Override diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java index 101a4b642a..41606737d5 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java @@ -50,8 +50,9 @@ public class ColorPickerHistory extends CustomField { ColorPickerGrid grid = new ColorPickerGrid(ROWS, COLUMNS); grid.setWidth("100%"); grid.setPosition(0, 0); - grid.addValueChangeListener(event -> fireEvent( - new ValueChangeEvent<>(this, event.isUserOriginated()))); + grid.addValueChangeListener( + event -> fireEvent(new ValueChangeEvent<>(this, + event.getOldValue(), event.isUserOriginated()))); return grid; } diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java index 9d88b03fa5..8d945e9e9c 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java @@ -73,6 +73,9 @@ public class ColorPickerPopup extends Window implements HasValue { /** The resize button. */ private final Button resize = new Button("show/hide history"); + /** The previously selected color. */ + private Color previouslySelectedColor = Color.WHITE; + /** The selected color. */ private Color selectedColor = Color.WHITE; @@ -448,7 +451,7 @@ public class ColorPickerPopup extends Window implements HasValue { } private void okButtonClick(ClickEvent event) { - fireEvent(new ValueChangeEvent<>(this, true)); + fireEvent(new ValueChangeEvent<>(this, previouslySelectedColor, true)); close(); } @@ -479,6 +482,7 @@ public class ColorPickerPopup extends Window implements HasValue { public void setValue(Color color) { Objects.requireNonNull(color, "color cannot be null"); + previouslySelectedColor = selectedColor; selectedColor = color; hsvGradient.setValue(selectedColor); diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java index 0c99779658..6953d980f4 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java @@ -122,6 +122,7 @@ public class ColorPickerPreview extends CssLayout implements HasValue { private void valueChange(ValueChangeEvent event) { String value = event.getValue(); + Color oldColor = color; try { if (value != null) { /* @@ -174,8 +175,8 @@ public class ColorPickerPreview extends CssLayout implements HasValue { } oldValue = value; - fireEvent( - new ValueChangeEvent<>(this, event.isUserOriginated())); + fireEvent(new ValueChangeEvent<>(this, oldColor, + event.isUserOriginated())); } } catch (NumberFormatException nfe) { diff --git a/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java b/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java index eeb57bab26..4b3eb2d41c 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/SingleSelectionModelImpl.java @@ -163,9 +163,10 @@ public class SingleSelectionModelImpl extends AbstractSelectionModel return; } + T oldSelection = this.getSelectedItem().orElse(null); doSetSelectedKey(key); - fireEvent( - new SingleSelectionEvent<>(getGrid(), asSingleSelect(), true)); + fireEvent(new SingleSelectionEvent<>(getGrid(), asSingleSelect(), + oldSelection, true)); } /** @@ -184,9 +185,11 @@ public class SingleSelectionModelImpl extends AbstractSelectionModel return; } + T oldSelection = this.getSelectedItem() + .orElse(asSingleSelect().getEmptyValue()); doSetSelectedKey(key); - fireEvent( - new SingleSelectionEvent<>(getGrid(), asSingleSelect(), false)); + fireEvent(new SingleSelectionEvent<>(getGrid(), asSingleSelect(), + oldSelection, false)); } /** diff --git a/server/src/test/java/com/vaadin/data/GridAsSingleSelectInBinderTest.java b/server/src/test/java/com/vaadin/data/GridAsSingleSelectInBinderTest.java index 9886df43fc..077ca185a2 100644 --- a/server/src/test/java/com/vaadin/data/GridAsSingleSelectInBinderTest.java +++ b/server/src/test/java/com/vaadin/data/GridAsSingleSelectInBinderTest.java @@ -129,9 +129,11 @@ public class GridAsSingleSelectInBinderTest select = grid.asSingleSelect(); List selected = new ArrayList<>(); + List oldSelected = new ArrayList<>(); List userOriginated = new ArrayList<>(); select.addValueChangeListener(event -> { selected.add(event.getValue()); + oldSelected.add(event.getOldValue()); userOriginated.add(event.isUserOriginated()); assertSame(grid, event.getComponent()); // cannot compare that the event source is the select since a new @@ -150,6 +152,8 @@ public class GridAsSingleSelectInBinderTest assertEquals(Arrays.asList(Sex.UNKNOWN, Sex.MALE, null, Sex.FEMALE), selected); + assertEquals(Arrays.asList(null, Sex.UNKNOWN, Sex.MALE, null), + oldSelected); assertEquals(Arrays.asList(false, true, true, false), userOriginated); } diff --git a/server/src/test/java/com/vaadin/tests/components/grid/GridSingleSelectionModelTest.java b/server/src/test/java/com/vaadin/tests/components/grid/GridSingleSelectionModelTest.java index cded4f878c..ce4da636d0 100644 --- a/server/src/test/java/com/vaadin/tests/components/grid/GridSingleSelectionModelTest.java +++ b/server/src/test/java/com/vaadin/tests/components/grid/GridSingleSelectionModelTest.java @@ -96,17 +96,22 @@ public class GridSingleSelectionModelTest { customGrid.setItems("Foo", "Bar", "Baz"); List selectionChanges = new ArrayList<>(); + List oldSelectionValues = new ArrayList<>(); ((SingleSelectionModelImpl) customGrid.getSelectionModel()) - .addSingleSelectionListener( - e -> selectionChanges.add(e.getValue())); + .addSingleSelectionListener(e -> { + selectionChanges.add(e.getValue()); + oldSelectionValues.add(e.getOldValue()); + }); customGrid.getSelectionModel().select("Foo"); assertEquals("Foo", customGrid.getSelectionModel().getFirstSelectedItem().get()); assertEquals(Arrays.asList("Foo"), selectionChanges); + assertEquals(Arrays.asList((String) null), oldSelectionValues); customGrid.setSelectionMode(SelectionMode.MULTI); assertEquals(Arrays.asList("Foo", null), selectionChanges); + assertEquals(Arrays.asList(null, "Foo"), oldSelectionValues); } @Test @@ -314,10 +319,11 @@ public class GridSingleSelectionModelTest { Assert.assertSame(registration, actualRegistration); selectionListener.get().selectionChange(new SingleSelectionEvent<>(grid, - select.asSingleSelect(), true)); + select.asSingleSelect(), null, true)); Assert.assertEquals(grid, event.get().getComponent()); Assert.assertEquals(value, event.get().getValue()); + Assert.assertEquals(null, event.get().getOldValue()); Assert.assertTrue(event.get().isUserOriginated()); } diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java index f545581bb3..a692ecc566 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java @@ -148,6 +148,7 @@ public class GridTest { grid.getSelectionModel().deselect("foo"); + event = eventCapture.getValue(); assertNotNull(event); assertFalse(event.isUserOriginated()); assertEquals("bar", event.getFirstSelected().get()); diff --git a/server/src/test/java/com/vaadin/ui/AbstractMultiSelectTest.java b/server/src/test/java/com/vaadin/ui/AbstractMultiSelectTest.java index d98477c66d..4b4ddbcae9 100644 --- a/server/src/test/java/com/vaadin/ui/AbstractMultiSelectTest.java +++ b/server/src/test/java/com/vaadin/ui/AbstractMultiSelectTest.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -60,6 +61,10 @@ public class AbstractMultiSelectTest & Lis private Registration registration; + private List> values; + + private List> oldValues; + @Before public void setUp() { selectToTest.deselectAll(); @@ -68,6 +73,13 @@ public class AbstractMultiSelectTest & Lis DataProvider.create("3", "2", "1", "5", "8", "7", "4", "6")); rpc = ComponentTest.getRpcProxy(selectToTest, MultiSelectServerRpc.class); + + values = new ArrayList<>(); + oldValues = new ArrayList<>(); + selectToTest + .addValueChangeListener(event -> values.add(event.getValue())); + selectToTest.addValueChangeListener( + event -> oldValues.add(event.getOldValue())); } @After @@ -102,6 +114,7 @@ public class AbstractMultiSelectTest & Lis new LinkedHashSet<>(Arrays.asList("5", "2")), new LinkedHashSet<>(Arrays.asList("3", "8"))); assertSelectionOrder("7", "5", "2"); + verifyValueChangeEvents(); } @Test @@ -140,6 +153,7 @@ public class AbstractMultiSelectTest & Lis // deselect completely not selected selectToTest.select("1", "4"); Assert.assertEquals(8, listenerCount.get()); + verifyValueChangeEvents(); } @Test @@ -216,6 +230,7 @@ public class AbstractMultiSelectTest & Lis rpcUpdateSelection(new String[] { "6", "8" }, new String[] { "6" }); Assert.assertEquals(11, listenerCount.get()); assertSelectionOrder("6", "4", "8"); + verifyValueChangeEvents(); } @Test @@ -235,6 +250,7 @@ public class AbstractMultiSelectTest & Lis set.add("3"); selectToTest.select("3"); Assert.assertEquals(set, selectToTest.getValue()); + verifyValueChangeEvents(); } @Test @@ -250,6 +266,7 @@ public class AbstractMultiSelectTest & Lis }; Assert.assertSame(set, select.getValue()); + verifyValueChangeEvents(); } @Test @@ -265,6 +282,7 @@ public class AbstractMultiSelectTest & Lis selectToTest.setValue(set); Assert.assertEquals(set, selectToTest.getSelectedItems()); + verifyValueChangeEvents(); } @Test @@ -351,4 +369,14 @@ public class AbstractMultiSelectTest & Lis Assert.assertEquals(Arrays.asList(selectionOrder), new ArrayList<>(selectToTest.getSelectedItems())); } + + private void verifyValueChangeEvents() { + if (oldValues.size() > 0) { + Assert.assertTrue(oldValues.get(0).isEmpty()); + Assert.assertEquals(values.size(), oldValues.size()); + for (int i = 0; i < oldValues.size() - 1; i++) { + Assert.assertEquals(values.get(i), oldValues.get(i + 1)); + } + } + } } diff --git a/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java b/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java index 9bbcc2ca43..cfcdba1f1c 100644 --- a/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java +++ b/server/src/test/java/com/vaadin/ui/AbstractSingleSelectTest.java @@ -49,6 +49,7 @@ import com.vaadin.ui.declarative.DesignContext; public class AbstractSingleSelectTest { private List selectionChanges; + private List oldSelections; private static class PersonListing extends AbstractSingleSelect implements Listing> { @@ -79,8 +80,11 @@ public class AbstractSingleSelectTest { public void initListing() { listing = new PersonListing(); listing.setItems(PERSON_A, PERSON_B, PERSON_C); + selectionChanges = new ArrayList<>(); + oldSelections = new ArrayList<>(); listing.addSelectionListener(e -> selectionChanges.add(e.getValue())); + listing.addSelectionListener(e -> oldSelections.add(e.getOldValue())); } public static final Person PERSON_C = new Person("c", 3); @@ -106,6 +110,7 @@ public class AbstractSingleSelectTest { assertEquals(Optional.of(PERSON_B), listing.getSelectedItem()); assertEquals(Arrays.asList(PERSON_B), selectionChanges); + verifyValueChanges(); } @Test @@ -123,6 +128,7 @@ public class AbstractSingleSelectTest { assertFalse(listing.getSelectedItem().isPresent()); assertEquals(Arrays.asList(PERSON_B, null), selectionChanges); + verifyValueChanges(); } @Test @@ -140,6 +146,7 @@ public class AbstractSingleSelectTest { assertEquals(Optional.of(PERSON_C), listing.getSelectedItem()); assertEquals(Arrays.asList(PERSON_B, PERSON_C), selectionChanges); + verifyValueChanges(); } @Test @@ -157,6 +164,7 @@ public class AbstractSingleSelectTest { assertEquals(Optional.of(PERSON_C), listing.getSelectedItem()); assertEquals(Arrays.asList(PERSON_C), selectionChanges); + verifyValueChanges(); } @Test @@ -175,6 +183,7 @@ public class AbstractSingleSelectTest { assertFalse(listing.getSelectedItem().isPresent()); assertEquals(Arrays.asList(PERSON_C, null), selectionChanges); + verifyValueChanges(); } @Test @@ -185,6 +194,7 @@ public class AbstractSingleSelectTest { listing.setValue(null); Assert.assertNull(listing.getValue()); + verifyValueChanges(); } @Test @@ -211,6 +221,7 @@ public class AbstractSingleSelectTest { listing.setValue(null); Assert.assertFalse(listing.getSelectedItem().isPresent()); + verifyValueChanges(); } @Test @@ -266,11 +277,22 @@ public class AbstractSingleSelectTest { Assert.assertSame(registration, actualRegistration); selectionListener.get() - .selectionChange(new SingleSelectionEvent<>(select, true)); + .selectionChange( + new SingleSelectionEvent<>(select, value, true)); Assert.assertEquals(select, event.get().getComponent()); + Assert.assertEquals(value, event.get().getOldValue()); Assert.assertEquals(value, event.get().getValue()); Assert.assertTrue(event.get().isUserOriginated()); } + private void verifyValueChanges() { + if (oldSelections.size() > 0) { + assertEquals(null, oldSelections.get(0)); + assertEquals(selectionChanges.size(), oldSelections.size()); + for (int i = 0; i < oldSelections.size() - 1; i++) { + assertEquals(selectionChanges.get(i), oldSelections.get(i + 1)); + } + } + } } -- 2.39.5