From 13e3d235e5ce62ad03a2da0046162e10bd5bd94c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Tue, 13 Sep 2016 12:31:08 +0300 Subject: [PATCH] Add multi selection support to CheckBoxGroup This patch adds multi selection support only for CheckBoxGroup without even trying to generalize anything. Adopting the concepts to work with other components will be done separately. Change-Id: Id4ccd2c743b74cb022dc9dfd8cd8dae3bf8f0c54 --- .../com/vaadin/client/ui/VCheckBoxGroup.java | 69 +++--- .../optiongroup/CheckBoxGroupConnector.java | 21 +- .../event/selection/MultiSelectionEvent.java | 77 +++++++ .../selection/MultiSelectionListener.java | 36 ++++ .../java/com/vaadin/ui/CheckBoxGroup.java | 197 +++++++++++++----- .../java/com/vaadin/ui/CheckBoxGroupTest.java | 54 +++++ .../optiongroup/CheckBoxGroupConstants.java | 2 + .../customelements/CheckBoxGroupElement.java | 34 +-- .../checkbox/CheckBoxGroupTestUI.java | 51 ++++- .../checkboxgroup/CheckBoxGroupTest.java | 53 ++++- 10 files changed, 471 insertions(+), 123 deletions(-) create mode 100644 server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java create mode 100644 server/src/main/java/com/vaadin/event/selection/MultiSelectionListener.java create mode 100644 server/src/test/java/com/vaadin/ui/CheckBoxGroupTest.java diff --git a/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java b/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java index 3b6c73950c..3fde2f6998 100644 --- a/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java +++ b/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java @@ -16,9 +16,16 @@ package com.vaadin.client.ui; +import static com.vaadin.shared.ui.optiongroup.CheckBoxGroupConstants.JSONKEY_ITEM_DISABLED; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.function.BiConsumer; + import com.google.gwt.aria.client.Roles; -import com.google.gwt.event.dom.client.ChangeEvent; -import com.google.gwt.event.dom.client.ChangeHandler; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.user.client.ui.Composite; @@ -32,16 +39,8 @@ import com.vaadin.client.ApplicationConnection; import com.vaadin.client.WidgetUtil; import com.vaadin.shared.Registration; import com.vaadin.shared.ui.optiongroup.CheckBoxGroupConstants; -import elemental.json.JsonObject; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.function.Consumer; - -import static com.vaadin.shared.ui.optiongroup.CheckBoxGroupConstants.JSONKEY_ITEM_DISABLED; +import elemental.json.JsonObject; /** * The client-side widget for the {@code CheckBoxGroup} component. @@ -49,8 +48,7 @@ import static com.vaadin.shared.ui.optiongroup.CheckBoxGroupConstants.JSONKEY_IT * @author Vaadin Ltd. * @since 8.0 */ -public class VCheckBoxGroup extends Composite - implements Field, ClickHandler, ChangeHandler, +public class VCheckBoxGroup extends Composite implements Field, ClickHandler, com.vaadin.client.Focusable, HasEnabled { public static final String CLASSNAME = "v-select-optiongroup"; @@ -62,10 +60,7 @@ public class VCheckBoxGroup extends Composite * For internal use only. May be removed or replaced in the future. */ public ApplicationConnection client; - /** - * For internal use only. May be removed or replaced in the future. - */ - public JsonObject selected; //TODO replace with SelectionModel + /** * Widget holding the different options (e.g. ListBox or Panel for radio * buttons) (optional, fallbacks to container Panel) @@ -78,7 +73,7 @@ public class VCheckBoxGroup extends Composite private boolean enabled; private boolean readonly; - private List> selectionChangeListeners; + private List> selectionChangeListeners; public VCheckBoxGroup() { optionsContainer = new FlowPanel(); @@ -102,15 +97,15 @@ public class VCheckBoxGroup extends Composite Roles.getRadiogroupRole().set(getElement()); optionsContainer.clear(); for (JsonObject item : items) { - String itemHtml = - item.getString(CheckBoxGroupConstants.JSONKEY_ITEM_VALUE); + String itemHtml = item + .getString(CheckBoxGroupConstants.JSONKEY_ITEM_VALUE); if (!isHtmlContentAllowed()) { itemHtml = WidgetUtil.escapeHTML(itemHtml); } VCheckBox checkBox = new VCheckBox(); - String iconUrl = - item.getString(CheckBoxGroupConstants.JSONKEY_ITEM_ICON); + String iconUrl = item + .getString(CheckBoxGroupConstants.JSONKEY_ITEM_ICON); if (iconUrl != null && iconUrl.length() != 0) { checkBox.icon = client.getIcon(iconUrl); } @@ -118,7 +113,8 @@ public class VCheckBoxGroup extends Composite checkBox.addStyleName(CLASSNAME_OPTION); checkBox.addClickHandler(this); checkBox.setHTML(itemHtml); - checkBox.setValue(true);//TODO selection model here + checkBox.setValue(item + .getBoolean(CheckBoxGroupConstants.JSONKEY_ITEM_SELECTED)); boolean optionEnabled = !item.getBoolean(JSONKEY_ITEM_DISABLED); boolean enabled = optionEnabled && !isReadonly() && isEnabled(); checkBox.setEnabled(enabled); @@ -138,13 +134,13 @@ public class VCheckBoxGroup extends Composite return; } - final boolean selected = source.getValue(); - JsonObject item = optionsToItems.get(source); //TODO SelectionModel - if (selected) { - this.selected = item; - } else { - this.selected = null; - } + Boolean selected = source.getValue(); + + JsonObject item = optionsToItems.get(source); + assert item != null; + + new ArrayList<>(selectionChangeListeners) + .forEach(listener -> listener.accept(item, selected)); } } @@ -163,8 +159,8 @@ public class VCheckBoxGroup extends Composite .entrySet()) { VCheckBox checkBox = entry.getKey(); JsonObject value = entry.getValue(); - Boolean isOptionEnabled = !value.getBoolean( - CheckBoxGroupConstants.JSONKEY_ITEM_DISABLED); + Boolean isOptionEnabled = !value + .getBoolean(CheckBoxGroupConstants.JSONKEY_ITEM_DISABLED); checkBox.setEnabled(optionGroupEnabled && isOptionEnabled); } } @@ -194,11 +190,6 @@ public class VCheckBoxGroup extends Composite return readonly; } - @Override - public void onChange(ChangeEvent event) { - //TODO selectionModel - } - public void setReadonly(boolean readonly) { if (this.readonly != readonly) { this.readonly = readonly; @@ -214,8 +205,8 @@ public class VCheckBoxGroup extends Composite } } - public Registration addNotifyHandler( - Consumer selectionChanged) { + public Registration addSelectionChangeHandler( + BiConsumer selectionChanged) { selectionChangeListeners.add(selectionChanged); return (Registration) () -> selectionChangeListeners .remove(selectionChanged); diff --git a/client/src/main/java/com/vaadin/client/ui/optiongroup/CheckBoxGroupConnector.java b/client/src/main/java/com/vaadin/client/ui/optiongroup/CheckBoxGroupConnector.java index dbd531382d..a60a1b111f 100644 --- a/client/src/main/java/com/vaadin/client/ui/optiongroup/CheckBoxGroupConnector.java +++ b/client/src/main/java/com/vaadin/client/ui/optiongroup/CheckBoxGroupConnector.java @@ -23,8 +23,8 @@ import com.vaadin.client.communication.StateChangeEvent; import com.vaadin.client.connectors.AbstractListingConnector; import com.vaadin.client.data.DataSource; import com.vaadin.client.ui.VCheckBoxGroup; -import com.vaadin.shared.Registration; import com.vaadin.shared.data.selection.SelectionModel; +import com.vaadin.shared.data.selection.SelectionServerRpc; import com.vaadin.shared.ui.Connect; import com.vaadin.shared.ui.optiongroup.CheckBoxGroupState; import com.vaadin.ui.CheckBoxGroup; @@ -32,20 +32,25 @@ import com.vaadin.ui.CheckBoxGroup; import elemental.json.JsonObject; @Connect(CheckBoxGroup.class) +// We don't care about the framework-provided selection model at this point public class CheckBoxGroupConnector - extends AbstractListingConnector> { - - private Registration selectionChangeRegistration; + extends AbstractListingConnector> { @Override protected void init() { super.init(); - selectionChangeRegistration = - getWidget().addNotifyHandler(this::selectionChanged); + getWidget().addSelectionChangeHandler(this::selectionChanged); } - private void selectionChanged(JsonObject newSelection) { - getSelectionModel().select(newSelection); + private void selectionChanged(JsonObject changedItem, Boolean selected) { + SelectionServerRpc rpc = getRpcProxy(SelectionServerRpc.class); + String key = getRowKey(changedItem); + + if (Boolean.TRUE.equals(selected)) { + rpc.select(key); + } else { + rpc.deselect(key); + } } @Override diff --git a/server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java b/server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java new file mode 100644 index 0000000000..81c8478721 --- /dev/null +++ b/server/src/main/java/com/vaadin/event/selection/MultiSelectionEvent.java @@ -0,0 +1,77 @@ +/* + * Copyright 2000-2016 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.event.selection; + +import java.util.Collections; +import java.util.Set; + +import com.vaadin.event.ConnectorEvent; +import com.vaadin.shared.data.selection.SelectionModel; +import com.vaadin.ui.AbstractListing; + +/** + * Event fired when the the selection changes in a + * {@link com.vaadin.shared.data.selection.SelectionModel.Multi}. + * + * @author Vaadin Ltd + * + * @since 8.0 + * + * @param + * the data type of the selection model + */ +public class MultiSelectionEvent extends ConnectorEvent { + + private Set oldSelection; + private Set newSelection; + + /** + * Creates a new event. + * + * @param source + * the listing component in which the selection changed + * @param oldSelection + * the old set of selected items + * @param newSelection + * the new set of selected items + */ + public MultiSelectionEvent( + AbstractListing> source, + Set oldSelection, Set newSelection) { + super(source); + this.oldSelection = oldSelection; + this.newSelection = newSelection; + } + + /** + * Gets the new selection. + * + * @return a set of items selected after the selection was changed + */ + public Set getNewSelection() { + return Collections.unmodifiableSet(newSelection); + } + + /** + * Gets the old selection. + * + * @return a set of items selected before the selection was changed + */ + public Set getOldSelection() { + return Collections.unmodifiableSet(oldSelection); + } + +} diff --git a/server/src/main/java/com/vaadin/event/selection/MultiSelectionListener.java b/server/src/main/java/com/vaadin/event/selection/MultiSelectionListener.java new file mode 100644 index 0000000000..44c317532d --- /dev/null +++ b/server/src/main/java/com/vaadin/event/selection/MultiSelectionListener.java @@ -0,0 +1,36 @@ +/* + * Copyright 2000-2016 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.event.selection; + +import com.vaadin.event.EventListener; + +/** + * Listens to changes from a + * {@link com.vaadin.shared.data.selection.SelectionModel.Multi}. + * + * @author Vaadin Ltd + * + * @since 8.0 + * + * @param + * the data type of the selection model + */ +public interface MultiSelectionListener + extends EventListener> { + @Override + // Explicitly defined to make reflection logic happy + void accept(MultiSelectionEvent event); +} diff --git a/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java b/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java index 2ad72c5e2c..8687aa2b92 100644 --- a/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java +++ b/server/src/main/java/com/vaadin/ui/CheckBoxGroup.java @@ -16,33 +16,103 @@ package com.vaadin.ui; +import java.lang.reflect.Method; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Predicate; + import com.vaadin.data.Listing; +import com.vaadin.event.selection.MultiSelectionEvent; +import com.vaadin.event.selection.MultiSelectionListener; import com.vaadin.server.Resource; import com.vaadin.server.ResourceReference; import com.vaadin.server.data.DataGenerator; import com.vaadin.server.data.DataSource; +import com.vaadin.shared.Registration; import com.vaadin.shared.data.selection.SelectionModel; +import com.vaadin.shared.data.selection.SelectionServerRpc; import com.vaadin.shared.ui.optiongroup.CheckBoxGroupConstants; import com.vaadin.shared.ui.optiongroup.CheckBoxGroupState; -import elemental.json.JsonObject; +import com.vaadin.util.ReflectTools; -import java.util.Collection; -import java.util.Collections; -import java.util.Objects; -import java.util.Set; -import java.util.function.Function; -import java.util.function.Predicate; +import elemental.json.JsonObject; /** - * A group of Checkboxes. Individual checkboxes are made from items given to - * supplied by {@code Datasource}. Checkboxes my have captions and icons. + * A group of Checkboxes. Individual checkboxes are made from items supplied by + * a {@link DataSource}. Checkboxes may have captions and icons. * * @param - * item type + * item type * @author Vaadin Ltd * @since 8.0 */ -public class CheckBoxGroup extends AbstractListing> { +public class CheckBoxGroup + extends AbstractListing> { + + private final class SimpleMultiSelectModel + implements SelectionModel.Multi { + + private Set selection = new LinkedHashSet<>(); + + @Override + public void select(T item) { + if (selection.contains(item)) { + return; + } + + updateSelection(set -> set.add(item)); + } + + @Override + public Set getSelectedItems() { + return Collections.unmodifiableSet(selection); + } + + @Override + public void deselect(T item) { + if (!selection.contains(item)) { + return; + } + + updateSelection(set -> set.remove(item)); + } + + @Override + public void deselectAll() { + if (selection.isEmpty()) { + return; + } + + updateSelection(Set::clear); + } + + private void updateSelection(Consumer> handler) { + LinkedHashSet oldSelection = new LinkedHashSet<>(selection); + handler.accept(selection); + LinkedHashSet newSelection = new LinkedHashSet<>(selection); + + fireEvent(new MultiSelectionEvent<>(CheckBoxGroup.this, + oldSelection, newSelection)); + + getDataCommunicator().reset(); + } + + @Override + public boolean isSelected(T item) { + return selection.contains(item); + } + } + + @Deprecated + private static final Method SELECTION_CHANGE_METHOD = ReflectTools + .findMethod(MultiSelectionListener.class, "accept", + MultiSelectionEvent.class); private Function itemIconProvider = item -> null; @@ -54,7 +124,7 @@ public class CheckBoxGroup extends AbstractListing> { * Constructs a new CheckBoxGroup with caption. * * @param caption - * caption text + * caption text * @see Listing#setDataSource(DataSource) */ public CheckBoxGroup(String caption) { @@ -66,9 +136,9 @@ public class CheckBoxGroup extends AbstractListing> { * Constructs a new CheckBoxGroup with caption and DataSource. * * @param caption - * the caption text + * the caption text * @param dataSource - * the data source, not null + * the data source, not null * @see Listing#setDataSource(DataSource) */ public CheckBoxGroup(String caption, DataSource dataSource) { @@ -81,9 +151,9 @@ public class CheckBoxGroup extends AbstractListing> { * given items. * * @param caption - * the caption text + * the caption text * @param items - * the data items to use, not null + * the data items to use, not null * @see Listing#setDataSource(DataSource) */ public CheckBoxGroup(String caption, Collection items) { @@ -96,51 +166,52 @@ public class CheckBoxGroup extends AbstractListing> { * @see Listing#setDataSource(DataSource) */ public CheckBoxGroup() { - //TODO Fix when MultiSelection is ready - // SingleSelection model = new SingleSelection<>(this); - // setSelectionModel(model); - // model.addSelectionListener(event -> beforeClientResponse(false)); - setSelectionModel(new SelectionModel.Multi() { - @Override - public void select(T item) { + setSelectionModel(new SimpleMultiSelectModel()); - } + registerRpc(new SelectionServerRpc() { @Override - public Set getSelectedItems() { - return Collections.emptySet(); + public void select(String key) { + getItemForSelectionChange(key) + .ifPresent(getSelectionModel()::select); } @Override - public void deselect(T item) { - + public void deselect(String key) { + getItemForSelectionChange(key) + .ifPresent(getSelectionModel()::deselect); } - @Override - public void deselectAll() { - - } + private Optional getItemForSelectionChange(String key) { + T item = getDataCommunicator().getKeyMapper().get(key); + if (item == null || !itemEnabledProvider.test(item)) { + return Optional.empty(); + } - @Override - public boolean isSelected(T item) { - return false; + return Optional.of(item); } }); + addDataGenerator(new DataGenerator() { @Override public void generateData(T data, JsonObject jsonObject) { jsonObject.put(CheckBoxGroupConstants.JSONKEY_ITEM_VALUE, - itemCaptionProvider.apply(data)); + itemCaptionProvider.apply(data)); Resource icon = itemIconProvider.apply(data); if (icon != null) { String iconUrl = ResourceReference .create(icon, CheckBoxGroup.this, null).getURL(); jsonObject.put(CheckBoxGroupConstants.JSONKEY_ITEM_ICON, - iconUrl); + iconUrl); } if (!itemEnabledProvider.test(data)) { jsonObject.put(CheckBoxGroupConstants.JSONKEY_ITEM_DISABLED, - true); + true); + } + + if (getSelectionModel().isSelected(data)) { + jsonObject.put(CheckBoxGroupConstants.JSONKEY_ITEM_SELECTED, + true); } } @@ -158,8 +229,8 @@ public class CheckBoxGroup extends AbstractListing> { * content is passed to the browser as plain text. * * @param htmlContentAllowed - * true if the captions are used as html, false if used as plain - * text + * true if the captions are used as html, false if used as plain + * text */ public void setHtmlContentAllowed(boolean htmlContentAllowed) { getState().htmlContentAllowed = htmlContentAllowed; @@ -169,7 +240,7 @@ public class CheckBoxGroup extends AbstractListing> { * Checks whether captions are interpreted as html or plain text. * * @return true if the captions are used as html, false if used as plain - * text + * text * @see #setHtmlContentAllowed(boolean) */ public boolean isHtmlContentAllowed() { @@ -197,13 +268,13 @@ public class CheckBoxGroup extends AbstractListing> { } /** - * Sets the item icon provider for this checkbox group. The icon provider - * is queried for each item to optionally display an icon next to - * the item caption. If the provider returns null for an item, no icon is - * displayed. The default provider always returns null (no icons). + * Sets the item icon provider for this checkbox group. The icon provider is + * queried for each item to optionally display an icon next to the item + * caption. If the provider returns null for an item, no icon is displayed. + * The default provider always returns null (no icons). * * @param itemIconProvider - * icons provider, not null + * icons provider, not null */ public void setItemIconProvider(Function itemIconProvider) { Objects.nonNull(itemIconProvider); @@ -222,12 +293,12 @@ public class CheckBoxGroup extends AbstractListing> { /** * Sets the item caption provider for this checkbox group. The caption - * provider is queried for each item to optionally display an item - * textual representation. The default provider returns + * provider is queried for each item to optionally display an item textual + * representation. The default provider returns * {@code String.valueOf(item)}. * * @param itemCaptionProvider - * the item caption provider, not null + * the item caption provider, not null */ public void setItemCaptionProvider( Function itemCaptionProvider) { @@ -246,17 +317,33 @@ public class CheckBoxGroup extends AbstractListing> { } /** - * Sets the item enabled predicate for this checkbox group. The predicate - * is applied to each item to determine whether the item should be - * enabled (true) or disabled (false). Disabled items are displayed as - * grayed out and the user cannot select them. The default predicate - * always returns true (all the items are enabled). + * Sets the item enabled predicate for this checkbox group. The predicate is + * applied to each item to determine whether the item should be enabled + * (true) or disabled (false). Disabled items are displayed as grayed out + * and the user cannot select them. The default predicate always returns + * true (all the items are enabled). * * @param itemEnabledProvider - * the item enable predicate, not null + * the item enable predicate, not null */ public void setItemEnabledProvider(Predicate itemEnabledProvider) { Objects.nonNull(itemEnabledProvider); this.itemEnabledProvider = itemEnabledProvider; } + + /** + * Adds a selection listener that will be called when the selection is + * changed either by the user or programmatically. + * + * @param listener + * the value change listener, not null + * @return a registration for the listener + */ + public Registration addSelectionListener( + MultiSelectionListener listener) { + Objects.requireNonNull(listener, "listener cannot be null"); + addListener(MultiSelectionEvent.class, listener, + SELECTION_CHANGE_METHOD); + return () -> removeListener(MultiSelectionEvent.class, listener); + } } diff --git a/server/src/test/java/com/vaadin/ui/CheckBoxGroupTest.java b/server/src/test/java/com/vaadin/ui/CheckBoxGroupTest.java new file mode 100644 index 0000000000..0ad1801e8b --- /dev/null +++ b/server/src/test/java/com/vaadin/ui/CheckBoxGroupTest.java @@ -0,0 +1,54 @@ +/* + * Copyright 2000-2016 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.ui; + +import java.util.ArrayList; +import java.util.Arrays; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.server.data.DataSource; +import com.vaadin.shared.data.selection.SelectionModel.Multi; + +public class CheckBoxGroupTest { + @Test + public void stableSelectionOrder() { + CheckBoxGroup checkBoxGroup = new CheckBoxGroup<>(); + // Intentional deviation from upcoming selection order + checkBoxGroup + .setDataSource(DataSource.create("Third", "Second", "First")); + Multi selectionModel = checkBoxGroup.getSelectionModel(); + + selectionModel.select("First"); + selectionModel.select("Second"); + selectionModel.select("Third"); + + assertSelectionOrder(selectionModel, "First", "Second", "Third"); + + selectionModel.deselect("First"); + assertSelectionOrder(selectionModel, "Second", "Third"); + + selectionModel.select("First"); + assertSelectionOrder(selectionModel, "Second", "Third", "First"); + } + + private static void assertSelectionOrder(Multi selectionModel, + String... selectionOrder) { + Assert.assertEquals(Arrays.asList(selectionOrder), + new ArrayList<>(selectionModel.getSelectedItems())); + } +} diff --git a/shared/src/main/java/com/vaadin/shared/ui/optiongroup/CheckBoxGroupConstants.java b/shared/src/main/java/com/vaadin/shared/ui/optiongroup/CheckBoxGroupConstants.java index 2747dd4c99..6bca43852a 100644 --- a/shared/src/main/java/com/vaadin/shared/ui/optiongroup/CheckBoxGroupConstants.java +++ b/shared/src/main/java/com/vaadin/shared/ui/optiongroup/CheckBoxGroupConstants.java @@ -25,4 +25,6 @@ public class CheckBoxGroupConstants implements Serializable { public static final String JSONKEY_ITEM_VALUE = "v"; public static final String JSONKEY_ITEM_KEY = "k"; + + public static final String JSONKEY_ITEM_SELECTED = "s"; } diff --git a/uitest-common/src/main/java/com/vaadin/testbench/customelements/CheckBoxGroupElement.java b/uitest-common/src/main/java/com/vaadin/testbench/customelements/CheckBoxGroupElement.java index b070ece852..b2dd93a7b0 100644 --- a/uitest-common/src/main/java/com/vaadin/testbench/customelements/CheckBoxGroupElement.java +++ b/uitest-common/src/main/java/com/vaadin/testbench/customelements/CheckBoxGroupElement.java @@ -1,12 +1,12 @@ /* * Copyright 2000-2014 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 @@ -32,13 +32,13 @@ import com.vaadin.testbench.elementsbase.ServerClass; @ServerClass("com.vaadin.ui.CheckBoxGroup") public class CheckBoxGroupElement extends AbstractSelectElement { - private static org.openqa.selenium.By byButtonSpan = - By.className("v-select-option"); + private static org.openqa.selenium.By byButtonSpan = By + .className("v-select-option"); private static org.openqa.selenium.By byLabel = By.tagName("label"); private static org.openqa.selenium.By byInput = By.tagName("input"); public List getOptions() { - List optionTexts = new ArrayList(); + List optionTexts = new ArrayList<>(); List options = findElements(byButtonSpan); for (WebElement option : options) { optionTexts.add(option.findElement(byLabel).getText()); @@ -51,9 +51,17 @@ public class CheckBoxGroupElement extends AbstractSelectElement { throw new ReadOnlyException(); } List options = findElements(byButtonSpan); - for (WebElement option : options) { + for (int i = 0; i < options.size(); i++) { + WebElement option = options.get(i); if (text.equals(option.findElement(byLabel).getText())) { option.findElement(byInput).click(); + + // Seems like this is needed because of #19753 + waitForVaadin(); + + // Toggling selection causes the DOM to be rebuilt, so fetch new + // items and continue iterating from the same index + options = findElements(byButtonSpan); } } } @@ -70,8 +78,8 @@ public class CheckBoxGroupElement extends AbstractSelectElement { WebElement checkedItem; checkedItem = option.findElement(By.tagName("input")); String checked = checkedItem.getAttribute("checked"); - if (checked != null && - checkedItem.getAttribute("checked").equals("true")) { + if (checked != null + && checkedItem.getAttribute("checked").equals("true")) { values.add(option.findElement(By.tagName("label")).getText()); } } @@ -82,7 +90,8 @@ public class CheckBoxGroupElement extends AbstractSelectElement { * Select option in the checkbox group with the specified value * * @param chars - * value of the option in the checkbox group which will be selected + * value of the option in the checkbox group which will be + * selected */ public void selectOption(CharSequence chars) throws ReadOnlyException { selectByText((String) chars); @@ -91,8 +100,7 @@ public class CheckBoxGroupElement extends AbstractSelectElement { @Override public void clear() { throw new UnsupportedOperationException( - "Clear operation is not supported for CheckBoxGroup." + - " This operation has no effect on the element."); + "Clear operation is not supported for CheckBoxGroup." + + " This operation has no effect on the element."); } } - diff --git a/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java index 343faf3930..fef23f56d3 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java +++ b/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java @@ -1,12 +1,12 @@ /* * Copyright 2000-2014 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 @@ -15,6 +15,9 @@ */ package com.vaadin.tests.components.checkbox; +import java.util.stream.IntStream; + +import com.vaadin.shared.data.selection.SelectionModel.Multi; import com.vaadin.tests.components.abstractlisting.AbstractListingTestUI; import com.vaadin.ui.CheckBoxGroup; @@ -25,8 +28,50 @@ import com.vaadin.ui.CheckBoxGroup; */ public class CheckBoxGroupTestUI extends AbstractListingTestUI> { + + private final String selectionCategory = "Selection"; + + @SuppressWarnings({ "unchecked", "rawtypes" }) @Override protected Class> getTestClass() { return (Class) CheckBoxGroup.class; } + + @Override + protected void createActions() { + super.createActions(); + createListenerMenu(); + createSelectionMenu(); + } + + protected void createSelectionMenu() { + createClickAction( + "Clear selection", selectionCategory, (component, item, + data) -> component.getSelectionModel().deselectAll(), + ""); + + Command, String> toggleSelection = (component, + item, data) -> toggleSelection(item); + + IntStream.of(0, 1, 5, 10, 25).mapToObj(i -> "Item " + i) + .forEach(item -> { + createClickAction("Toggle " + item, selectionCategory, + toggleSelection, item); + }); + } + + private void toggleSelection(String item) { + Multi selectionModel = getComponent().getSelectionModel(); + if (selectionModel.isSelected(item)) { + selectionModel.deselect(item); + } else { + selectionModel.select(item); + } + } + + protected void createListenerMenu() { + createListenerAction("Selection listener", "Listeners", + c -> c.addSelectionListener( + e -> log("Selected: " + e.getNewSelection()))); + } } diff --git a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java index 9a3c2a656c..9a8f631594 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java @@ -1,6 +1,6 @@ /* * Copyright 2000-2014 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 @@ -15,13 +15,17 @@ */ package com.vaadin.tests.components.checkboxgroup; -import com.vaadin.testbench.customelements.CheckBoxGroupElement; -import com.vaadin.tests.components.checkbox.CheckBoxGroupTestUI; -import com.vaadin.tests.tb3.MultiBrowserTest; +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; + +import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import static org.junit.Assert.assertEquals; +import com.vaadin.testbench.customelements.CheckBoxGroupElement; +import com.vaadin.tests.components.checkbox.CheckBoxGroupTestUI; +import com.vaadin.tests.tb3.MultiBrowserTest; /** * Test for CheckBoxGroup @@ -53,6 +57,45 @@ public class CheckBoxGroupTest extends MultiBrowserTest { assertItems(100); } + @Test + public void clickToSelect() { + selectMenuPath("Component", "Listeners", "Selection listener"); + + getSelect().selectByText("Item 4"); + Assert.assertEquals("1. Selected: [Item 4]", getLogRow(0)); + + getSelect().selectByText("Item 2"); + // Selection order (most recently selected is last) + Assert.assertEquals("2. Selected: [Item 4, Item 2]", getLogRow(0)); + + getSelect().selectByText("Item 4"); + Assert.assertEquals("3. Selected: [Item 2]", getLogRow(0)); + } + + @Test + public void selectProgramatically() { + selectMenuPath("Component", "Listeners", "Selection listener"); + + selectMenuPath("Component", "Selection", "Toggle Item 5"); + Assert.assertEquals("2. Selected: [Item 5]", getLogRow(0)); + assertSelected("Item 5"); + + selectMenuPath("Component", "Selection", "Toggle Item 1"); + // Selection order (most recently selected is last) + Assert.assertEquals("4. Selected: [Item 5, Item 1]", getLogRow(0)); + // DOM order + assertSelected("Item 1", "Item 5"); + + selectMenuPath("Component", "Selection", "Toggle Item 5"); + Assert.assertEquals("6. Selected: [Item 1]", getLogRow(0)); + assertSelected("Item 1"); + } + + private void assertSelected(String... expectedSelection) { + Assert.assertEquals(Arrays.asList(expectedSelection), + getSelect().getSelection()); + } + @Override protected Class getUIClass() { return CheckBoxGroupTestUI.class; -- 2.39.5