Fix ComboBox popup scrolling when paging disabled (#13488)

Added pagelength == 0 conditions on scroll and hasNextPage.

Change-Id: I42c0eb56d42cc54ff57a6bc6b9eee2f6734315bb

diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java
index e0ced983946e8e991dd1edc5baea2b230db4ac4f..b68c978aee9150e51fe95f8d89a1dfe0c41181fd 100644 (file)
--- a/client/src/com/vaadin/client/ui/VFilterSelect.java
+++ b/client/src/com/vaadin/client/ui/VFilterSelect.java
@@ -453,7 +453,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
             public void scrollUp() {
                 debug("VFS.SP.LPS: scrollUp()");
-                if (currentPage + pagesToScroll > 0) {
+                if (pageLength > 0 && currentPage + pagesToScroll > 0) {
                     pagesToScroll--;
                     cancel();
                     schedule(200);
@@ -462,8 +462,9 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
             public void scrollDown() {
                 debug("VFS.SP.LPS: scrollDown()");
-                if (totalMatches > (currentPage + pagesToScroll + 1)
-                        * pageLength) {
+                if (pageLength > 0
+                        && totalMatches > (currentPage + pagesToScroll + 1)
+                                * pageLength) {
                     pagesToScroll++;
                     cancel();
                     schedule(200);
@@ -1217,7 +1218,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
      *         last page
      */
     public boolean hasNextPage() {
-        if (totalMatches > (currentPage + 1) * pageLength) {
+        if (pageLength > 0 && totalMatches > (currentPage + 1) * pageLength) {
             return true;
         } else {
             return false;

diff --git a/uitest/src/com/vaadin/tests/components/ui/ComboboxPageLengthZeroScroll.java b/uitest/src/com/vaadin/tests/components/ui/ComboboxPageLengthZeroScroll.java
new file mode 100644 (file)
index 0000000..82c0419
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/ui/ComboboxPageLengthZeroScroll.java
@@ -0,0 +1,56 @@
+/*
+ * Copyright 2000-2013 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.tests.components.ui;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.ComboBox;
+
+/**
+ * Test UI for issue #13488, where scrolling to the next page with pagelength 0
+ * would break the rendering of any page except the first.
+ * 
+ * @author Vaadin Ltd
+ */
+@SuppressWarnings("serial")
+public class ComboboxPageLengthZeroScroll extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        ComboBox combobox = new ComboBox("New items enabled:");
+        combobox.setPageLength(0);
+
+        for (int i = 0; i++ < 10;) {
+            combobox.addItem("1 AMERICAN SAMOA " + i);
+            combobox.addItem("ANTIGUA AND BARBUDA " + i);
+        }
+
+        getLayout().addComponent(combobox);
+        getLayout().addComponent(new Button("dummy"));
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Scrolling with pagelength == 0 previously resulted in broken style, should be fixed now";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 13488;
+    }
+
+}

diff --git a/uitest/src/com/vaadin/tests/components/ui/ComboboxPageLengthZeroScrollTest.java b/uitest/src/com/vaadin/tests/components/ui/ComboboxPageLengthZeroScrollTest.java
new file mode 100644 (file)
index 0000000..eecf834
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/ui/ComboboxPageLengthZeroScrollTest.java
@@ -0,0 +1,63 @@
+/*
+ * Copyright 2000-2014 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.tests.components.ui;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.Actions;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Test class for testing issue #13488 - changing pages with pagelength=0 breaks
+ * the style.
+ * 
+ * @author Vaadin Ltd
+ */
+
+public class ComboboxPageLengthZeroScrollTest extends MultiBrowserTest {
+    @Test
+    public void testComboboxPageLength() {
+        openTestURL();
+
+        WebElement comboBox = vaadinElement("/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VFilterSelect[0]#textbox");
+
+        // navigate to the next page. keyboard navigation is the preferred
+        // method here since it's much easier to implement.
+
+        Actions keyNavigation = new Actions(driver).moveToElement(comboBox)
+                .click();
+
+        for (int i = 0; i < 25; ++i) {
+            keyNavigation.sendKeys(Keys.ARROW_DOWN);
+
+        }
+        keyNavigation.perform();
+
+        // The broken behavior always caused a v-shadow element to have
+        // height: 10px. Verify that this does no longer happen.
+
+        String cssValue = driver.findElement(By.className("v-shadow"))
+                .getCssValue("height");
+
+        Assert.assertNotEquals("v-shadow height should not be 10px", "10px",
+                cssValue);
+
+    }
+}