From 599b61bc8598db35fa111880dd4db57f9da2adda Mon Sep 17 00:00:00 2001 From: Aleksi Hietanen Date: Thu, 29 Sep 2016 09:19:35 +0300 Subject: [PATCH] Fix item caption generators in RadioButton/CheckBoxGroup This patch corrects the behavior of setting caption generators that may return null. Additionally renames RadioButtonGroup's ItemIconProviders to generators, unifying it with other select components. Change-Id: I5e8491834dccce1c37ab9c2002acc475e8945f4b --- .../com/vaadin/ui/AbstractMultiSelect.java | 13 ++- .../java/com/vaadin/ui/RadioButtonGroup.java | 93 +++++++++++-------- .../vaadin/ui/RadioButtonGroupBoVTest.java | 2 +- .../AbstractMultiSelectTestUI.java | 25 ++--- .../radiobutton/RadioButtonGroupTestUI.java | 27 +++--- .../checkboxgroup/CheckBoxGroupTest.java | 13 ++- .../radiobutton/RadioButtonGroupTest.java | 16 +++- .../twincolselect/TwinColSelectTest.java | 15 ++- 8 files changed, 124 insertions(+), 80 deletions(-) diff --git a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java index 3d2cb151b8..4094ee383c 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java @@ -229,8 +229,13 @@ public abstract class AbstractMultiSelect private class MultiSelectDataGenerator implements DataGenerator { @Override public void generateData(T data, JsonObject jsonObject) { - jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, - getItemCaptionGenerator().apply(data)); + String caption = getItemCaptionGenerator().apply(data); + if (caption != null) { + jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, + caption); + } else { + jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, ""); + } Resource icon = getItemIconGenerator().apply(data); if (icon != null) { String iconUrl = ResourceReference @@ -381,8 +386,8 @@ public abstract class AbstractMultiSelect /** * Sets the item enabled predicate for this multiselect. The predicate is - * applied to each item to determine whether the item should be enabled - * ({@code true}) or disabled ({@code false}). Disabled items are displayed + * applied to each item to determine whether the item should be enabled ( + * {@code true}) or disabled ({@code false}). Disabled items are displayed * as grayed out and the user cannot select them. The default predicate * always returns {@code true} (all the items are enabled). *

diff --git a/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java b/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java index f22e4a9535..2e7ff8a706 100644 --- a/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java +++ b/server/src/main/java/com/vaadin/ui/RadioButtonGroup.java @@ -16,6 +16,10 @@ package com.vaadin.ui; +import java.util.Collection; +import java.util.Objects; +import java.util.function.Predicate; + import com.vaadin.data.Listing; import com.vaadin.server.Resource; import com.vaadin.server.ResourceReference; @@ -23,16 +27,12 @@ import com.vaadin.server.data.DataGenerator; import com.vaadin.server.data.DataSource; import com.vaadin.shared.ui.ListingJsonConstants; import com.vaadin.shared.ui.optiongroup.RadioButtonGroupState; -import elemental.json.JsonObject; -import java.util.Collection; -import java.util.Objects; -import java.util.function.Function; -import java.util.function.Predicate; +import elemental.json.JsonObject; /** - * A group of RadioButtons. Individual radiobuttons are made from items supplied by - * a {@link DataSource}. RadioButtons may have captions and icons. + * A group of RadioButtons. Individual radiobuttons are made from items supplied + * by a {@link DataSource}. RadioButtons may have captions and icons. * * @param * item type @@ -41,9 +41,9 @@ import java.util.function.Predicate; */ public class RadioButtonGroup extends AbstractSingleSelect { - private Function itemIconProvider = item -> null; + private IconGenerator itemIconGenerator = item -> null; - private Function itemCaptionProvider = String::valueOf; + private ItemCaptionGenerator itemCaptionGenerator = String::valueOf; private Predicate itemEnabledProvider = item -> true; @@ -98,9 +98,14 @@ public class RadioButtonGroup extends AbstractSingleSelect { addDataGenerator(new DataGenerator() { @Override public void generateData(T data, JsonObject jsonObject) { - jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, - itemCaptionProvider.apply(data)); - Resource icon = itemIconProvider.apply(data); + String caption = getItemCaptionGenerator().apply(data); + if (caption != null) { + jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, + caption); + } else { + jsonObject.put(ListingJsonConstants.JSONKEY_ITEM_VALUE, ""); + } + Resource icon = getItemIconGenerator().apply(data); if (icon != null) { String iconUrl = ResourceReference .create(icon, RadioButtonGroup.this, null).getURL(); @@ -161,52 +166,58 @@ public class RadioButtonGroup extends AbstractSingleSelect { } /** - * Returns the item icons provider. + * Returns the item icon generator. * - * @return the icons provider for items + * @return the currently set icon generator * @see #setItemIconProvider + * @see IconGenerator */ - public Function getItemIconProvider() { - return itemIconProvider; + public IconGenerator getItemIconGenerator() { + return itemIconGenerator; } /** - * Sets the item icon provider for this radiobutton 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 generator for this radiobutton group. The icon + * generator 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 + * @param itemIconGenerator + * the icon generator, not null + * @see IconGenerator */ - public void setItemIconProvider(Function itemIconProvider) { - Objects.requireNonNull(itemIconProvider); - this.itemIconProvider = itemIconProvider; + public void setItemIconGenerator(IconGenerator itemIconGenerator) { + Objects.requireNonNull(itemIconGenerator, + "Item icon generator cannot be null."); + this.itemIconGenerator = itemIconGenerator; } /** - * Returns the item caption provider. + * Returns the currently set item caption generator. * - * @return the captions provider - * @see #setItemCaptionProvider + * @return the currently set caption generator + * @see #setItemCaptionGenerator + * @see ItemCaptionGenerator */ - public Function getItemCaptionProvider() { - return itemCaptionProvider; + public ItemCaptionGenerator getItemCaptionGenerator() { + return itemCaptionGenerator; } /** - * Sets the item caption provider for this radiobutton group. The caption - * provider is queried for each item to optionally display an item textual - * representation. The default provider returns + * Sets the item caption generator for this radiobutton group. The caption + * generator is queried for each item to optionally display an item textual + * representation. The default generator returns * {@code String.valueOf(item)}. * - * @param itemCaptionProvider - * the item caption provider, not null + * @param itemCaptionGenerator + * the item caption generator, not null + * @see ItemCaptionGenerator */ - public void setItemCaptionProvider( - Function itemCaptionProvider) { - Objects.requireNonNull(itemCaptionProvider); - this.itemCaptionProvider = itemCaptionProvider; + public void setItemCaptionGenerator( + ItemCaptionGenerator itemCaptionGenerator) { + Objects.requireNonNull(itemCaptionGenerator, + "Item caption generator cannot be null."); + this.itemCaptionGenerator = itemCaptionGenerator; } /** @@ -220,8 +231,8 @@ public class RadioButtonGroup extends AbstractSingleSelect { } /** - * Sets the item enabled predicate for this radiobutton group. The predicate is - * applied to each item to determine whether the item should be enabled + * Sets the item enabled predicate for this radiobutton 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). diff --git a/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java b/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java index 2ec2404515..2fa64ee0ec 100644 --- a/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java +++ b/server/src/test/java/com/vaadin/ui/RadioButtonGroupBoVTest.java @@ -40,7 +40,7 @@ public class RadioButtonGroupBoVTest public void createOptionGroup() { RadioButtonGroup s = new RadioButtonGroup<>(); s.setItems(EnumSet.allOf(Status.class)); - s.setItemCaptionProvider(Status::getCaption); + s.setItemCaptionGenerator(Status::getCaption); } } diff --git a/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java index fadd94a4be..71fd0fd37d 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java +++ b/uitest/src/main/java/com/vaadin/tests/components/abstractlisting/AbstractMultiSelectTestUI.java @@ -1,11 +1,13 @@ package com.vaadin.tests.components.abstractlisting; +import java.util.LinkedHashMap; import java.util.List; import java.util.stream.Collectors; import java.util.stream.IntStream; import com.vaadin.shared.data.selection.SelectionModel.Multi; import com.vaadin.ui.AbstractMultiSelect; +import com.vaadin.ui.ItemCaptionGenerator; public abstract class AbstractMultiSelectTestUI> extends AbstractListingTestUI { @@ -21,19 +23,18 @@ public abstract class AbstractMultiSelectTestUI> options = new LinkedHashMap<>(); + options.put("Null Caption Generator", item -> null); + options.put("Default Caption Generator", item -> item.toString()); + options.put("Custom Caption Generator", + item -> item.toString() + " Caption"); - private void useItemCaptionProvider(MULTISELECT select, boolean activate, - Object data) { - if (activate) { - select.setItemCaptionGenerator( - item -> item.toString() + " Caption"); - } else { - select.setItemCaptionGenerator(item -> item.toString()); - } - select.getDataSource().refreshAll(); + createSelectAction("Item Caption Generator", "Item Generator", options, + "None", (abstractMultiSelect, captionGenerator, data) -> { + abstractMultiSelect + .setItemCaptionGenerator(captionGenerator); + abstractMultiSelect.getDataSource().refreshAll(); + }, true); } protected void createSelectionMenu() { diff --git a/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java index 19d9ffd3f3..041c578f1b 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java +++ b/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java @@ -15,10 +15,12 @@ */ package com.vaadin.tests.components.radiobutton; +import java.util.LinkedHashMap; import java.util.stream.IntStream; import com.vaadin.shared.data.selection.SelectionModel; import com.vaadin.tests.components.abstractlisting.AbstractListingTestUI; +import com.vaadin.ui.ItemCaptionGenerator; import com.vaadin.ui.RadioButtonGroup; /** @@ -42,7 +44,7 @@ public class RadioButtonGroupTestUI super.createActions(); createListenerMenu(); createSelectionMenu(); - createItemProviderMenu(); + createItemGeneratorMenu(); } protected void createSelectionMenu() { @@ -59,19 +61,18 @@ public class RadioButtonGroupTestUI selectionCategory, toggleSelection, item)); } - private void createItemProviderMenu() { - createBooleanAction("Use Item Caption Provider", "Item Provider", false, - this::useItemCaptionProvider); - } + private void createItemGeneratorMenu() { + LinkedHashMap> options = new LinkedHashMap<>(); + options.put("Null Caption Generator", item -> null); + options.put("Default Caption Generator", item -> item.toString()); + options.put("Custom Caption Generator", + item -> item.toString() + " Caption"); - private void useItemCaptionProvider(RadioButtonGroup group, - boolean activate, Object data) { - if (activate) { - group.setItemCaptionProvider(item -> item.toString() + " Caption"); - } else { - group.setItemCaptionProvider(item -> item.toString()); - } - group.getDataSource().refreshAll(); + createSelectAction("Item Caption Generator", "Item Generator", options, + "None", (radioButtonGroup, captionGenerator, data) -> { + radioButtonGroup.setItemCaptionGenerator(captionGenerator); + radioButtonGroup.getDataSource().refreshAll(); + }, true); } private void toggleSelection(String item) { 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 4bb1defbf6..b4e462a983 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 @@ -123,11 +123,20 @@ public class CheckBoxGroupTest extends MultiBrowserTest { @Test public void itemCaptionGenerator() { - selectMenuPath("Component", "Item Generator", - "Use Item Caption Generator"); + selectMenuPath("Component", "Item Generator", "Item Caption Generator", + "Custom Caption Generator"); assertItems(20, " Caption"); } + @Test + public void nullItemCaptionGenerator() { + selectMenuPath("Component", "Item Generator", "Item Caption Generator", + "Null Caption Generator"); + for (String text : getSelect().getOptions()) { + Assert.assertEquals("", text); + } + } + @Test public void itemIconGenerator() { selectMenuPath("Component", "Item Generator", diff --git a/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java index c909333290..42d912edd6 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTest.java @@ -112,12 +112,21 @@ public class RadioButtonGroupTest extends MultiBrowserTest { } @Test - public void itemCaptionProvider() { - selectMenuPath("Component", "Item Provider", - "Use Item Caption Provider"); + public void itemCaptionGenerator() { + selectMenuPath("Component", "Item Generator", "Item Caption Generator", + "Custom Caption Generator"); assertItems(20, " Caption"); } + @Test + public void nullItemCaptionGenerator() { + selectMenuPath("Component", "Item Generator", "Item Caption Generator", + "Null Caption Generator"); + for (String text : getSelect().getOptions()) { + Assert.assertEquals("", text); + } + } + @Test public void selectProgramatically() { selectMenuPath("Component", "Listeners", "Selection listener"); @@ -162,5 +171,4 @@ public class RadioButtonGroupTest extends MultiBrowserTest { } assertEquals("Number of items", count, i); } - } diff --git a/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java b/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java index e5f44a405b..ad2752112f 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/twincolselect/TwinColSelectTest.java @@ -167,12 +167,21 @@ public class TwinColSelectTest extends MultiBrowserTest { } @Test - public void itemCaptionProvider() { - selectMenuPath("Component", "Item Generator", - "Use Item Caption Generator"); + public void itemCaptionGenerator() { + selectMenuPath("Component", "Item Generator", "Item Caption Generator", + "Custom Caption Generator"); assertItems(20, " Caption"); } + @Test + public void nullItemCaptionGenerator() { + selectMenuPath("Component", "Item Generator", "Item Caption Generator", + "Null Caption Generator"); + for (String text : getTwinColSelect().getOptions()) { + Assert.assertEquals("", text); + } + } + @Test public void selectProgramatically() { selectMenuPath("Component", "Listeners", "Selection listener"); -- 2.39.5