Reset selected icon when blurring a ComboBox (#8082)

* Reset selected icon when blurring a ComboBox

Fixes #8073

* Merge branch '7.7' into _combobox-icon-reset-selection

* Merge branch '7.7' into _combobox-icon-reset-selection

diff --git a/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java b/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java
index 38e067658bd43f317d1e3fb657eec654874e65aa..e407aaafe45d0b6383ebfb9fa206de5fa589c9eb 100644 (file)
--- a/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java
+++ b/client/src/main/java/com/vaadin/client/ui/VFilterSelect.java
@@ -1445,7 +1445,10 @@ public class VFilterSelect extends Composite
     };
 
     private class IconWidget extends Widget {
+        private Icon icon;
+
         IconWidget(Icon icon) {
+            this.icon = icon;
             setElement(icon.getElement());
             addDomHandler(VFilterSelect.this, ClickEvent.getType());
         }
@@ -1903,25 +1906,59 @@ public class VFilterSelect extends Composite
                 afterSelectedItemIconChange();
             }
         } else {
+            IconWidget newIcon = new IconWidget(client.getIcon(iconUri));
+            if (iconEquals(newIcon, selectedItemIcon)) {
+                /*
+                 * Do not update the icon if nothing has changed. Otherwise we
+                 * can cause problems such as not being able to click in the
+                 * icon to open the popup (blur might occur and call this
+                 * method, icon is replaced and the click event is not delivered
+                 * to the new icon)
+                 */
+                return;
+            }
+
             if (selectedItemIcon != null) {
                 panel.remove(selectedItemIcon);
             }
-            selectedItemIcon = new IconWidget(client.getIcon(iconUri));
+
             // Older IE versions don't scale icon correctly if DOM
             // contains height and width attributes.
-            selectedItemIcon.getElement().removeAttribute("height");
-            selectedItemIcon.getElement().removeAttribute("width");
-            selectedItemIcon.addDomHandler(new LoadHandler() {
+            newIcon.getElement().removeAttribute("height");
+            newIcon.getElement().removeAttribute("width");
+            newIcon.addDomHandler(new LoadHandler() {
                 @Override
                 public void onLoad(LoadEvent event) {
                     afterSelectedItemIconChange();
                 }
             }, LoadEvent.getType());
-            panel.insert(selectedItemIcon, 0);
+            panel.insert(newIcon, 0);
+            selectedItemIcon = newIcon;
             afterSelectedItemIconChange();
         }
     }
 
+    /**
+     * Checks if the icon widgets show the same icon.
+     * 
+     * @param icon1
+     *            the first widget
+     * @param icon2
+     *            the second widget
+     * @return <code>true</code> if they show the same icon, <code>false</code>
+     *         otherwise
+     */
+    private static boolean iconEquals(IconWidget icon1, IconWidget icon2) {
+        if (icon1 == null) {
+            return icon2 == null;
+        } else if (icon2 == null) {
+            return false;
+        } else {
+            return icon1.icon.getUri().equals(icon2.icon.getUri());
+        }
+
+    }
+
     private void afterSelectedItemIconChange() {
         if (BrowserInfo.get().isWebkit() || BrowserInfo.get().isIE8()) {
             // Some browsers need a nudge to reposition the text field
@@ -2387,6 +2424,7 @@ public class VFilterSelect extends Composite
                 }
             } else if (currentSuggestion != null) {
                 setPromptingOff(currentSuggestion.caption);
+                setSelectedItemIcon(currentSuggestion.getIconUri());
             }
         }
         removeStyleDependentName("focus");

diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxItemIconTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxItemIconTest.java
index 15c87b65f03e66fb0c490b26c62eca2e9c0c32c2..2b8ffa1583f42dada0a10aa775e2fe319648e9eb 100644 (file)
--- a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxItemIconTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxItemIconTest.java
@@ -1,7 +1,10 @@
 package com.vaadin.tests.components.combobox;
 
+import org.junit.Assert;
 import org.junit.Test;
+import org.openqa.selenium.Keys;
 
+import com.vaadin.testbench.By;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 import com.vaadin.tests.tb3.newelements.ComboBoxElement;
 
@@ -29,4 +32,41 @@ public class ComboBoxItemIconTest extends MultiBrowserTest {
         compareScreen("fi-au-selected");
     }
 
-}
\ No newline at end of file
+    @Test
+    public void iconResetOnSelectionCancelByEscape() {
+        openTestURL();
+        ComboBoxElement cb = $(ComboBoxElement.class).get(1);
+
+        assertSelection(cb, "hu.gif", "Hungary");
+        cb.openPopup();
+        cb.sendKeys(Keys.UP);
+        assertSelection(cb, "au.gif", "Australia");
+        cb.sendKeys(Keys.ESCAPE);
+        assertSelection(cb, "hu.gif", "Hungary");
+    }
+
+    @Test
+    public void iconResetOnSelectionCancelByClickingOutside() {
+        openTestURL();
+        ComboBoxElement cb = $(ComboBoxElement.class).get(1);
+
+        assertSelection(cb, "hu.gif", "Hungary");
+        cb.openPopup();
+        cb.sendKeys(Keys.UP);
+        assertSelection(cb, "au.gif", "Australia");
+        findElement(By.tagName("body")).click();
+        assertSelection(cb, "hu.gif", "Hungary");
+
+    }
+
+    private void assertSelection(ComboBoxElement cb, String imageSuffix,
+            String caption) {
+        Assert.assertEquals(caption, cb.getValue());
+        String imgSrc = cb.findElement(By.className("v-icon"))
+                .getAttribute("src");
+        imgSrc = imgSrc.substring(imgSrc.lastIndexOf('/') + 1);
+        Assert.assertEquals(imageSuffix, imgSrc);
+
+    }
+
+}