Skip to content

Selection handling fix when adding new item to ComboBox. #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
makimat wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Selection handling fix when adding new item to ComboBox. #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions client/src/main/java/com/vaadin/client/ui/VComboBox.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,8 @@ public class SuggestionMenu extends MenuBar
}
});

private String handledNewItem = null;

/**
* Default constructor
*/
Expand Down Expand Up @@ -1134,7 +1136,11 @@ public void setSuggestions(Collection<ComboBoxSuggestion> suggestions) {
public void actOnEnteredValueAfterFiltering(String enteredItemValue) {
debug("VComboBox.SM: doPostFilterSelectedItemAction()");
final MenuItem item = getSelectedItem();

boolean handledOnServer = handledNewItem == enteredItemValue;
if (handledOnServer) {
// clear value to mark it as handled
handledNewItem = null;
}
// check for exact match in menu
int p = getItems().size();
if (p > 0) {
Expand All @@ -1151,13 +1157,17 @@ public void actOnEnteredValueAfterFiltering(String enteredItemValue) {
doItemAction(potentialExactMatch, true);
}
suggestionPopup.hide();
lastNewItemString = null;
connector.clearNewItemHandlingIfMatch(enteredItemValue);
return;
}
}
}
if ("".equals(enteredItemValue) && nullSelectionAllowed) {

if (!handledOnServer && "".equals(enteredItemValue)
&& nullSelectionAllowed) {
onNullSelected();
} else if (allowNewItems) {
} else if (!handledOnServer && allowNewItems) {
if (!enteredItemValue.equals(lastNewItemString)) {
// Store last sent new item string to avoid double sends
lastNewItemString = enteredItemValue;
Expand All @@ -1182,6 +1192,10 @@ public void actOnEnteredValueAfterFiltering(String enteredItemValue) {
}
}
suggestionPopup.hide();

if (handledOnServer || !allowNewItems) {
lastNewItemString = null;
}
}

private static final String SUBPART_PREFIX = "item";
Expand Down Expand Up @@ -1319,6 +1333,10 @@ public void highlightSelectedItem() {
}
}
}

public void markNewItemsHandled(String handledNewItem) {
this.handledNewItem = handledNewItem;
}
}

private String getSuggestionKey(MenuItem item) {
Expand Down Expand Up @@ -1442,9 +1460,15 @@ public void dataReceived() {
waitingForFilteringResponse = false;

if (pendingUserInput != null) {
boolean pendingHandled = suggestionPopup.menu.handledNewItem == pendingUserInput;
suggestionPopup.menu
.actOnEnteredValueAfterFiltering(pendingUserInput);
pendingUserInput = null;
if (!allowNewItems || (pendingHandled
&& suggestionPopup.menu.handledNewItem == null)) {
pendingUserInput = null;
} else {
waitingForFilteringResponse = true;
}
} else if (popupOpenerClicked) {
// make sure the current item is selected in the popup
suggestionPopup.menu.highlightSelectedItem();
Expand Down Expand Up @@ -1474,6 +1498,10 @@ public void reactOnInputWhenReady(String value) {
filterOptions(0, value);
}

public boolean isPending(String value) {
return value != null && value.equals(pendingUserInput);
}

/*
* This method navigates to the proper item in the combobox page. This
* should be executed after setSuggestions() method which is called from
Expand Down Expand Up @@ -1541,7 +1569,6 @@ private void cancelPendingPostFiltering() {
*/
public void serverReplyHandled() {
popupOpenerClicked = false;
lastNewItemString = null;

// if (!initDone) {
// debug("VComboBox: init done, updating widths");
Expand Down Expand Up @@ -2542,7 +2569,7 @@ public void onBlur(BlurEvent event) {
// Send new items when clicking out with the mouse.
if (!readonly) {
if (textInputEnabled && allowNewItems
&& (currentSuggestion == null || tb.getText().equals(
&& (currentSuggestion == null || !tb.getText().equals(
currentSuggestion.getReplacementString()))) {
dataReceivedHandler.reactOnInputWhenReady(tb.getText());
} else {
Expand Down
88 changes: 88 additions & 0 deletions client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.vaadin.shared.data.DataCommunicatorConstants;
import com.vaadin.shared.data.selection.SelectionServerRpc;
import com.vaadin.shared.ui.Connect;
import com.vaadin.shared.ui.combobox.ComboBoxClientRpc;
import com.vaadin.shared.ui.combobox.ComboBoxConstants;
import com.vaadin.shared.ui.combobox.ComboBoxServerRpc;
import com.vaadin.shared.ui.combobox.ComboBoxState;
Expand All @@ -55,10 +56,27 @@ public class ComboBoxConnector extends AbstractListingConnector

private Registration dataChangeHandlerRegistration;

/**
* new item value that has been sent to server but selection handling hasn't
* been performed for it yet
*/
private String pendingNewItemValue = null;

@Override
protected void init() {
super.init();
getWidget().connector = this;
registerRpc(ComboBoxClientRpc.class, new ComboBoxClientRpc() {
@Override
public void newItemNotAdded(String itemValue) {
if (itemValue != null && itemValue.equals(pendingNewItemValue)
&& isNewItemStillPending()) {
// handled but not added, perform (de-)selection handling
// immediately
completeNewItemHandling();
}
}
});
}

@Override
Expand Down Expand Up @@ -158,6 +176,11 @@ public void setWidgetEnabled(boolean widgetEnabled) {
* user entered string value for the new item
*/
public void sendNewItem(String itemValue) {
if (itemValue != null && !itemValue.equals(pendingNewItemValue)) {
// clear any previous handling as outdated
clearNewItemHandling();
}
pendingNewItemValue = itemValue;
rpc.createNewItem(itemValue);
getDataReceivedHandler().clearPendingNavigation();
}
Expand All @@ -180,6 +203,19 @@ protected void setFilter(String filter) {
}
}

/**
* Confirm with the widget that the pending new item value is still pending.
*
* This method is for internal use only and may be removed in future
* versions.
*
* @return {@code true} if the value is still pending, {@code false} if
* there is no pending value or it doesn't match
*/
private boolean isNewItemStillPending() {
return getDataReceivedHandler().isPending(pendingNewItemValue);
}

/**
* Send a message to the server to request a page of items with the current
* filter.
Expand Down Expand Up @@ -377,6 +413,58 @@ && getWidget().selectedOptionKey != null) {
}
}

/**
* If previous calls to refreshData haven't sorted out the selection yet,
* enforce it.
*
* This method is for internal use only and may be removed in future
* versions.
*/
private void completeNewItemHandling() {
// ensure the widget hasn't got a new selection in the meantime
if (isNewItemStillPending()) {
// mark new item for selection handling on the widget
getWidget().suggestionPopup.menu
.markNewItemsHandled(pendingNewItemValue);
// clear pending value
pendingNewItemValue = null;
// trigger the final selection handling
refreshData();
} else {
clearNewItemHandling();
}
}

/**
* Clears the pending new item value if the widget's pending value no longer
* matches.
*
* This method is for internal use only and may be removed in future
* versions.
*/
private void clearNewItemHandling() {
// never clear pending value before it has been handled
if (!isNewItemStillPending()) {
pendingNewItemValue = null;
}
}

/**
* Clears the new item handling variables if the given value matches the
* pending value.
*
* This method is for internal use only and may be removed in future
* versions.
*
* @param value
* already handled value
*/
public void clearNewItemHandlingIfMatch(String value) {
if (value != null && value.equals(pendingNewItemValue)) {
pendingNewItemValue = null;
}
}

private static final Logger LOGGER = Logger
.getLogger(ComboBoxConnector.class.getName());

Expand Down
23 changes: 20 additions & 3 deletions server/src/main/java/com/vaadin/ui/ComboBox.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.vaadin.server.SerializableToIntFunction;
import com.vaadin.shared.Registration;
import com.vaadin.shared.data.DataCommunicatorConstants;
import com.vaadin.shared.ui.combobox.ComboBoxClientRpc;
import com.vaadin.shared.ui.combobox.ComboBoxConstants;
import com.vaadin.shared.ui.combobox.ComboBoxServerRpc;
import com.vaadin.shared.ui.combobox.ComboBoxState;
Expand Down Expand Up @@ -105,6 +106,16 @@ public interface FetchItemsCallback<T> extends Serializable {
/**
* Handler that adds a new item based on user input when the new items
* allowed mode is active.
* <p>
* NOTE 1: If the new item is rejected the client must be notified of the
* fact via ComboBoxClientRpc or selection handling won't complete.
* </p>
* <p>
* NOTE 2: Selection handling must be completed separately if filtering the
* data source with the same value won't include the new item in the initial
* list of suggestions. Failing to do so will lead to selection handling
* never completing and previous selection remaining on the server.
* </p>
*
* @since 8.0
*/
Expand Down Expand Up @@ -154,9 +165,15 @@ protected void setStyle(T item, String style) {
@Override
public void createNewItem(String itemValue) {
// New option entered
if (getNewItemHandler() != null && itemValue != null
&& !itemValue.isEmpty()) {
getNewItemHandler().accept(itemValue);
if (itemValue != null && !itemValue.isEmpty()) {
if (getNewItemHandler() != null) {
getNewItemHandler().accept(itemValue);
} else {
// selection handling is needed at the client even if
// NewItemHandler is missing
getRpcProxy(ComboBoxClientRpc.class)
.newItemNotAdded(itemValue);
}
}
}

Expand Down
34 changes: 34 additions & 0 deletions shared/src/main/java/com/vaadin/shared/ui/combobox/ComboBoxClientRpc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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.shared.ui.combobox;

import com.vaadin.shared.communication.ClientRpc;

/**
* Server to client RPC interface for ComboBox.
*
* @since 8.0
*/
public interface ComboBoxClientRpc extends ClientRpc {

/**
* Signal the client that attempt to add a new item failed.
*
* @param itemValue
* user entered string value for the new item
*/
public void newItemNotAdded(String itemValue);
}
Loading