diff options
author | Jouni Koivuviita <jouni@vaadin.com> | 2014-12-10 10:42:37 +0200 |
---|---|---|
committer | Vaadin Code Review <review@vaadin.com> | 2014-12-10 08:49:07 +0000 |
commit | 98d0eec7bb50e4af342358d2aa2ed80fe4564c85 (patch) | |
tree | 176e1e22e7b131b5e5e68fff19dc6c3cd33363f7 | |
parent | e1b335022ec359061e696cf2711327663267e704 (diff) | |
download | vaadin-framework-98d0eec7bb50e4af342358d2aa2ed80fe4564c85.tar.gz vaadin-framework-98d0eec7bb50e4af342358d2aa2ed80fe4564c85.zip |
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 dfd81faf94..9071b6ee47 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" @@ -164,7 +164,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 @@ -187,9 +187,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; @@ -218,7 +218,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] * ---------------------------------------------- @@ -231,9 +231,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 @@ -280,7 +280,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 @@ -294,10 +294,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 @@ -361,7 +361,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 */ @@ -574,12 +574,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 @@ -593,7 +593,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); @@ -612,7 +612,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). @@ -721,7 +721,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() { @@ -783,13 +783,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 @@ -813,7 +813,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() { @@ -823,7 +823,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 */ @@ -834,7 +834,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() { @@ -854,7 +854,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. @@ -878,7 +878,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 @@ -905,7 +905,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 @@ -938,10 +938,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 */ @@ -957,10 +957,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 */ @@ -1000,10 +1000,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 @@ -1029,7 +1052,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); + } +} |