From 55ea6dce33bec44140984633a6d3aee7910b89da Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Tue, 7 May 2013 13:06:34 +0000 Subject: More specific workaround for no rows in TreeTable with pagelenght = 0 (#9203) svn changeset:25915/svn branch:6.8 Conflicts: client/src/com/vaadin/client/ui/VScrollTable.java Change-Id: I3f5b9dc988f5911023f77f184f5bd6770bbd9f1b --- client/src/com/vaadin/client/ui/VScrollTable.java | 33 ++++++++++++++++++----- 1 file changed, 27 insertions(+), 6 deletions(-) (limited to 'client') diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index 4d61fba429..b03fa945cd 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -1113,10 +1113,10 @@ public class VScrollTable extends FlowPanel implements HasWidgets, if (firstvisible != lastRequestedFirstvisible && scrollBody != null) { // received 'surprising' firstvisible from server: scroll there firstRowInViewPort = firstvisible; - + /* - * Schedule the scrolling to be executed last so no updates to the rows - * affect scrolling measurements. + * Schedule the scrolling to be executed last so no updates to the + * rows affect scrolling measurements. */ Scheduler.get().scheduleFinally(lazyScroller); } @@ -2104,6 +2104,15 @@ public class VScrollTable extends FlowPanel implements HasWidgets, * might have been (incorrectly) calculated earlier */ + /* + * TreeTable updates stuff in a funky order, so we must set the + * height as zero here before doing the real update to make it + * realize that there is no content, + */ + if (pageLength == totalRows && pageLength == 0) { + scrollBody.setHeight("0px"); + } + int bodyHeight; if (pageLength == totalRows) { /* @@ -3056,7 +3065,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, .hasNext(); columnIndex++) { if (it.next() == this) { break; - } + } } } final int cw = scrollBody.getColWidth(columnIndex); @@ -4796,8 +4805,20 @@ public class VScrollTable extends FlowPanel implements HasWidgets, prepx = 0; } preSpacer.getStyle().setPropertyPx("height", prepx); - int postpx = measureRowHeightOffset(totalRows - 1) - - measureRowHeightOffset(lastRendered); + int postpx; + if (pageLength == 0 && totalRows == pageLength) { + /* + * TreeTable depends on having lastRendered out of sync in some + * situations, which makes this method miss the special + * situation in which one row worth of post spacer to be added + * if there are no rows in the table. #9203 + */ + postpx = measureRowHeightOffset(1); + } else { + postpx = measureRowHeightOffset(totalRows - 1) + - measureRowHeightOffset(lastRendered); + } + if (postpx < 0) { postpx = 0; } -- cgit v1.2.3 From 29eeda592cf0f219582f1ccc9320457ca630a28e Mon Sep 17 00:00:00 2001 From: Johannes Dahlström Date: Wed, 8 May 2013 10:49:20 +0000 Subject: Merge "Clean up Table popup menu close handler to prevent a memory leak" from 6.8 (#11840) svn changeset:25919/svn branch:6.8 Change-Id: If0b414588252226947230cf2f7a5a8ab253cc5d7 --- client/src/com/vaadin/client/ui/VScrollTable.java | 31 ++++++++++++++++------ .../com/vaadin/client/ui/table/TableConnector.java | 5 ++-- 2 files changed, 25 insertions(+), 11 deletions(-) (limited to 'client') diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index b03fa945cd..8705a826cc 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -60,6 +60,7 @@ import com.google.gwt.event.dom.client.ScrollEvent; import com.google.gwt.event.dom.client.ScrollHandler; import com.google.gwt.event.logical.shared.CloseEvent; import com.google.gwt.event.logical.shared.CloseHandler; +import com.google.gwt.event.shared.HandlerRegistration; import com.google.gwt.user.client.Command; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Element; @@ -556,15 +557,24 @@ public class VScrollTable extends FlowPanel implements HasWidgets, *

* For internal use only. May be removed or replaced in the future. */ - public class ContextMenuDetails { + public class ContextMenuDetails implements CloseHandler { public String rowKey; public int left; public int top; + HandlerRegistration closeRegistration; - public ContextMenuDetails(String rowKey, int left, int top) { + public ContextMenuDetails(VContextMenu menu, String rowKey, int left, + int top) { this.rowKey = rowKey; this.left = left; this.top = top; + this.closeRegistration = menu.addCloseHandler(this); + } + + @Override + public void onClose(CloseEvent event) { + contextMenu = null; + closeRegistration.removeHandler(); } } @@ -6021,15 +6031,20 @@ public class VScrollTable extends FlowPanel implements HasWidgets, public void showContextMenu(Event event) { if (enabled && actionKeys != null) { // Show context menu if there are registered action handlers - int left = Util.getTouchOrMouseClientX(event); - int top = Util.getTouchOrMouseClientY(event); - top += Window.getScrollTop(); - left += Window.getScrollLeft(); - contextMenu = new ContextMenuDetails(getKey(), left, top); - client.getContextMenu().showAt(this, left, top); + int left = Util.getTouchOrMouseClientX(event) + + Window.getScrollLeft(); + int top = Util.getTouchOrMouseClientY(event) + + Window.getScrollTop(); + showContextMenu(left, top); } } + public void showContextMenu(int left, int top) { + VContextMenu menu = client.getContextMenu(); + contextMenu = new ContextMenuDetails(menu, getKey(), left, top); + menu.showAt(this, left, top); + } + /** * Has the row been selected? * diff --git a/client/src/com/vaadin/client/ui/table/TableConnector.java b/client/src/com/vaadin/client/ui/table/TableConnector.java index fc31cdf8ea..6e50bab9ab 100644 --- a/client/src/com/vaadin/client/ui/table/TableConnector.java +++ b/client/src/com/vaadin/client/ui/table/TableConnector.java @@ -364,9 +364,8 @@ public class TableConnector extends AbstractHasComponentsConnector implements Widget w = iterator.next(); VScrollTableRow row = (VScrollTableRow) w; if (row.getKey().equals(savedContextMenu.rowKey)) { - getWidget().contextMenu = savedContextMenu; - getConnection().getContextMenu().showAt(row, - savedContextMenu.left, savedContextMenu.top); + row.showContextMenu(savedContextMenu.left, + savedContextMenu.top); } } } -- cgit v1.2.3 From 6d7f5e4bc9b16cb5b6bc3ad2503f251977cbb2af Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Fri, 10 May 2013 13:51:31 +0300 Subject: Eliminate unnecessary conversions of option keys Change-Id: I5f3b267cec0357fe1a1c1a13bafdf3c1f3b6d426 --- client/src/com/vaadin/client/ui/VFilterSelect.java | 6 +++--- client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) (limited to 'client') diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java index b83197ed2d..11f89ee232 100644 --- a/client/src/com/vaadin/client/ui/VFilterSelect.java +++ b/client/src/com/vaadin/client/ui/VFilterSelect.java @@ -147,8 +147,8 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, * * @return The key of the item */ - public int getOptionKey() { - return Integer.parseInt(key); + public String getOptionKey() { + return key; } /** @@ -1281,7 +1281,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, newKey = ""; } else { // normal selection - newKey = String.valueOf(suggestion.getOptionKey()); + newKey = suggestion.getOptionKey(); } String text = suggestion.getReplacementString(); diff --git a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java index e96d27032b..7c82db1b00 100644 --- a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java +++ b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java @@ -131,8 +131,7 @@ public class ComboBoxConnector extends AbstractFieldConnector implements if (optionUidl.hasAttribute("selected")) { if (!getWidget().waitingForFilteringResponse || getWidget().popupOpenerClicked) { - String newSelectedOptionKey = Integer.toString(suggestion - .getOptionKey()); + String newSelectedOptionKey = suggestion.getOptionKey(); if (!newSelectedOptionKey .equals(getWidget().selectedOptionKey) || suggestion.getReplacementString().equals( -- cgit v1.2.3 From b8c6a15da87ff6d2a83e7c0d79dd45ee120fe6a2 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Fri, 10 May 2013 16:25:55 +0300 Subject: Clear items in ComboBox only if changed (#10924) Selection is now sent only as a key, removed redundant attribute on the item. Change-Id: I882d4ae17a1dc91f7a55a0b4a94e47c078ffc022 --- client/src/com/vaadin/client/ui/VFilterSelect.java | 21 +++ .../client/ui/combobox/ComboBoxConnector.java | 147 +++++++++++++-------- server/src/com/vaadin/ui/ComboBox.java | 4 +- .../tests/components/combobox/ComboPushTiming.java | 110 +++++++++++++++ 4 files changed, 222 insertions(+), 60 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/combobox/ComboPushTiming.java (limited to 'client') diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java index 11f89ee232..a3156221b9 100644 --- a/client/src/com/vaadin/client/ui/VFilterSelect.java +++ b/client/src/com/vaadin/client/ui/VFilterSelect.java @@ -168,6 +168,27 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler, public void execute() { onSuggestionSelected(this); } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof FilterSelectSuggestion)) { + return false; + } + FilterSelectSuggestion other = (FilterSelectSuggestion) obj; + 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))) { + return false; + } + if ((iconUri == null && other.iconUri != null) + || (iconUri != null && !iconUri.equals(other.iconUri))) { + return false; + } + return true; + } } /** diff --git a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java index 7c82db1b00..345bdc0cbb 100644 --- a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java +++ b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java @@ -15,7 +15,9 @@ */ package com.vaadin.client.ui.combobox; +import java.util.ArrayList; import java.util.Iterator; +import java.util.List; import com.vaadin.client.ApplicationConnection; import com.vaadin.client.Paintable; @@ -98,24 +100,6 @@ public class ComboBoxConnector extends AbstractFieldConnector implements getWidget().allowNewItem = uidl.hasAttribute("allownewitem"); getWidget().lastNewItemString = null; - getWidget().currentSuggestions.clear(); - if (!getWidget().waitingForFilteringResponse) { - /* - * Clear the current suggestions as the server response always - * includes the new ones. Exception is when filtering, then we need - * to retain the value if the user does not select any of the - * options matching the filter. - */ - getWidget().currentSuggestion = null; - /* - * Also ensure no old items in menu. Unless cleared the old values - * may cause odd effects on blur events. Suggestions in menu might - * not necessary exist in select at all anymore. - */ - getWidget().suggestionPopup.menu.clearItems(); - - } - final UIDL options = uidl.getChildUIDL(0); if (uidl.hasAttribute("totalMatches")) { getWidget().totalMatches = uidl.getIntAttribute("totalMatches"); @@ -123,54 +107,52 @@ public class ComboBoxConnector extends AbstractFieldConnector implements getWidget().totalMatches = 0; } + List newSuggestions = new ArrayList(); + for (final Iterator i = options.getChildIterator(); i.hasNext();) { final UIDL optionUidl = (UIDL) i.next(); final FilterSelectSuggestion suggestion = getWidget().new FilterSelectSuggestion( optionUidl); - getWidget().currentSuggestions.add(suggestion); - if (optionUidl.hasAttribute("selected")) { - if (!getWidget().waitingForFilteringResponse - || getWidget().popupOpenerClicked) { - String newSelectedOptionKey = suggestion.getOptionKey(); - if (!newSelectedOptionKey - .equals(getWidget().selectedOptionKey) - || suggestion.getReplacementString().equals( - getWidget().tb.getText())) { - // Update text field if we've got a new selection - // Also update if we've got the same text to retain old - // text selection behavior - getWidget().setPromptingOff( - suggestion.getReplacementString()); - getWidget().selectedOptionKey = newSelectedOptionKey; - } - } - getWidget().currentSuggestion = suggestion; - getWidget().setSelectedItemIcon(suggestion.getIconUri()); + newSuggestions.add(suggestion); + } + + // only close the popup if the suggestions list has actually changed + boolean suggestionsChanged = !getWidget().initDone + || !newSuggestions.equals(getWidget().currentSuggestions); + + if (suggestionsChanged) { + getWidget().currentSuggestions.clear(); + + if (!getWidget().waitingForFilteringResponse) { + /* + * Clear the current suggestions as the server response always + * includes the new ones. Exception is when filtering, then we + * need to retain the value if the user does not select any of + * the options matching the filter. + */ + getWidget().currentSuggestion = null; + /* + * Also ensure no old items in menu. Unless cleared the old + * values may cause odd effects on blur events. Suggestions in + * menu might not necessary exist in select at all anymore. + */ + getWidget().suggestionPopup.menu.clearItems(); + + } + + for (FilterSelectSuggestion suggestion : newSuggestions) { + getWidget().currentSuggestions.add(suggestion); } } - if ((!getWidget().waitingForFilteringResponse || getWidget().popupOpenerClicked) - && uidl.hasVariable("selected") - && uidl.getStringArrayVariable("selected").length == 0) { - // select nulled - if (!getWidget().waitingForFilteringResponse - || !getWidget().popupOpenerClicked) { - if (!getWidget().focused) { - /* - * client.updateComponent overwrites all styles so we must - * ALWAYS set the prompting style at this point, even though - * we think it has been set already... - */ - getWidget().prompting = false; - getWidget().setPromptingOn(); - } else { - // we have focus in field, prompting can't be set on, - // instead just clear the input - getWidget().tb.setValue(""); - } + // handle selection (null or a single value) + if (uidl.hasVariable("selected")) { + String[] selectedKeys = uidl.getStringArrayVariable("selected"); + if (selectedKeys.length > 0) { + performSelection(selectedKeys[0]); + } else { + resetSelection(); } - getWidget().setSelectedItemIcon(null); - getWidget().selectedOptionKey = null; } if (getWidget().waitingForFilteringResponse @@ -229,6 +211,55 @@ public class ComboBoxConnector extends AbstractFieldConnector implements getWidget().initDone = true; } + private void performSelection(String selectedKey) { + // some item selected + for (FilterSelectSuggestion suggestion : getWidget().currentSuggestions) { + String suggestionKey = suggestion.getOptionKey(); + if (suggestionKey.equals(selectedKey)) { + if (!getWidget().waitingForFilteringResponse + || getWidget().popupOpenerClicked) { + if (!suggestionKey.equals(getWidget().selectedOptionKey) + || suggestion.getReplacementString().equals( + getWidget().tb.getText())) { + // Update text field if we've got a new + // selection + // Also update if we've got the same text to + // retain old text selection behavior + getWidget().setPromptingOff( + suggestion.getReplacementString()); + getWidget().selectedOptionKey = suggestionKey; + } + } + getWidget().currentSuggestion = suggestion; + getWidget().setSelectedItemIcon(suggestion.getIconUri()); + // only a single item can be selected + break; + } + } + } + + private void resetSelection() { + if (!getWidget().waitingForFilteringResponse + || getWidget().popupOpenerClicked) { + // select nulled + if (!getWidget().focused) { + /* + * client.updateComponent overwrites all styles so we must + * ALWAYS set the prompting style at this point, even though we + * think it has been set already... + */ + getWidget().prompting = false; + getWidget().setPromptingOn(); + } else { + // we have focus in field, prompting can't be set on, instead + // just clear the input + getWidget().tb.setValue(""); + } + getWidget().setSelectedItemIcon(null); + getWidget().selectedOptionKey = null; + } + } + @Override public VFilterSelect getWidget() { return (VFilterSelect) super.getWidget(); diff --git a/server/src/com/vaadin/ui/ComboBox.java b/server/src/com/vaadin/ui/ComboBox.java index 0f42749acd..88e895df82 100644 --- a/server/src/com/vaadin/ui/ComboBox.java +++ b/server/src/com/vaadin/ui/ComboBox.java @@ -262,8 +262,8 @@ public class ComboBox extends AbstractSelect implements target.addAttribute("nullselection", true); } target.addAttribute("key", key); - if (isSelected(id) && keyIndex < selectedKeys.length) { - target.addAttribute("selected", true); + if (keyIndex < selectedKeys.length && isSelected(id)) { + // at most one item can be selected at a time selectedKeys[keyIndex++] = key; } target.endTag("so"); diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboPushTiming.java b/uitest/src/com/vaadin/tests/components/combobox/ComboPushTiming.java new file mode 100644 index 0000000000..fe2cffdc4c --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/combobox/ComboPushTiming.java @@ -0,0 +1,110 @@ +package com.vaadin.tests.components.combobox; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +import com.vaadin.data.util.ObjectProperty; +import com.vaadin.event.FieldEvents; +import com.vaadin.event.FieldEvents.BlurEvent; +import com.vaadin.event.FieldEvents.FocusEvent; +import com.vaadin.server.VaadinSession; +import com.vaadin.shared.ui.label.ContentMode; +import com.vaadin.tests.components.TestBase; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.Label; +import com.vaadin.ui.ProgressIndicator; +import com.vaadin.ui.TextField; + +public class ComboPushTiming extends TestBase { + + private int counter = 0; + private final MyExecutor executor = new MyExecutor(); + + @Override + protected void setup() { + + List list = new ArrayList(); + for (int i = 0; i < 100; i++) { + list.add("Item " + i); + } + + final ComboBox cb = new ComboBox("Combobox", list); + cb.setImmediate(true); + cb.setInputPrompt("Enter text"); + cb.setDescription("Some Combobox"); + addComponent(cb); + + final ObjectProperty log = new ObjectProperty(""); + + cb.addListener(new FieldEvents.FocusListener() { + @Override + public void focus(FocusEvent event) { + log.setValue(log.getValue().toString() + "
" + counter + + ": Focus event!"); + counter++; + changeValue(cb); + } + }); + + cb.addListener(new FieldEvents.BlurListener() { + @Override + public void blur(BlurEvent event) { + log.setValue(log.getValue().toString() + "
" + counter + + ": Blur event!"); + counter++; + } + }); + + TextField field = new TextField("Some textfield"); + field.setImmediate(true); + addComponent(field); + + Label output = new Label(log); + output.setCaption("Events:"); + + output.setContentMode(ContentMode.HTML); + addComponent(output); + + ProgressIndicator progressIndicator = new ProgressIndicator(); + addComponent(progressIndicator); + progressIndicator.setPollingInterval(3000); + } + + private void changeValue(final ComboBox cb) { + executor.execute(new Runnable() { + public void run() { + VaadinSession.getCurrent().lock(); + try { + cb.setEnabled(true); + cb.setValue("B"); + cb.setEnabled(true); + + // If this isn't sent by push or poll in the background, the + // problem will go away + } finally { + VaadinSession.getCurrent().unlock(); + } + } + }); + } + + class MyExecutor extends ThreadPoolExecutor { + public MyExecutor() { + super(5, 20, 20, TimeUnit.SECONDS, new SynchronousQueue()); + } + } + + @Override + protected String getDescription() { + return "When an update is received while the popup is open, the suggestion popup blurs away"; + } + + @Override + protected Integer getTicketNumber() { + return 10924; + } + +} -- cgit v1.2.3