diff options
9 files changed, 778 insertions, 109 deletions
diff --git a/client/src/main/java/com/vaadin/client/ui/VComboBox.java b/client/src/main/java/com/vaadin/client/ui/VComboBox.java index 6ecea2d116..77e1108417 100644 --- a/client/src/main/java/com/vaadin/client/ui/VComboBox.java +++ b/client/src/main/java/com/vaadin/client/ui/VComboBox.java @@ -208,12 +208,12 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, return false; } ComboBoxSuggestion other = (ComboBoxSuggestion) obj; - if ((key == null && other.key != null) - || (key != null && !key.equals(other.key))) { + if (key == null && other.key != null + || key != null && !key.equals(other.key)) { return false; } - if ((caption == null && other.caption != null) - || (caption != null && !caption.equals(other.caption))) { + if (caption == null && other.caption != null + || caption != null && !caption.equals(other.caption)) { return false; } if (!SharedUtil.equals(untranslatedIconUri, @@ -241,12 +241,12 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, return $entry(function(e) { var deltaX = e.deltaX ? e.deltaX : -0.5*e.wheelDeltaX; var deltaY = e.deltaY ? e.deltaY : -0.5*e.wheelDeltaY; - + // IE8 has only delta y if (isNaN(deltaY)) { deltaY = -0.5*e.wheelDelta; } - + @com.vaadin.client.ui.VComboBox.JsniUtil::moveScrollFromEvent(*)(widget, deltaX, deltaY, e, e.deltaMode); }); }-*/; @@ -398,14 +398,11 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, * * @param currentPage * The current page number - * @param totalSuggestions - * The total amount of suggestions */ - public void showSuggestions(final int currentPage, - final int totalSuggestions) { + public void showSuggestions(final int currentPage) { debug("VComboBox.SP: showSuggestions(" + currentPage + ", " - + totalSuggestions + ")"); + + getTotalSuggestions() + ")"); final SuggestionPopup popup = this; // Add TT anchor point @@ -418,14 +415,13 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, setPopupPosition(x, topPosition); - int nullOffset = (nullSelectionAllowed && "".equals(lastFilter) ? 1 - : 0); - boolean firstPage = (currentPage == 0); + int nullOffset = getNullSelectionItemShouldBeVisible() ? 1 : 0; + boolean firstPage = currentPage == 0; final int first = currentPage * pageLength + 1 - (firstPage ? 0 : nullOffset); final int last = first + currentSuggestions.size() - 1 - (firstPage && "".equals(lastFilter) ? nullOffset : 0); - final int matches = totalSuggestions - nullOffset; + final int matches = getTotalSuggestions(); if (last > 0) { // nullsel not counted, as requested by user status.setInnerText((matches == 0 ? 0 : first) + "-" + last @@ -435,7 +431,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, } // We don't need to show arrows or statusbar if there is // only one page - if (totalSuggestions <= pageLength || pageLength == 0) { + if (getTotalSuggestionsIncludingNullSelectionItem() <= pageLength + || pageLength == 0) { setPagingEnabled(false); } else { setPagingEnabled(true); @@ -609,8 +606,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, public void scrollDown() { debug("VComboBox.SP.LPS: scrollDown()"); if (pageLength > 0 - && totalMatches > (currentPage + pagesToScroll + 1) - * pageLength) { + && getTotalSuggestionsIncludingNullSelectionItem() > (currentPage + + pagesToScroll + 1) * pageLength) { pagesToScroll++; cancel(); schedule(200); @@ -860,7 +857,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, if (suggestionPopupWidth == null) { if (naturalMenuWidth < desiredWidth) { naturalMenuWidth = desiredWidth - popupOuterPadding; - width = (desiredWidth - popupOuterPadding) + "px"; + width = desiredWidth - popupOuterPadding + "px"; } } else if (isrelativeUnits(suggestionPopupWidth)) { float mainComponentWidth = desiredWidth - popupOuterPadding; @@ -904,8 +901,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, */ public boolean isJustClosed() { debug("VComboBox.SP: justClosed()"); - final long now = (new Date()).getTime(); - return (lastAutoClosed > 0 && (now - lastAutoClosed) < 200); + final long now = new Date().getTime(); + return lastAutoClosed > 0 && now - lastAutoClosed < 200; } /* @@ -922,7 +919,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, debug("VComboBox.SP: onClose(" + event.isAutoClosed() + ")"); } if (event.isAutoClosed()) { - lastAutoClosed = (new Date()).getTime(); + lastAutoClosed = new Date().getTime(); } } @@ -999,8 +996,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, */ String getPreferredHeight(int pageItemsCount) { if (currentSuggestions.size() > 0) { - final int pixels = (getPreferredHeight() - / currentSuggestions.size()) * pageItemsCount; + final int pixels = getPreferredHeight() + / currentSuggestions.size() * pageItemsCount; return pixels + "px"; } else { return ""; @@ -1022,9 +1019,10 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, final Iterator<ComboBoxSuggestion> it = suggestions.iterator(); boolean isFirstIteration = true; while (it.hasNext()) { - final ComboBoxSuggestion s = it.next(); - final MenuItem mi = new MenuItem(s.getDisplayString(), true, s); - String style = s.getStyle(); + final ComboBoxSuggestion suggestion = it.next(); + final MenuItem mi = new MenuItem(suggestion.getDisplayString(), + true, suggestion); + String style = suggestion.getStyle(); if (style != null) { mi.addStyleName("v-filterselect-item-" + style); } @@ -1040,20 +1038,23 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, selectItem(mi); } - if (currentSuggestion != null && s.getOptionKey() + if (currentSuggestion != null && suggestion.getOptionKey() .equals(currentSuggestion.getOptionKey())) { - // refresh also selected caption and icon in case they have - // been updated on the server + // Refresh also selected caption and icon in case they have + // been updated on the server, e.g. just the item has been + // updated, but selection (from state) has stayed the same. + // FIXME need to update selected item caption separately, if + // the selected item is not in "active data range" that is + // being sent to the client. Then this can be removed. if (currentSuggestion.getReplacementString() .equals(tb.getText())) { - currentSuggestion = s; + currentSuggestion = suggestion; selectItem(mi); setSelectedCaption( currentSuggestion.getReplacementString()); setSelectedItemIcon(currentSuggestion.getIconUri()); } } - isFirstIteration = false; } } @@ -1080,8 +1081,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, // do not send a value change event if null was and // stays selected if (!"".equals(enteredItemValue) - || (selectedOptionKey != null - && !"".equals(selectedOptionKey))) { + || selectedOptionKey != null + && !"".equals(selectedOptionKey)) { doItemAction(potentialExactMatch, true); } suggestionPopup.hide(); @@ -1099,8 +1100,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, // TODO try to select the new value if it matches what was // sent for V7 compatibility } - } else if (item != null && !"".equals(lastFilter) && (item.getText() - .toLowerCase().contains(lastFilter.toLowerCase()))) { + } else if (item != null && !"".equals(lastFilter) && item.getText() + .toLowerCase().contains(lastFilter.toLowerCase())) { doItemAction(item, true); } else { // currentSuggestion has key="" for nullselection @@ -1367,7 +1368,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, showPopup = true; } if (showPopup) { - suggestionPopup.showSuggestions(currentPage, totalMatches); + suggestionPopup.showSuggestions(currentPage); } waitingForFilteringResponse = false; @@ -1523,9 +1524,12 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, * new selected item caption if sent by the server or null - * this is used when the selected item is not on the current * page + * @param selectedIconUri + * new selected item icon if sent by the server or {@ code + * null} to clear */ public void updateSelectionFromServer(String selectedKey, - String selectedCaption) { + String selectedCaption, String selectedIconUri) { boolean oldSuggestionTextMatchTheOldSelection = currentSuggestion != null && currentSuggestion.getReplacementString() .equals(tb.getText()); @@ -1538,10 +1542,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, cancelPendingPostFiltering(); setSelectedCaption(selectedCaption); - if (currentSuggestion != null && serverSelectedKey - .equals(currentSuggestion.getOptionKey())) { - setSelectedItemIcon(currentSuggestion.getIconUri()); - } + + setSelectedItemIcon(selectedIconUri); } } @@ -1617,8 +1619,8 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, /** For internal use only. May be removed or replaced in the future. */ public boolean allowNewItems; - /** For internal use only. May be removed or replaced in the future. */ - public int totalMatches; + /** Total number of suggestions, excluding null selection item. */ + private int totalSuggestions; /** For internal use only. May be removed or replaced in the future. */ public boolean nullSelectionAllowed; @@ -1765,8 +1767,9 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, * last page */ public boolean hasNextPage() { - return (pageLength > 0 - && totalMatches > (currentPage + 1) * pageLength); + return pageLength > 0 + && getTotalSuggestionsIncludingNullSelectionItem() > (currentPage + + 1) * pageLength; } /** @@ -1793,16 +1796,13 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, public void filterOptions(int page, String filter) { debug("VComboBox: filterOptions(" + page + ", " + filter + ")"); - if (filter.equals(lastFilter) && currentPage == page) { - if (!suggestionPopup.isAttached()) { - dataReceivedHandler.startWaitingForFilteringResponse(); - connector.requestPage(currentPage); - } else { - // already have the page - dataReceivedHandler.dataReceived(); - } + if (filter.equals(lastFilter) && currentPage == page + && suggestionPopup.isAttached()) { + // already have the page + dataReceivedHandler.dataReceived(); return; } + if (!filter.equals(lastFilter)) { // when filtering, let the server decide the page unless we've // set the filter to empty and explicitly said that we want to see @@ -1816,11 +1816,15 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, } dataReceivedHandler.startWaitingForFilteringResponse(); - connector.setFilter(filter); - connector.requestPage(currentPage); + connector.requestPage(page, filter); lastFilter = filter; - currentPage = page; + + // If the data was updated from cache, the page has been updated too, if + // not, update + if (dataReceivedHandler.isWaitingForFilteringResponse()) { + currentPage = page; + } } /** For internal use only. May be removed or replaced in the future. */ @@ -1914,7 +1918,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, setText(text); setSelectedItemIcon(suggestion.getIconUri()); - if (!(newKey.equals(selectedOptionKey))) { + if (!newKey.equals(selectedOptionKey)) { selectedOptionKey = newKey; connector.sendSelection(selectedOptionKey); setSelectedCaption(text); @@ -1932,23 +1936,19 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, if (enableDebug) { debug("VComboBox: onNullSelected()"); } - // TODO do we need this feature? - if (nullSelectItem) { - reset(); - return; - } dataReceivedHandler.cancelPendingPostFiltering(); currentSuggestion = null; setText(""); setSelectedItemIcon(null); - if (!"".equals(selectedOptionKey) || (selectedOptionKey != null)) { + if (!"".equals(selectedOptionKey) || selectedOptionKey != null) { selectedOptionKey = ""; setSelectedCaption(""); connector.sendSelection(null); // currentPage = 0; } + updatePlaceholder(); suggestionPopup.hide(); } @@ -1958,7 +1958,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, * side of the item caption text. Set the URI to null to remove the icon. * * @param iconUri - * The URI of the icon + * The URI of the icon, or null to remove icon */ public void setSelectedItemIcon(String iconUri) { @@ -2017,7 +2017,6 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, boolean updatePromptAndSelectionIfMatchFound) { if (selectedKey == null || "".equals(selectedKey)) { currentSuggestion = null; // #13217 - setSelectedItemIcon(null); selectedOptionKey = null; setText(""); } @@ -2042,7 +2041,6 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, } } currentSuggestion = suggestion; - setSelectedItemIcon(suggestion.getIconUri()); // only a single item can be selected break; } @@ -2294,19 +2292,21 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, */ private void reset() { debug("VComboBox: reset()"); - if (currentSuggestion != null) { - String text = currentSuggestion.getReplacementString(); - setText(text); - setSelectedItemIcon(currentSuggestion.getIconUri()); - - selectedOptionKey = currentSuggestion.key; - - } else { - setText(""); - setSelectedItemIcon(null); - updatePlaceholder(); + // just fetch selected information from state + String text = connector.getState().selectedItemCaption; + setText(text == null ? "" : text); + setSelectedItemIcon(connector.getState().selectedItemIcon); + selectedOptionKey = (connector.getState().selectedItemKey); + if (selectedOptionKey == null || "".equals(selectedOptionKey)) { + currentSuggestion = null; // #13217 selectedOptionKey = null; + updatePlaceholder(); + } else { + currentSuggestion = currentSuggestions.stream() + .filter(suggestion -> suggestion.getOptionKey() + .equals(selectedOptionKey)) + .findAny().orElse(null); } lastFilter = ""; @@ -2551,7 +2551,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, * popupopener */ - tb.setWidth((suggestionPopupMinWidth - iconWidth - buttonWidth) + tb.setWidth(suggestionPopupMinWidth - iconWidth - buttonWidth + "px"); } @@ -2563,7 +2563,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, if (!tb.getElement().getStyle().getWidth().endsWith("px")) { int iconWidth = selectedItemIcon == null ? 0 : selectedItemIcon.getOffsetWidth(); - tb.setWidth((tb.getOffsetWidth() - iconWidth) + "px"); + tb.setWidth(tb.getOffsetWidth() - iconWidth + "px"); } } } @@ -2763,4 +2763,52 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, this.allowNewItems = allowNewItems; } + /** + * Sets the total number of suggestions. + * <p> + * NOTE: this excluded the possible null selection item! + * <p> + * NOTE: this just updates the state, but doesn't update any UI. + * + * @since 8.0 + * @param totalSuggestions + * total number of suggestions + */ + public void setTotalSuggestions(int totalSuggestions) { + this.totalSuggestions = totalSuggestions; + } + + /** + * Gets the total number of suggestions, excluding the null selection item. + * + * @since 8.0 + * @return total number of suggestions + */ + public int getTotalSuggestions() { + return totalSuggestions; + } + + /** + * Gets the total number of suggestions, including the possible null + * selection item, if it should be visible. + * + * @return total number of suggestions with null selection items + */ + private int getTotalSuggestionsIncludingNullSelectionItem() { + return getTotalSuggestions() + + (getNullSelectionItemShouldBeVisible() ? 1 : 0); + } + + /** + * Returns null selection item should be visible or not. + * <p> + * NOTE: this checks for any entered filter value, and whether the feature + * is enabled + * + * @since 8.0 + * @return {@code true} if it should be visible, {@code} + */ + public boolean getNullSelectionItemShouldBeVisible() { + return nullSelectionAllowed && "".equals(lastFilter); + } } diff --git a/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java b/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java index a4a9c17f4d..7fec2ae8e0 100644 --- a/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java +++ b/client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java @@ -16,12 +16,15 @@ package com.vaadin.client.ui.combobox; import java.util.List; +import java.util.Objects; +import java.util.logging.Logger; import com.vaadin.client.Profiler; import com.vaadin.client.annotations.OnStateChange; import com.vaadin.client.communication.StateChangeEvent; import com.vaadin.client.connectors.AbstractListingConnector; import com.vaadin.client.connectors.data.HasDataSource; +import com.vaadin.client.data.DataChangeHandler; import com.vaadin.client.data.DataSource; import com.vaadin.client.ui.HasErrorIndicator; import com.vaadin.client.ui.HasRequiredIndicator; @@ -76,6 +79,8 @@ public class ComboBoxConnector extends AbstractListingConnector getWidget().suggestionPopup.updateStyleNames(getState()); + // TODO if the pop up is opened, the actual item should be removed from + // the popup (?) getWidget().nullSelectionAllowed = getState().emptySelectionAllowed; // TODO having this true would mean that the empty selection item comes // from the data source so none needs to be added - currently @@ -87,6 +92,9 @@ public class ComboBoxConnector extends AbstractListingConnector getDataReceivedHandler().serverReplyHandled(); + // all updates except options have been done + getWidget().initDone = true; + Profiler.leave("ComboBoxConnector.onStateChanged update content"); } @@ -99,10 +107,11 @@ public class ComboBoxConnector extends AbstractListingConnector } } - @OnStateChange({ "selectedItemKey", "selectedItemCaption" }) + @OnStateChange({ "selectedItemKey", "selectedItemCaption", "selectedItemIcon" }) private void onSelectionChange() { getDataReceivedHandler().updateSelectionFromServer( - getState().selectedItemKey, getState().selectedItemCaption); + getState().selectedItemKey, getState().selectedItemCaption, + getState().selectedItemIcon); } @Override @@ -164,11 +173,12 @@ public class ComboBoxConnector extends AbstractListingConnector * @param filter * the current filter string */ - public void setFilter(String filter) { - if (filter != getWidget().lastFilter) { + protected void setFilter(String filter) { + if (!Objects.equals(filter, getWidget().lastFilter)) { getDataReceivedHandler().clearPendingNavigation(); + + rpc.setFilter(filter); } - rpc.setFilter(filter); } /** @@ -183,22 +193,27 @@ public class ComboBoxConnector extends AbstractListingConnector * the page number to get or -1 to let the server/connector * decide based on current selection (possibly loading more data * from the server) + * @param filter + * the filter to apply, never {@code null} */ - public void requestPage(int page) { + public void requestPage(int page, String filter) { + setFilter(filter); + if (page < 0) { if (getState().scrollToSelectedItem) { - getDataSource().ensureAvailability(0, 10000); + // TODO this should be optimized not to try to fetch everything + getDataSource().ensureAvailability(0, getDataSource().size()); return; } else { page = 0; } } - int adjustment = getWidget().nullSelectionAllowed - && "".equals(getWidget().lastFilter) ? 1 : 0; + int adjustment = getWidget().nullSelectionAllowed && "".equals(filter) + ? 1 : 0; int startIndex = Math.max(0, page * getWidget().pageLength - adjustment); int pageLength = getWidget().pageLength > 0 ? getWidget().pageLength - : 10000; + : getDataSource().size(); getDataSource().ensureAvailability(startIndex, pageLength); } @@ -261,7 +276,7 @@ public class ComboBoxConnector extends AbstractListingConnector public void setDataSource(DataSource<JsonObject> dataSource) { super.setDataSource(dataSource); dataChangeHandlerRegistration = dataSource - .addDataChangeHandler(range -> refreshData()); + .addDataChangeHandler(new PagedDataChangeHandler(dataSource)); } @Override @@ -278,14 +293,13 @@ public class ComboBoxConnector extends AbstractListingConnector private void refreshData() { updateCurrentPage(); - getWidget().currentSuggestions.clear(); - int start = getWidget().currentPage * getWidget().pageLength; int end = getWidget().pageLength > 0 ? start + getWidget().pageLength : getDataSource().size(); - if (getWidget().nullSelectionAllowed - && "".equals(getWidget().lastFilter)) { + getWidget().currentSuggestions.clear(); + + if (getWidget().getNullSelectionItemShouldBeVisible()) { // add special null selection item... if (isFirstPage()) { addEmptySelectionItem(); @@ -294,13 +308,14 @@ public class ComboBoxConnector extends AbstractListingConnector start = start - 1; } // in either case, the last item to show is - // shifted by one - end = end - 1; + // shifted by one, unless no paging is used + if (getState().pageLength != 0) { + end = end - 1; + } } updateSuggestions(start, end); - getWidget().totalMatches = getDataSource().size() - + (getState().emptySelectionAllowed ? 1 : 0); + getWidget().setTotalSuggestions(getDataSource().size()); getDataReceivedHandler().dataReceived(); } @@ -308,16 +323,18 @@ public class ComboBoxConnector extends AbstractListingConnector private void updateSuggestions(int start, int end) { for (int i = start; i < end; ++i) { JsonObject row = getDataSource().getRow(i); - if (row != null) { String key = getRowKey(row); String caption = row.getString(DataCommunicatorConstants.NAME); String style = row.getString(ComboBoxConstants.STYLE); String untranslatedIconUri = row .getString(ComboBoxConstants.ICON); - getWidget().currentSuggestions - .add(getWidget().new ComboBoxSuggestion(key, caption, - style, untranslatedIconUri)); + ComboBoxSuggestion suggestion = getWidget().new ComboBoxSuggestion( + key, caption, style, untranslatedIconUri); + getWidget().currentSuggestions.add(suggestion); + } else { + // there is not enough options to fill the page + return; } } } @@ -360,4 +377,54 @@ public class ComboBoxConnector extends AbstractListingConnector getWidget().currentPage = 0; } } + + private static final Logger LOGGER = Logger + .getLogger(ComboBoxConnector.class.getName()); + + private class PagedDataChangeHandler implements DataChangeHandler { + + private final DataSource<?> dataSource; + + public PagedDataChangeHandler(DataSource<?> dataSource) { + this.dataSource = dataSource; + } + + @Override + public void dataUpdated(int firstRowIndex, int numberOfRows) { + // NOOP since dataAvailable is always triggered afterwards + } + + @Override + public void dataRemoved(int firstRowIndex, int numberOfRows) { + // NOOP since dataAvailable is always triggered afterwards + } + + @Override + public void dataAdded(int firstRowIndex, int numberOfRows) { + // NOOP since dataAvailable is always triggered afterwards + } + + @Override + public void dataAvailable(int firstRowIndex, int numberOfRows) { + refreshData(); + } + + @Override + public void resetDataAndSize(int estimatedNewDataSize) { + if (getState().pageLength == 0) { + if (getWidget().suggestionPopup.isShowing()) { + dataSource.ensureAvailability(0, estimatedNewDataSize); + } + // else lets just wait till the popup is opened before + // everything is fetched to it. this could be optimized later on + // to fetch everything if in-memory data is used. + } else { + // reset data: clear any current options, set page to 0 + getWidget().currentPage = 0; + getWidget().currentSuggestions.clear(); + dataSource.ensureAvailability(0, getState().pageLength); + } + } + + } } diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java index a2db94a8c6..ed1e9d8beb 100644 --- a/server/src/main/java/com/vaadin/ui/ComboBox.java +++ b/server/src/main/java/com/vaadin/ui/ComboBox.java @@ -575,6 +575,9 @@ public class ComboBox<T> extends AbstractSingleSelect<T> public void setItemCaptionGenerator( ItemCaptionGenerator<T> itemCaptionGenerator) { super.setItemCaptionGenerator(itemCaptionGenerator); + if (getSelectedItem().isPresent()) { + updateSelectedItemCaption(); + } } /** @@ -614,6 +617,10 @@ public class ComboBox<T> extends AbstractSingleSelect<T> @Override public void setItemIconGenerator(IconGenerator<T> itemIconGenerator) { super.setItemIconGenerator(itemIconGenerator); + + if (getSelectedItem().isPresent()) { + updateSelectedItemIcon(); + } } @Override @@ -631,7 +638,7 @@ public class ComboBox<T> extends AbstractSingleSelect<T> */ public void setNewItemHandler(NewItemHandler newItemHandler) { this.newItemHandler = newItemHandler; - getState().allowNewItems = (newItemHandler != null); + getState().allowNewItems = newItemHandler != null; markAsDirty(); } @@ -670,14 +677,32 @@ public class ComboBox<T> extends AbstractSingleSelect<T> protected void doSetSelectedKey(String key) { super.doSetSelectedKey(key); + updateSelectedItemCaption(); + updateSelectedItemIcon(); + } + + private void updateSelectedItemCaption() { String selectedCaption = null; - T value = getDataCommunicator().getKeyMapper().get(key); + T value = getDataCommunicator().getKeyMapper().get(getSelectedKey()); if (value != null) { selectedCaption = getItemCaptionGenerator().apply(value); } getState().selectedItemCaption = selectedCaption; } + private void updateSelectedItemIcon() { + String selectedItemIcon = null; + T value = getDataCommunicator().getKeyMapper().get(getSelectedKey()); + if (value != null) { + Resource icon = getItemIconGenerator().apply(value); + if (icon != null) { + selectedItemIcon = ResourceReference + .create(icon, ComboBox.this, null).getURL(); + } + } + getState().selectedItemIcon = selectedItemIcon; + } + @Override protected Element writeItem(Element design, T item, DesignContext context) { Element element = design.appendElement("option"); @@ -748,7 +773,7 @@ public class ComboBox<T> extends AbstractSingleSelect<T> "filterConverter cannot be null"); SerializableFunction<String, C> convertOrNull = filterText -> { - if (filterText == null) { + if (filterText == null || filterText.isEmpty()) { return null; } @@ -814,10 +839,12 @@ public class ComboBox<T> extends AbstractSingleSelect<T> extends SerializableBiPredicate<String, String> { /** - * Check item caption against entered text + * Check item caption against entered text. * * @param itemCaption + * the caption of the item to filter, not {@code null} * @param filterText + * user entered filter, not {@code null} * @return {@code true} if item passes the filter and should be listed, * {@code false} otherwise */ diff --git a/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java b/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java index a7be4865a7..666fff1e58 100644 --- a/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java +++ b/shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxState.java @@ -93,4 +93,11 @@ public class ComboBoxState extends AbstractSingleSelectState { */ public String emptySelectionCaption = ""; + /** + * Selected item icon uri. + * + * @since 8.0 + */ + public String selectedItemIcon; + } diff --git a/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java b/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java index 64023c7d69..e0e4799c15 100644 --- a/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java +++ b/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java @@ -109,6 +109,15 @@ public class ComboBoxElement extends AbstractSelectElement { return !isReadOnly(getInputField()); } + /** + * Checks whether the suggestion popup is open or not. + * + * @return {@code true} if popup is open, {@code false if not} + */ + public boolean isPopupOpen() { + return isElementPresent(bySuggestionPopup); + } + /* * Workaround selenium's bug: sendKeys() will not send left parentheses * properly. See #14048. @@ -147,7 +156,7 @@ public class ComboBoxElement extends AbstractSelectElement { * @return List of suggestion texts */ public List<String> getPopupSuggestions() { - List<String> suggestionsTexts = new ArrayList<String>(); + List<String> suggestionsTexts = new ArrayList<>(); List<WebElement> suggestions = getPopupSuggestionElements(); for (WebElement suggestion : suggestions) { String text = suggestion.getText(); @@ -280,7 +289,7 @@ public class ComboBoxElement extends AbstractSelectElement { private void clickElement(WebElement element) { if (isFirefox()) { // Workaround for Selenium/TB and Firefox 45 issue - ((TestBenchElement) (element)).clickHiddenElement(); + ((TestBenchElement) element).clickHiddenElement(); } else { element.click(); } diff --git a/tests/screenshots b/tests/screenshots -Subproject c233d26d0af136183eef3707461e2df0d026ff4 +Subproject 9a77b414d2eed1d74c559916d81df9b92ce680e diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxBackEndRequests.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxBackEndRequests.java new file mode 100644 index 0000000000..be2ce6c67e --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxBackEndRequests.java @@ -0,0 +1,72 @@ +package com.vaadin.tests.components.combobox; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.ui.combobox.ComboBoxState; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.tests.util.LoggingItemDataProvider; +import com.vaadin.ui.Button; +import com.vaadin.ui.CheckBox; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.Panel; +import com.vaadin.ui.VerticalLayout; + +public class ComboBoxBackEndRequests extends AbstractTestUI { + + public static final String PAGE_LENGTH_REQUEST_PARAMETER = "pageLength"; + public static final String ITEMS_REQUEST_PARAMETER = "items"; + public static final int DEFAULT_NUMBER_OF_ITEMS = 200; + public static final int DEFAULT_PAGE_LENGTH = new ComboBoxState().pageLength; + + @Override + protected void setup(VaadinRequest request) { + int pageLength = DEFAULT_PAGE_LENGTH; + int items = DEFAULT_NUMBER_OF_ITEMS; + if (request.getParameter(PAGE_LENGTH_REQUEST_PARAMETER) != null) { + pageLength = Integer.parseInt(request + .getParameter(PAGE_LENGTH_REQUEST_PARAMETER).toString()); + } + if (request.getParameter(ITEMS_REQUEST_PARAMETER) != null) { + items = Integer.parseInt( + request.getParameter(ITEMS_REQUEST_PARAMETER).toString()); + } + + ComboBox<String> cb = new ComboBox<>(); + cb.setPageLength(pageLength); + VerticalLayout logContainer = new VerticalLayout(); + logContainer.setSpacing(false); + + CheckBox textInputAllowed = new CheckBox("textInputAllowed", + cb.isTextInputAllowed()); + textInputAllowed.addValueChangeListener( + event -> cb.setTextInputAllowed(textInputAllowed.getValue())); + + CheckBox emptySelectionAllowed = new CheckBox("emptySelectionAllowed", + cb.isEmptySelectionAllowed()); + emptySelectionAllowed.addValueChangeListener(event -> cb + .setEmptySelectionAllowed(emptySelectionAllowed.getValue())); + + CheckBox scrollToSelectedItem = new CheckBox("scrollToSelectedItem", + cb.isScrollToSelectedItem()); + scrollToSelectedItem.addValueChangeListener(event -> cb + .setScrollToSelectedItem(scrollToSelectedItem.getValue())); + + VerticalLayout options = new VerticalLayout(textInputAllowed, + emptySelectionAllowed, scrollToSelectedItem, + new Button("Swap DataProvider", + event -> cb.setDataProvider(new LoggingItemDataProvider( + 500, logContainer))), + new Button("Clear logs", + event -> logContainer.removeAllComponents())); + + cb.setDataProvider(new LoggingItemDataProvider(items, logContainer)); + Panel panel = new Panel(logContainer); + addComponent(new HorizontalLayout(cb, panel, options)); + } + + @Override + protected Integer getTicketNumber() { + return 8496; + } + +} diff --git a/uitest/src/main/java/com/vaadin/tests/util/LoggingItemDataProvider.java b/uitest/src/main/java/com/vaadin/tests/util/LoggingItemDataProvider.java new file mode 100644 index 0000000000..0cc12c1ed6 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/util/LoggingItemDataProvider.java @@ -0,0 +1,71 @@ +package com.vaadin.tests.util; + +import java.util.Locale; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import com.vaadin.data.provider.CallbackDataProvider; +import com.vaadin.data.provider.Query; +import com.vaadin.ui.Label; +import com.vaadin.ui.VerticalLayout; + +/** + * CallbackDataProvider that logs info in UI on Queries it receives. + */ +public class LoggingItemDataProvider + extends CallbackDataProvider<String, String> { + + private int counter; + + public LoggingItemDataProvider(int size, VerticalLayout logContainer) { + super(q -> fetch(logContainer, q, size), + q -> size(logContainer, q, size)); + } + + private static Stream<String> fetch(VerticalLayout logContainer, + Query<String, String> query, int size) { + log("FETCH", query, logContainer); + return itemStream(query, size).skip(query.getOffset()) + .limit(query.getLimit()); + } + + private static int size(VerticalLayout logContainer, + Query<String, String> query, int size) { + log("SIZE", query, logContainer); + return size(query, size); + } + + private static Stream<String> itemStream(Query<String, String> q, + int size) { + Stream<String> stream = IntStream.range(0, size) + .mapToObj(i -> "Item " + i); + String filterText = q.getFilter().orElse("").toLowerCase(Locale.US); + + if (filterText.isEmpty()) { + return stream; + } + + return stream.filter( + text -> text.toLowerCase(Locale.US).contains(filterText)); + } + + private static int size(Query<String, String> q, int size) { + if (!q.getFilter().orElse("").isEmpty()) { + return (int) itemStream(q, size).count(); + } + return size; + } + + private static Label log(String queryType, Query<String, String> query, + VerticalLayout logContainer) { + int componentCount = logContainer.getComponentCount(); + Label label = new Label(componentCount + ": " + queryType + " " + + query.getOffset() + " " + query.getLimit() + " " + + query.getFilter().orElse("")); + logContainer.addComponentAsFirst(label); + label.setId(queryType + "-" + componentCount); + label.setStyleName("log"); + return label; + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxBackEndRequestsTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxBackEndRequestsTest.java new file mode 100644 index 0000000000..73447a09c4 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxBackEndRequestsTest.java @@ -0,0 +1,368 @@ +package com.vaadin.tests.components.combobox; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.math.BigDecimal; +import java.math.RoundingMode; +import java.util.List; + +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.CheckBoxElement; +import com.vaadin.testbench.elements.ComboBoxElement; +import com.vaadin.tests.tb3.SingleBrowserTest; + +public class ComboBoxBackEndRequestsTest extends SingleBrowserTest { + + @Before + public void open() { + openTestURL("?debug"); + } + + private void open(int pageLength, int items) { + openTestURL(ComboBoxBackEndRequests.PAGE_LENGTH_REQUEST_PARAMETER + "=" + + pageLength + "&" + + ComboBoxBackEndRequests.ITEMS_REQUEST_PARAMETER + "=" + items + + "&restartApplication"); + } + + @Test + public void testInitialLoad_onlySizeAndFirstItemsRequested() { + verifyInitialLoadRequests(); + } + + @Test + public void testOpeningDropDown_noRequests() { + verifyInitialLoadRequests(); + + clearLogs(); + openPopup(); + + verifyNumberOrRequests( + "opening drop down should not have caused requests", 0); + + // select something to close popup + selectByClick("Item 2"); + + verifyNumberOrRequests("selecting should have not caused requests", 0); + + openPopup(); + verifyNumberOrRequests( + "opening drop down should not have caused requests", 0); + } + + @Test + public void testNoPaging_nullSelectionAllowed() { + final int pageLength = 0; + final int items = 20; + open(pageLength, items); + + verifyInitialLoadRequests(); + + clearLogs(); + + openPopup(); + + verifyNumberOrRequests("extra request expected", 1); + // TODO with pageLength 0, the cache strategy forces to fetch the whole + // set for opening popup + verifyFetchRequest(0, 0, items, null); + + verifyPopupItems(true, 0, items - 1); + + // pop up status not shown for only one page! + } + + @Test + public void testNoPaging_nullSelectionDisallowed() { + final int pageLength = 0; + final int items = 20; + open(pageLength, items); + + verifyInitialLoadRequests(); + + clearLogs(); + + triggerNullSelectionAllowed(false); + + openPopup(); + + verifyNumberOrRequests("extra request expected", 1); + // TODO the cache strategy forces to fetch the whole range, instead of + // just the missing 40-50 + verifyFetchRequest(0, 0, items, null); + + verifyPopupItems(false, 0, items - 1); + + // pop up status not shown for only one page! + } + + @Test + public void testPagingWorks_nullSelectionAllowed_defaultSizes() { + verifyPopupPages(ComboBoxBackEndRequests.DEFAULT_PAGE_LENGTH, + ComboBoxBackEndRequests.DEFAULT_NUMBER_OF_ITEMS, true); + } + + @Test + public void testPagingWorks_nullSelectionDisallowed_defaultSizes() { + triggerNullSelectionAllowed(false); + + verifyPopupPages(ComboBoxBackEndRequests.DEFAULT_PAGE_LENGTH, + ComboBoxBackEndRequests.DEFAULT_NUMBER_OF_ITEMS, false); + } + + @Test + public void testInitialPage_pageLengthBiggerThanInitialCache() { + // initial request is for 40 items + final int pageLength = 50; + final int items = 100; + + open(pageLength, items); + + verifyNumberOrRequests("three initial requests expected", 3); + verifySizeRequest(0, null); + verifyFetchRequest(1, 0, 40, null); + verifyFetchRequest(2, 40, 60, null); + + clearLogs(); + + verifyPopupPages(pageLength, items, true); + + // browsing through the pages should't have caused more requests + verifyNumberOrRequests("no additional requests should have happened", + 0); + } + + @Test + public void testPagingWorks_nullSelectionAllowed_customSizes() { + final int pageLength = 23; + final int items = 333; + open(pageLength, items); + + // with null selection allowed + verifyPopupPages(pageLength, items, true); + } + + @Test + public void testPagingWorks_nullSelectionDisallowed_customSizes() { + final int pageLength = 23; + final int items = 333; + open(pageLength, items); + + triggerNullSelectionAllowed(false); + + verifyPopupPages(pageLength, items, false); + } + + @Test + @Ignore("cache strategy is still broken for CB") + public void testPaging_usesCachedData() { + verifyInitialLoadRequests(); + clearLogs(); + openPopup(); + verifyNumberOrRequests( + "opening drop down should not have caused requests", 0); + + nextPage(); + + verifyNumberOrRequests("next page should have not caused more requests", + 0); + } + + @Test + public void testPaging_nullSelectionAllowed_correctNumberOfItemsShown() { + verifyInitialLoadRequests(); + openPopup(); + + verifyPopupItems(true, 0, 8); + + nextPage(); + + verifyPopupItems(false, 9, 18); + } + + @Test + public void testPaging_nullSelectionDisallowed_correctNumberOfItemsShown() { + verifyInitialLoadRequests(); + triggerNullSelectionAllowed(false); + + openPopup(); + + verifyPopupItems(false, 0, 9); + + nextPage(); + + verifyPopupItems(false, 10, 19); + } + + private void triggerNullSelectionAllowed(boolean expectedValue) { + $(CheckBoxElement.class).caption("emptySelectionAllowed").first() + .click(); + assertEquals(expectedValue, $(CheckBoxElement.class) + .caption("emptySelectionAllowed").first().isChecked()); + } + + private void clearLogs() { + $(ButtonElement.class).caption("Clear logs").first().click(); + verifyNumberOrRequests("logs should have been cleared", 0); + } + + private void selectByClick(String itemText) { + $(ComboBoxElement.class).first().getPopupSuggestionElements().stream() + .filter(element -> element.getText().equals(itemText)) + .findFirst().get().click(); + assertEquals("Wrong selected", itemText, + $(ComboBoxElement.class).first().getValue()); + } + + private void nextPage() { + assertTrue("no next page available", + $(ComboBoxElement.class).first().openNextPage()); + } + + private void previousPage() { + assertTrue("no previous page available", + $(ComboBoxElement.class).first().openPrevPage()); + } + + private void openPopup() { + ComboBoxElement element = $(ComboBoxElement.class).first(); + assertFalse("popup shouldn't be open", element.isPopupOpen()); + + element.getSuggestionPopup(); + + assertTrue("popup should be open", element.isPopupOpen()); + } + + private void verifyPopupPages(int pageSize, int totalItems, + boolean nullSelectionAllowed) { + final int numberOfPages = new BigDecimal( + totalItems + (nullSelectionAllowed ? 1 : 0)) + .divide(new BigDecimal(pageSize), RoundingMode.UP) + .intValue(); + openPopup(); + + verifyPopupStatus(pageSize, 1, numberOfPages, totalItems, + nullSelectionAllowed); + + for (int i = 2; i <= numberOfPages; i++) { + nextPage(); + verifyPopupStatus(pageSize, i, numberOfPages, totalItems, + nullSelectionAllowed); + } + + assertFalse("there should not be a next page", + $(ComboBoxElement.class).first().openNextPage()); + + if (numberOfPages < 2) { + return; + } + for (int i = numberOfPages - 1; i > 0; i--) { + previousPage(); + verifyPopupStatus(pageSize, i, numberOfPages, totalItems, + nullSelectionAllowed); + } + + assertFalse("there should not be a previous page", + $(ComboBoxElement.class).first().openPrevPage()); + } + + private void verifyPopupStatus(int pageSize, int currentPage, + int numberOfPages, int totalItems, boolean nullSelectionAllowed) { + int start; + int end; + + if (currentPage == 1) { // first page + start = 1; + end = pageSize - (nullSelectionAllowed ? 1 : 0); + } else if (currentPage == numberOfPages) { // last page + final int lastPageSize = (totalItems + + (nullSelectionAllowed ? 1 : 0)) % pageSize; + + start = pageSize * (currentPage - 1) + + (nullSelectionAllowed ? 0 : 1); + end = start + (lastPageSize == 0 ? pageSize : lastPageSize) - 1; + } else { // all other + start = pageSize * (currentPage - 1) + + (nullSelectionAllowed ? 0 : 1); + end = pageSize * currentPage - (nullSelectionAllowed ? 1 : 0); + } + + verifyPopupStatus(start, end, totalItems, currentPage, numberOfPages); + } + + private void verifyPopupStatus(int start, int end, int totalItems, + int currentPage, int numberOfPages) { + WebElement element = findElement(By.className("v-filterselect-status")); + assertEquals( + "Wrong status text on popup page " + currentPage + "/" + + numberOfPages, + start + "-" + end + "/" + totalItems, element.getText()); + } + + private void verifyPopupItems(boolean hasNullSelectionItem, int startIndex, + int lastIndex) { + List<String> popupSuggestions = $(ComboBoxElement.class).first() + .getPopupSuggestions(); + if (hasNullSelectionItem) { + String text = popupSuggestions.remove(0); + assertEquals("nullSelectionItem should be visible on page", " ", + text); + } + assertEquals("invalid number of suggestions", + lastIndex - startIndex + 1, popupSuggestions.size()); + for (int i = startIndex; i <= lastIndex; i++) { + assertEquals("unexpected item", "Item " + i, + popupSuggestions.get(i - startIndex)); + } + } + + private void verifyInitialLoadRequests() { + verifyNumberOrRequests("two initial requests expected", 2); + verifySizeRequest(0, null); + verifyFetchRequest(1, 0, 40, null); + } + + private WebElement verifyFetchRequest(int requestNumber, int startIndex, + int size, String filter) { + WebElement element = verifyRequest(requestNumber, "FETCH"); + // format is 1: FETCH 0 40 _filter_ + final String actual = element.getText(); + String expected = requestNumber + ": FETCH " + startIndex + " " + size + + (filter == null ? "" : " " + filter); + assertEquals("invalid fetch data", expected, actual); + return element; + + } + + private WebElement verifySizeRequest(int index, String filter) { + WebElement element = verifyRequest(index, "SIZE"); + if (filter != null) { + // format is "0: SIZE 0 2147483647 _filter_ + String currentFilter = element.getText().split("2147483647")[1]; + assertEquals("Wrong filter", filter, currentFilter); + } + return element; + } + + private WebElement verifyRequest(int index, String queryType) { + WebElement element = findElement(By.id(queryType + "-" + index)); + return element; + } + + private void verifyNumberOrRequests(String message, int numberOfRequests) { + List<WebElement> logRows = getLogRows(); + assertEquals(message, numberOfRequests, logRows.size()); + } + + private List<WebElement> getLogRows() { + return findElements(By.className("log")); + } +} |