Re-showed tab is displayed if there is room for it (#17096)

Change-Id: I52c9a47e2b9eed3a3f73f34e4c3e770b915509bf

3 files changed, 244 insertions, 9 deletions

diff --git a/client/src/com/vaadin/client/ui/VTabsheet.java b/client/src/com/vaadin/client/ui/VTabsheet.java
index c8984ece51..a02679a0fc 100644
--- a/client/src/com/vaadin/client/ui/VTabsheet.java
+++ b/client/src/com/vaadin/client/ui/VTabsheet.java
@@ -628,9 +628,7 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
         }
 
         /**
-         * Returns the index of the first visible tab
-         *
-         * @return
+         * Returns the index of the first visible tab on the server
          */
         private int getFirstVisibleTab() {
             return getNextVisibleTab(-1);
@@ -656,6 +654,23 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
         }
 
         /**
+         * Returns the index of the first visible tab in browser.
+         */
+        private int getFirstVisibleTabClient() {
+            int tabs = getTabCount();
+            int i = 0;
+            while (i < tabs && !getTab(i).isVisible()) {
+                i++;
+            }
+
+            if (i == tabs) {
+                return -1;
+            } else {
+                return i;
+            }
+        }
+
+        /**
          * Find the previous visible tab. Returns -1 if none is found.
          *
          * @param i
@@ -1086,8 +1101,13 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
                 PREV_SCROLLER_DISABLED_CLASSNAME);
     }
 
+    private boolean isScrollerHidden() {
+        return scroller.getStyle().getDisplay().equals(Display.NONE.getCssName());
+    }
+
     private boolean isIndexSkippingHiddenTabs() {
-        return isAllTabsBeforeIndexInvisible() && isScrollerPrevDisabled();
+        return isAllTabsBeforeIndexInvisible()
+                && (isScrollerPrevDisabled() || isScrollerHidden());
     }
 
     @Override
@@ -1105,12 +1125,21 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
             // Should not set tabs visible if they are scrolled out of view
             tab.setVisible(false);
         } else {
-            // reset the scroller index back to zero if tab is visible
-            // again and tab is in view
-            if (isIndexSkippingHiddenTabs() && tabState.visible) {
-                scrollerIndex = 0;
+            //When the tab was hidden and then turned visible again
+            //and there is space for it, it should be in view (#17096) (#17333)
+            if (isTabSetVisibleBeforeScroller(tabState, index, tab)) {
+                scrollerIndex = index;
+                tab.setVisible(true);
+                tab.setStyleNames(false, true);
+
+                //scroll to the currently selected tab if it got clipped
+                //after making another tab visible
+                if(isClippedTabs()) {
+                    scrollIntoView(getActiveTab());
+                }
+            } else {
+                tab.setVisible(tabState.visible);
             }
-            tab.setVisible(tabState.visible);
         }
 
         /*
@@ -1121,6 +1150,26 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
     }
 
     /**
+     * Checks whether the tab has been set to visible and the scroller is at the first visible tab.
+     * That means that the scroller has to be adjusted so that the tab is visible again.
+     */
+    private boolean isTabSetVisibleBeforeScroller(TabState tabState, int index, Tab tab) {
+        return isIndexSkippingHiddenTabs() && isScrollerAtFirstVisibleTab()
+                && hasTabChangedVisibility(tabState, tab) && scrolledOutOfView(index);
+    }
+
+    /**
+     * Checks whether the tab is visible on server but is not visible on client yet.
+     */
+    private boolean hasTabChangedVisibility(TabState tabState, Tab tab) {
+        return !tab.isVisible() && tabState.visible;
+    }
+
+    private boolean isScrollerAtFirstVisibleTab() {
+        return tb.getFirstVisibleTabClient() == scrollerIndex;
+    }
+
+    /**
      * @deprecated as of 7.1, VTabsheet only keeps the active tab in the DOM
      *             without any place holders.
      */
diff --git a/uitest/src/com/vaadin/tests/components/tabsheet/FirstTabNotVisibleWhenTabsheetNotClipped.java b/uitest/src/com/vaadin/tests/components/tabsheet/FirstTabNotVisibleWhenTabsheetNotClipped.java
new file mode 100644
index 0000000000..6d66f1d295
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tabsheet/FirstTabNotVisibleWhenTabsheetNotClipped.java
@@ -0,0 +1,130 @@
+package com.vaadin.tests.components.tabsheet;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.*;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class FirstTabNotVisibleWhenTabsheetNotClipped extends AbstractTestUI {
+
+    private TabSheet.Tab firstNotClippedTab;
+    private TabSheet.Tab firstClippedTab;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        addButton("Toggle first not clipped tab", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                firstNotClippedTab.setVisible(!firstNotClippedTab.isVisible());
+            }
+        });
+        addComponent(createNotClippedTabSheet());
+
+        addButton("Toggle first clipped tab", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                firstClippedTab.setVisible(!firstClippedTab.isVisible());
+            }
+        });
+        addComponent(createClippedTabSheet());
+
+        addComponent(new Label("VerticalLayout:"));
+        addBlock(new VerticalLayout());
+        addComponent(new Label("HorizontalLayout:"));
+        addBlock(new HorizontalLayout());
+    }
+
+    private TabSheet createNotClippedTabSheet() {
+        TabSheet notClippedTabSheet = new TabSheet();
+        for (int i = 0; i < 2; i++) {
+            notClippedTabSheet.addTab(createTabContent(i), "Tab " + i);
+        }
+        firstNotClippedTab = notClippedTabSheet.getTab(0);
+        return notClippedTabSheet;
+    }
+
+    private TabSheet createClippedTabSheet() {
+        TabSheet clippedTabSheet = new TabSheet();
+        for (int i = 0; i < 50; i++) {
+            clippedTabSheet.addTab(createTabContent(i), "Tab " + i);
+        }
+        firstClippedTab = clippedTabSheet.getTab(0);
+        return clippedTabSheet;
+    }
+
+    private VerticalLayout createTabContent(int index) {
+        VerticalLayout layout = new VerticalLayout();
+        layout.addComponent(new Label("Tab " + index + " Content"));
+        return layout;
+    }
+
+    private void addBlock(Layout layout) {
+        layout.setWidth("300px");
+
+        TabSheet tabsheet = new TabSheet();
+        String[] letters = { "A", "B", "C", "D" };
+        HashMap<String, TabSheet.Tab> tabMap = new HashMap<String, TabSheet.Tab>();
+
+        for (String letter : letters) {
+            VerticalLayout vLayout = new VerticalLayout();
+            vLayout.addComponent(new Label(letter + 1));
+            vLayout.addComponent(new Label(letter + 2));
+            vLayout.addComponent(new Label(letter + 3));
+
+            tabsheet.addTab(vLayout);
+            tabsheet.getTab(vLayout).setCaption("tab " + letter);
+
+            tabMap.put("tab " + letter, tabsheet.getTab(vLayout));
+        }
+
+        VerticalLayout vtabLayout = new VerticalLayout();
+
+        for (String letter : letters) {
+            Button btntab = new Button("show tab " + letter);
+            btntab.setId("tab " + letter);
+            btntab.addClickListener(createTabListener(tabMap, tabsheet));
+            vtabLayout.addComponent(btntab);
+        }
+
+        layout.addComponent(vtabLayout);
+        layout.addComponent(tabsheet);
+        addComponent(layout);
+    }
+
+    private Button.ClickListener createTabListener(final HashMap<String, TabSheet.Tab> map,
+                                            final TabSheet tabsheet) {
+
+        Button.ClickListener clickListener = new Button.ClickListener() {
+
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                // id of the button is the same as the tab's caption
+                String tabName = event.getComponent().getId();
+
+                for (Map.Entry<String, TabSheet.Tab> entry : map.entrySet()) {
+                    TabSheet.Tab tab = entry.getValue();
+
+                    if (entry.getKey().equals(tabName)) {
+                        tab.setVisible(true);
+                        tabsheet.setSelectedTab(tab.getComponent());
+                    } else {
+                        tab.setVisible(false);
+                    }
+                }
+            }
+        };
+        return clickListener;
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 17096;
+    }
+
+    @Override
+    public String getDescription() {
+        return "TabSheet should display re-shown tab if there's room for it";
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/tabsheet/FirstTabNotVisibleWhenTabsheetNotClippedTest.java b/uitest/src/com/vaadin/tests/components/tabsheet/FirstTabNotVisibleWhenTabsheetNotClippedTest.java
new file mode 100644
index 0000000000..74a725f5ed
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tabsheet/FirstTabNotVisibleWhenTabsheetNotClippedTest.java
@@ -0,0 +1,56 @@
+package com.vaadin.tests.components.tabsheet;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TabSheetElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.WebElement;
+
+public class FirstTabNotVisibleWhenTabsheetNotClippedTest extends MultiBrowserTest {
+    @Test
+    public void testNotClippedTabIsVisible() throws InterruptedException {
+        openTestURL();
+
+        ButtonElement toggleNotClipped = $(ButtonElement.class)
+                .caption("Toggle first not clipped tab").first();
+
+        toggleNotClipped.click();
+        TabSheetElement notClippedTabSheet = $(TabSheetElement.class).get(0);
+        WebElement firstTab = notClippedTabSheet.findElement(
+                By.className("v-tabsheet-tabitemcell-first"));
+        String caption = firstTab.findElement(By.className("v-captiontext")).getText();
+        Assert.assertEquals("Tab with -first style should be Tab 1", "Tab 1", caption);
+
+        toggleNotClipped.click();
+        firstTab = notClippedTabSheet.findElement(
+                By.className("v-tabsheet-tabitemcell-first"));
+        caption = firstTab.findElement(By.className("v-captiontext")).getText();
+        Assert.assertEquals("Tab with -first style should be Tab 0", "Tab 0", caption);
+    }
+
+
+    @Test
+    public void testShowPreviouslyHiddenTab() {
+        openTestURL();
+
+        $(ButtonElement.class).caption("show tab D").get(0).click();
+        $(ButtonElement.class).caption("show tab C").get(0).click();
+
+        WebElement firstTab = $(TabSheetElement.class).get(2)
+                .findElement(By.className("v-tabsheet-tabitemcell-first"));
+        String firstCaption = firstTab.findElement(By.className("v-captiontext")).getText();
+
+        org.junit.Assert.assertEquals("tab C", firstCaption);
+
+        $(ButtonElement.class).caption("show tab D").get(1).click();
+        $(ButtonElement.class).caption("show tab C").get(1).click();
+
+        WebElement secondTab = $(TabSheetElement.class).get(3)
+                .findElement(By.className("v-tabsheet-tabitemcell-first"));
+        String secondCaption = secondTab.findElement(By.className("v-captiontext")).getText();
+
+        org.junit.Assert.assertEquals("tab C", secondCaption);
+    }
+}