MenuBar submenus close unexpectedly if use Valo theme (#15255)

This fix is for animation-in and animation-out.

Fix was done in VMenuBar. VOverlay provides now hide() method with
possibility to enable/disable the animations via parameters boolean
animateIn, boolean animateOut. By default they are true (not to break
animate-in, animate-out for VWindow and VNotification)

Change-Id: I49981952c7c18a02edd7fa9e6d247bb18f660207

5 files changed, 212 insertions, 43 deletions

diff --git a/WebContent/VAADIN/themes/valo/components/_menubar.scss b/WebContent/VAADIN/themes/valo/components/_menubar.scss
index 9075eb7ba8..0f9c61d2ce 100644
--- a/WebContent/VAADIN/themes/valo/components/_menubar.scss
+++ b/WebContent/VAADIN/themes/valo/components/_menubar.scss
@@ -70,10 +70,6 @@
 
   .#{$primary-stylename}-popup {
     @include valo-menubar-popup-style($primary-stylename);
-
-    &.#{$primary-stylename}-popup-animate-out {
-      @include animation(none);
-    }
   }
 
 
diff --git a/client/src/com/vaadin/client/ui/VMenuBar.java b/client/src/com/vaadin/client/ui/VMenuBar.java
index 5102e6faea..66160e691d 100644
--- a/client/src/com/vaadin/client/ui/VMenuBar.java
+++ b/client/src/com/vaadin/client/ui/VMenuBar.java
@@ -476,7 +476,8 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
         if (menuVisible && visibleChildMenu != item.getSubMenu()
                 && popup != null) {
-            popup.hide();
+            // #15255 - disable animation-in/out when hide in this case
+            popup.hide(false, false, false);
         }
 
         if (menuVisible && item.getSubMenu() != null
@@ -707,9 +708,22 @@ public class VMenuBar extends SimpleFocusablePanel implements
      * Recursively hide all child menus
      */
     public void hideChildren() {
+        hideChildren(true, true);
+    }
+
+    /**
+     * 
+     * Recursively hide all child menus
+     * 
+     * @param animateIn
+     *            enable/disable animate-in animation when hide popup
+     * @param animateOut
+     *            enable/disable animate-out animation when hide popup
+     */
+    public void hideChildren(boolean animateIn, boolean animateOut) {
         if (visibleChildMenu != null) {
-            visibleChildMenu.hideChildren();
-            popup.hide();
+            visibleChildMenu.hideChildren(animateIn, animateOut);
+            popup.hide(false, animateIn, animateOut);
         }
     }
 
@@ -1326,7 +1340,8 @@ public class VMenuBar extends SimpleFocusablePanel implements
                 VMenuBar root = getParentMenu();
 
                 root.getSelected().getSubMenu().setSelected(null);
-                root.hideChildren();
+                // #15255 - disable animate-in/out when hide popup
+                root.hideChildren(false, false);
 
                 // Get the root menus items and select the previous one
                 int idx = root.getItems().indexOf(root.getSelected());
@@ -1383,8 +1398,9 @@ public class VMenuBar extends SimpleFocusablePanel implements
                     root = root.getParentMenu();
                 }
 
-                // Hide the submenu
-                root.hideChildren();
+                // Hide the submenu (#15255 - disable animate-in/out when hide
+                // popup)
+                root.hideChildren(false, false);
 
                 // Get the root menus items and select the next one
                 int idx = root.getItems().indexOf(root.getSelected());
diff --git a/client/src/com/vaadin/client/ui/VOverlay.java b/client/src/com/vaadin/client/ui/VOverlay.java
index afa13dc337..a620fa6268 100644
--- a/client/src/com/vaadin/client/ui/VOverlay.java
+++ b/client/src/com/vaadin/client/ui/VOverlay.java
@@ -51,7 +51,7 @@ import com.vaadin.client.Util;
  * temporary float over other components like context menus etc. This is to deal
  * stacking order correctly with VWindow objects.
  * </p>
- *
+ * 
  * <h3>Shadow</h3>
  * <p>
  * The separate shadow element underneath the main overlay element is <strong>
@@ -62,7 +62,7 @@ import com.vaadin.client.Util;
  * supports, add <code>-webkit-box-shadow</code> and the standard
  * <code>box-shadow</code> properties.
  * </p>
- *
+ * 
  * <p>
  * For IE8, which doesn't support CSS box-shadow, you can use the proprietary
  * DropShadow filter. It doesn't provide the exact same features as box-shadow,
@@ -70,7 +70,7 @@ import com.vaadin.client.Util;
  * border or a pseudo-element underneath the overlay which mimics a shadow, or
  * any combination of these.
  * </p>
- *
+ * 
  * <p>
  * Read more about the DropShadow filter from <a
  * href="http://msdn.microsoft.com/en-us/library/ms532985(v=vs.85).aspx"
@@ -152,7 +152,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * Shadow element style. If an extending class wishes to use a different
      * style of shadow, it can use setShadowStyle(String) to give the shadow
      * element a new style name.
-     *
+     * 
      * @deprecated See main JavaDoc for VOverlay
      */
     @Deprecated
@@ -175,9 +175,9 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
 
     /**
      * The shadow element for this overlay.
-     *
+     * 
      * @deprecated See main JavaDoc for VOverlay
-     *
+     * 
      */
     @Deprecated
     private Element shadow;
@@ -206,7 +206,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * The HTML snippet that is used to render the actual shadow. In consists of
      * nine different DIV-elements with the following class names:
-     *
+     * 
      * <pre>
      *   .v-shadow[-stylename]
      *   ----------------------------------------------
@@ -219,9 +219,9 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      *   | .bottom-left  |  .bottom  |  .bottom-right |
      *   ----------------------------------------------
      * </pre>
-     *
+     * 
      * See default theme 'shadow.css' for implementation example.
-     *
+     * 
      * @deprecated See main JavaDoc for VOverlay
      */
     @Deprecated
@@ -268,7 +268,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * Return true if a separate shadow div should be used. Since Vaadin 7.3,
      * shadows are implemented with CSS box-shadow. Thus, a shadow div is only
      * used for IE8 by default.
-     *
+     * 
      * @deprecated See main JavaDoc for VOverlay
      * @since 7.3
      * @return true to use a shadow div
@@ -282,10 +282,10 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * Method to control whether DOM elements for shadow are added. With this
      * method subclasses can control displaying of shadow also after the
      * constructor.
-     *
+     * 
      * @param enabled
      *            true if shadow should be displayed
-     *
+     * 
      * @deprecated See main JavaDoc for VOverlay
      */
     @Deprecated
@@ -349,7 +349,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
 
     /**
      * Set the z-index (visual stack position) for this overlay.
-     *
+     * 
      * @param zIndex
      *            The new z-index
      */
@@ -562,12 +562,12 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * Sets the shadow style for this overlay. Will override any previous style
      * for the shadow. The default style name is defined by CLASSNAME_SHADOW.
      * The given style will be prefixed with CLASSNAME_SHADOW.
-     *
+     * 
      * @param style
      *            The new style name for the shadow element. Will be prefixed by
      *            CLASSNAME_SHADOW, e.g. style=='foobar' -> actual style
      *            name=='v-shadow-foobar'.
-     *
+     * 
      * @deprecated See main JavaDoc for VOverlay
      */
     @Deprecated
@@ -581,7 +581,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * Extending classes should always call this method after they change the
      * size of overlay without using normal 'setWidth(String)' and
      * 'setHeight(String)' methods (if not calling super.setWidth/Height).
-     *
+     * 
      */
     public void positionOrSizeUpdated() {
         positionOrSizeUpdated(1.0);
@@ -600,7 +600,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * elements. Can be used to animate the related elements, using the
      * 'progress' parameter (used to animate the shadow in sync with GWT
      * PopupPanel's default animation 'PopupPanel.AnimationType.CENTER').
-     *
+     * 
      * @param progress
      *            A value between 0.0 and 1.0, indicating the progress of the
      *            animation (0=start, 1=end).
@@ -709,7 +709,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * Returns true if we should add a shim iframe below the overlay to deal
      * with zindex issues with PDFs and applets. Can be overriden to disable
      * shim iframes if they are not needed.
-     *
+     * 
      * @return true if a shim iframe should be added, false otherwise
      */
     protected boolean needsShimElement() {
@@ -771,13 +771,13 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Enables or disables sinking the events of the shadow to the same
      * onBrowserEvent as events to the actual overlay goes.
-     *
+     * 
      * Please note, that if you enable this, you can't assume that e.g.
      * event.getEventTarget returns an element inside the DOM structure of the
      * overlay
-     *
+     * 
      * @param sinkShadowEvents
-     *
+     * 
      * @deprecated See main JavaDoc for VOverlay
      */
     @Deprecated
@@ -801,7 +801,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Get owner (Widget that made this VOverlay, not the layout parent) of
      * VOverlay
-     *
+     * 
      * @return Owner (creator) or null if not defined
      */
     public Widget getOwner() {
@@ -811,7 +811,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Set owner (Widget that made this VOverlay, not the layout parent) of
      * VOverlay
-     *
+     * 
      * @param owner
      *            Owner (creator) of VOverlay
      */
@@ -822,7 +822,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Get the {@link ApplicationConnection} that this overlay belongs to. If
      * it's not set, {@link #getOwner()} is used to figure it out.
-     *
+     * 
      * @return
      */
     protected ApplicationConnection getApplicationConnection() {
@@ -842,7 +842,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Gets the 'overlay container' element. Tries to find the current
      * {@link ApplicationConnection} using {@link #getApplicationConnection()}.
-     *
+     * 
      * @return the overlay container element for the current
      *         {@link ApplicationConnection} or another element if the current
      *         {@link ApplicationConnection} cannot be determined.
@@ -866,7 +866,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      * {@link ApplicationConnection}. Each overlay should be created in a
      * overlay container element, so that the correct theme and styles can be
      * applied.
-     *
+     * 
      * @param ac
      *            A reference to {@link ApplicationConnection}
      * @return The overlay container
@@ -893,7 +893,7 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Set the label of the container element, where tooltip, notification and
      * dialgs are added to.
-     *
+     * 
      * @param applicationConnection
      *            the application connection for which to change the label
      * @param overlayContainerLabel
@@ -926,10 +926,10 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Gets the visual viewport width, which is useful for e.g iOS where the
      * view can be zoomed in while keeping the layout viewport intact.
-     *
+     * 
      * Falls back to layout viewport; for those browsers/devices the difference
      * is that the scrollbar with is included (if there is a scrollbar).
-     *
+     * 
      * @since 7.0.7
      * @return
      */
@@ -945,10 +945,10 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
     /**
      * Gets the visual viewport height, which is useful for e.g iOS where the
      * view can be zoomed in while keeping the layout viewport intact.
-     *
+     * 
      * Falls back to layout viewport; for those browsers/devices the difference
      * is that the scrollbar with is included (if there is a scrollbar).
-     *
+     * 
      * @since 7.0.7
      * @return
      */
@@ -988,10 +988,33 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
      */
     @Override
     public void hide(final boolean autoClosed) {
+        hide(autoClosed, true, true);
+    }
+
+    /**
+     * 
+     * Hides the popup and detaches it from the page. This has no effect if it
+     * is not currently showing. Animation-in, animation-out can be
+     * enable/disabled for different use cases.
+     * 
+     * @see com.google.gwt.user.client.ui.PopupPanel#hide(boolean)
+     * 
+     * @param autoClosed
+     *            the value that will be passed to
+     *            {@link CloseHandler#onClose(CloseEvent)} when the popup is
+     *            closed
+     * @param animateIn
+     *            enable/disable animate-in animation
+     * @param animateOut
+     *            enable/disable animate-out animation
+     */
+    public void hide(final boolean autoClosed, final boolean animateIn,
+            final boolean animateOut) {
         if (BrowserInfo.get().isIE8() || BrowserInfo.get().isIE9()) {
             super.hide(autoClosed);
         } else {
-            if (getStyleName().contains(ADDITIONAL_CLASSNAME_ANIMATE_IN)) {
+            if (animateIn
+                    && getStyleName().contains(ADDITIONAL_CLASSNAME_ANIMATE_IN)) {
                 AnimationUtil.addAnimationEndListener(getElement(),
                         new AnimationEndListener() {
                             @Override
@@ -1017,7 +1040,9 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
                     animationName = "";
                 }
 
-                if (animationName.contains(ADDITIONAL_CLASSNAME_ANIMATE_OUT)) {
+                if (animateOut
+                        && animationName
+                                .contains(ADDITIONAL_CLASSNAME_ANIMATE_OUT)) {
                     // Disable GWT PopupPanel closing animation if used
                     setAnimationEnabled(false);
 
diff --git a/uitest/src/com/vaadin/tests/components/menubar/MenuBarSubmenusClosingValo.java b/uitest/src/com/vaadin/tests/components/menubar/MenuBarSubmenusClosingValo.java
new file mode 100644
index 0000000000..88d89abbf4
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/menubar/MenuBarSubmenusClosingValo.java
@@ -0,0 +1,60 @@
+package com.vaadin.tests.components.menubar;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.MenuBar;
+import com.vaadin.ui.MenuBar.MenuItem;
+
+@Theme("valo")
+public class MenuBarSubmenusClosingValo extends AbstractTestUI {
+
+    private MenuItem edit;
+    private MenuItem file;
+    private MenuItem help;
+
+    @Override
+    protected String getTestDescription() {
+        return "Tests that when moving mouse fast over menu items "
+                + "previous submenu popup closes before new submenu popup opens";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 15255;
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        // here we increase animation time to 1 second for to do auto testing
+        // possible
+        getPage().getStyles().add(
+                ".valo .v-menubar-popup[class*=\"animate-in\"] {"
+                        + "-webkit-animation: valo-overlay-animate-in 1000ms; "
+                        + "-moz-animation: valo-overlay-animate-in 1000ms; "
+                        + "animation: valo-overlay-animate-in 1000ms;};");
+
+        getPage().getStyles().add(
+                ".valo .v-menubar-popup[class*=\"animate-out\"] {"
+                        + "-webkit-animation: valo-animate-out-fade 1000ms; "
+                        + "-moz-animation: valo-animate-out-fade 1000ms; "
+                        + "animation: valo-animate-out-fade 1000ms;};");
+
+        MenuBar mb = new MenuBar();
+        file = mb.addItem("File", null);
+        file.addItem("File1", null);
+        file.addItem("File2", null);
+        file.addItem("File3", null);
+        edit = mb.addItem("Edit", null);
+        edit.addItem("Edit1", null);
+        edit.addItem("Edit2", null);
+        edit.addItem("Edit3", null);
+        help = mb.addItem("Help", null);
+        help.addItem("Help1", null);
+        help.addItem("Help2", null);
+        MenuItem helpMenuItem = help.addItem("Help3", null);
+        helpMenuItem.addItem("SubHelp3", null);
+
+        addComponent(mb);
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/menubar/MenuBarSubmenusClosingValoTest.java b/uitest/src/com/vaadin/tests/components/menubar/MenuBarSubmenusClosingValoTest.java
new file mode 100644
index 0000000000..b7ed88c9ca
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/menubar/MenuBarSubmenusClosingValoTest.java
@@ -0,0 +1,72 @@
+/*
+ * 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.menubar;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.HasInputDevices;
+import org.openqa.selenium.interactions.Mouse;
+import org.openqa.selenium.internal.Locatable;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.MenuBarElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class MenuBarSubmenusClosingValoTest extends MultiBrowserTest {
+
+    @Test
+    public void testEnableParentLayoutControlByKeyboard() {
+        openTestURL();
+
+        MenuBarElement menu = $(MenuBarElement.class).get(0);
+        menu.focus();
+        menu.sendKeys(Keys.SPACE);
+        menu.sendKeys(Keys.DOWN);
+
+        waitForElementPresent(By.className("v-menubar-popup"));
+
+        menu.sendKeys(Keys.ARROW_RIGHT);
+        menu.sendKeys(Keys.ARROW_RIGHT);
+
+        int count = driver.findElements(By.className("v-menubar-popup")).size();
+        Assert.assertTrue("The count of open popups should be one", count == 1);
+    }
+
+    @Test
+    public void testEnableParentLayoutControlByMouse() {
+        openTestURL();
+
+        Mouse mouse = ((HasInputDevices) getDriver()).getMouse();
+
+        List<WebElement> menuItemList = driver.findElements(By
+                .className("v-menubar-menuitem"));
+
+        mouse.click(((Locatable) menuItemList.get(0)).getCoordinates());
+        waitForElementPresent(By.className("v-menubar-popup"));
+
+        mouse.mouseMove(((Locatable) menuItemList.get(1)).getCoordinates());
+        mouse.mouseMove(((Locatable) menuItemList.get(2)).getCoordinates());
+
+        waitForElementPresent(By.className("v-menubar-popup"));
+
+        int count = driver.findElements(By.className("v-menubar-popup")).size();
+        Assert.assertTrue("The count of open popups should be one", count == 1);
+    }
+}