Test case and fix for #3141 - Multiple problems with tab scrolling

svn changeset:8864/svn branch:6.1

3 files changed, 344 insertions, 18 deletions

diff --git a/src/com/vaadin/terminal/gwt/client/ui/VTabsheet.java b/src/com/vaadin/terminal/gwt/client/ui/VTabsheet.java
index 9d3a16ebfa..f73a2d4fc1 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VTabsheet.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VTabsheet.java
@@ -34,6 +34,9 @@ import com.vaadin.terminal.gwt.client.VCaption;
 public class VTabsheet extends VTabsheetBase {
 
     private class TabSheetCaption extends VCaption {
+
+        private boolean hidden = false;
+
         TabSheetCaption() {
             super(null, client);
         }
@@ -97,6 +100,14 @@ public class VTabsheet extends VTabsheetBase {
             captionText.getStyle().setPropertyPx("width", captionWidth);
         }
 
+        public boolean isHidden() {
+            return hidden;
+        }
+
+        public void setHidden(boolean hidden) {
+            this.hidden = hidden;
+        }
+
     }
 
     class TabBar extends ComplexPanel implements ClickHandler {
@@ -232,6 +243,10 @@ public class VTabsheet extends VTabsheetBase {
     private final Element scroller; // tab-scroller element
     private final Element scrollerNext; // tab-scroller next button element
     private final Element scrollerPrev; // tab-scroller prev button element
+
+    /**
+     * The index of the first visible tab (when scrolled)
+     */
     private int scrollerIndex = 0;
 
     private final TabBar tb = new TabBar();
@@ -346,22 +361,20 @@ public class VTabsheet extends VTabsheetBase {
 
         // Tab scrolling
         if (isScrolledTabs() && DOM.eventGetTarget(event) == scrollerPrev) {
-            if (scrollerIndex > 0) {
-                scrollerIndex--;
-                DOM.setStyleAttribute(DOM.getChild(DOM.getFirstChild(DOM
-                        .getFirstChild(tb.getElement())), scrollerIndex),
-                        "display", "");
-                tb.updateCaptionSize(scrollerIndex);
+            int prevVisible = getPreviousVisibleTab(scrollerIndex);
+            if (prevVisible != -1) {
+                tb.setVisible(prevVisible, true);
+                tb.updateCaptionSize(prevVisible);
+                scrollerIndex = prevVisible;
                 updateTabScroller();
             }
         } else if (isClippedTabs() && DOM.eventGetTarget(event) == scrollerNext) {
-            int tabs = tb.getTabCount();
-            if (scrollerIndex + 1 <= tabs) {
-                DOM.setStyleAttribute(DOM.getChild(DOM.getFirstChild(DOM
-                        .getFirstChild(tb.getElement())), scrollerIndex),
-                        "display", "none");
-                tb.updateCaptionSize(scrollerIndex);
-                scrollerIndex++;
+            int firstVisible = scrollerIndex;
+            int nextVisible = getNextVisibleTab(firstVisible);
+            if (nextVisible != -1) {
+                tb.setVisible(firstVisible, false);
+                tb.updateCaptionSize(firstVisible);
+                scrollerIndex = nextVisible;
                 updateTabScroller();
             }
         } else {
@@ -369,6 +382,51 @@ public class VTabsheet extends VTabsheetBase {
         }
     }
 
+    /**
+     * Find the next visible tab. Returns -1 if none is found.
+     * 
+     * @param i
+     * @return
+     */
+    private int getNextVisibleTab(int i) {
+        int tabs = tb.getTabCount();
+        do {
+            i++;
+        } while (i < tabs && tb.getTab(i).isHidden());
+
+        if (i == tabs) {
+            return -1;
+        } else {
+            return i;
+        }
+    }
+
+    /**
+     * Find the previous visible tab. Returns -1 if none is found.
+     * 
+     * @param i
+     * @return
+     */
+    private int getPreviousVisibleTab(int i) {
+        do {
+            i--;
+        } while (i >= 0 && tb.getTab(i).isHidden());
+
+        return i;
+
+    }
+
+    /**
+     * Checks if the tab with the selected index has been scrolled out of the
+     * view (on the left side).
+     * 
+     * @param index
+     * @return
+     */
+    private boolean scrolledOutOfView(int index) {
+        return scrollerIndex > index;
+    }
+
     @Override
     public void updateFromUIDL(UIDL uidl, ApplicationConnection client) {
         rendering = true;
@@ -513,6 +571,12 @@ public class VTabsheet extends VTabsheetBase {
         }
         c.updateCaption(tabUidl);
 
+        c.setHidden(hidden);
+        if (scrolledOutOfView(index)) {
+            // Should not set tabs visible if they are scrolled out of view
+            hidden = true;
+        }
+        // Set the current visibility of the tab (in the browser)
         tb.setVisible(index, !hidden);
 
         /*
@@ -736,9 +800,14 @@ public class VTabsheet extends VTabsheetBase {
         if (width != null) {
             DOM.setStyleAttribute(tabs, "width", width);
         }
+
+        // Make sure scrollerIndex is valid
         if (scrollerIndex > tb.getTabCount()) {
-            scrollerIndex = 0;
+            scrollerIndex = getNextVisibleTab(-1);
+        } else if (tb.getTab(scrollerIndex).isHidden()) {
+            scrollerIndex = getNextVisibleTab(scrollerIndex);
         }
+
         boolean scrolled = isScrolledTabs();
         boolean clipped = isClippedTabs();
         if (tb.isVisible() && (scrolled || clipped)) {
@@ -774,15 +843,16 @@ public class VTabsheet extends VTabsheetBase {
     }
 
     private void showAllTabs() {
-        scrollerIndex = 0;
-        Element tr = DOM.getFirstChild(DOM.getFirstChild(tb.getElement()));
+        scrollerIndex = getNextVisibleTab(-1);
         for (int i = 0; i < tb.getTabCount(); i++) {
-            DOM.setStyleAttribute(DOM.getChild(tr, i), "display", "");
+            if (!tb.getTab(i).isHidden()) {
+                tb.setVisible(i, true);
+            }
         }
     }
 
     private boolean isScrolledTabs() {
-        return scrollerIndex > 0;
+        return scrollerIndex > getNextVisibleTab(-1);
     }
 
     private boolean isClippedTabs() {
diff --git a/src/com/vaadin/tests/components/tabsheet/TabsheetScrolling.html b/src/com/vaadin/tests/components/tabsheet/TabsheetScrolling.html
new file mode 100644
index 0000000000..16657e27c4
--- /dev/null
+++ b/src/com/vaadin/tests/components/tabsheet/TabsheetScrolling.html
@@ -0,0 +1,182 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="" />
+<title>TabsheetScrolling</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">TabsheetScrolling</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.components.tabsheet.TabsheetScrolling?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[1]/domChild[1]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[1]/domChild[1]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[1]/domChild[1]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[1]/domChild[1]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[1]/domChild[1]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTabsheet[0]/VTabsheetPanel[0]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTabsheet[0]/VTabsheetPanel[0]/VButton[1]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/VTabsheetPanel[0]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/VTabsheetPanel[0]/VButton[1]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/VTabsheetPanel[0]/VButton[2]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/VTabsheetPanel[0]/VButton[3]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/VTabsheetPanel[0]/VButton[4]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/VTabsheetPanel[0]/VButton[5]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstabsheetTabsheetScrolling::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/VTabsheetPanel[0]/VButton[6]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td></td>
+</tr>
+
+</tbody></table>
+</body>
+</html>
diff --git a/src/com/vaadin/tests/components/tabsheet/TabsheetScrolling.java b/src/com/vaadin/tests/components/tabsheet/TabsheetScrolling.java
new file mode 100644
index 0000000000..718a10aadd
--- /dev/null
+++ b/src/com/vaadin/tests/components/tabsheet/TabsheetScrolling.java
@@ -0,0 +1,74 @@
+package com.vaadin.tests.components.tabsheet;

+

+import com.vaadin.tests.components.TestBase;

+import com.vaadin.ui.Button;

+import com.vaadin.ui.TabSheet;

+import com.vaadin.ui.Button.ClickEvent;

+import com.vaadin.ui.Button.ClickListener;

+import com.vaadin.ui.TabSheet.Tab;

+

+public class TabsheetScrolling extends TestBase {

+

+    private TabSheet fixedSizeTabSheet;

+    private TabSheet autoWideTabSheet;

+

+    @Override

+    protected void setup() {

+        fixedSizeTabSheet = new TabSheet();

+        fixedSizeTabSheet.setHeight("200px");

+        fixedSizeTabSheet.setWidth("400px");

+

+        for (int i = 0; i < 100; i++) {

+            Button b = new Button("Hide this tab (" + i + ")",

+                    new ClickListener() {

+

+                        public void buttonClick(ClickEvent event) {

+                            fixedSizeTabSheet.getTab(event.getButton())

+                                    .setVisible(false);

+                        }

+

+                    });

+            Tab t = fixedSizeTabSheet.addTab(b, "Tab " + i, null);

+            if (i % 2 == 0) {

+                t.setVisible(false);

+            }

+        }

+

+        addComponent(fixedSizeTabSheet);

+

+        autoWideTabSheet = new TabSheet();

+        autoWideTabSheet.setHeight("200px");

+        autoWideTabSheet.setWidth(null);

+

+        for (int i = 0; i < 10; i++) {

+            Button b = new Button("Hide this tab (" + i + ")",

+                    new ClickListener() {

+

+                        public void buttonClick(ClickEvent event) {

+                            autoWideTabSheet.getTab(event.getButton())

+                                    .setVisible(false);

+                        }

+                    });

+

+            Tab t = autoWideTabSheet.addTab(b, "Tab " + i, null);

+            if (i % 2 == 0) {

+                t.setVisible(false);

+

+            }

+        }

+

+        addComponent(autoWideTabSheet);

+

+    }

+

+    @Override

+    protected String getDescription() {

+        return "Two tabsheets, upper has fixed width, lower has dynamic width. Every other tab in both tabsheets are hidden (even numbered tabs). Scrolling the upper tab sheet should never display a hidden tab. Hiding a tab in the upper tabsheet should not affect scrolling. Hiding a tab in the lower tabsheet should make the tabsheet width change (auto wide).";

+    }

+

+    @Override

+    protected Integer getTicketNumber() {

+        return 3141;

+    }

+

+}