From dd8cf12dde433198070f77d97d9220dd66a7e7d5 Mon Sep 17 00:00:00 2001 From: Dmitrii Rogozin Date: Tue, 17 Jun 2014 14:43:34 +0300 Subject: [PATCH] Bug fix menu navigation with space (#14041) Change-Id: I1466381b12a147fea90240420d3f6c05cd156a1b --- client/src/com/vaadin/client/Util.java | 19 ++++- client/src/com/vaadin/client/ui/VMenuBar.java | 84 +++++++++++-------- .../menubar/SpaceMenuBarNavigationTest.java | 17 ++-- 3 files changed, 77 insertions(+), 43 deletions(-) diff --git a/client/src/com/vaadin/client/Util.java b/client/src/com/vaadin/client/Util.java index e031b37422..f12a02c64f 100644 --- a/client/src/com/vaadin/client/Util.java +++ b/client/src/com/vaadin/client/Util.java @@ -35,6 +35,7 @@ import com.google.gwt.dom.client.Style; import com.google.gwt.dom.client.Style.Display; import com.google.gwt.dom.client.Style.Unit; import com.google.gwt.dom.client.Touch; +import com.google.gwt.event.dom.client.KeyEvent; import com.google.gwt.user.client.Command; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Event; @@ -66,7 +67,23 @@ public class Util { }-*/; /** - * + * Helper method for a bug fix #14041. For mozilla getKeyCode return 0 for + * space bar (because space is considered as char). If return 0 use + * getCharCode. + * + * @param event + * @return return key code + */ + public static int getKeyCode(KeyEvent event) { + int keyCode = event.getNativeEvent().getKeyCode(); + if (keyCode == 0) { + keyCode = event.getNativeEvent().getCharCode(); + } + return keyCode; + } + + /** + * * Returns the topmost element of from given coordinates. * * TODO fix crossplat issues clientX vs pageX. See quircksmode. Not critical diff --git a/client/src/com/vaadin/client/ui/VMenuBar.java b/client/src/com/vaadin/client/ui/VMenuBar.java index 11fda6222c..909b25f7fb 100644 --- a/client/src/com/vaadin/client/ui/VMenuBar.java +++ b/client/src/com/vaadin/client/ui/VMenuBar.java @@ -1,12 +1,12 @@ /* * 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 @@ -243,7 +243,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * This is called by the items in the menu and it communicates the * information to the server - * + * * @param clickedItemId * id of the item that was clicked */ @@ -280,7 +280,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Returns the containing element of the menu - * + * * @return */ @Override @@ -290,7 +290,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Add a new item to this menu - * + * * @param html * items text * @param cmd @@ -307,7 +307,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Add a new item to this menu - * + * * @param item */ public void addItem(CustomMenuItem item) { @@ -332,7 +332,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Remove the given item from this menu - * + * * @param item */ public void removeItem(CustomMenuItem item) { @@ -429,7 +429,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * When an item is clicked - * + * * @param item */ public void itemClick(CustomMenuItem item) { @@ -460,7 +460,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * When the user hovers the mouse over the item - * + * * @param item */ public void itemOver(CustomMenuItem item) { @@ -485,7 +485,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * When the mouse is moved away from an item - * + * * @param item */ public void itemOut(CustomMenuItem item) { @@ -543,7 +543,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Shows the child menu of an item. The caller must ensure that the item has * a submenu. - * + * * @param item */ public void showChildMenu(CustomMenuItem item) { @@ -665,7 +665,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Hides the submenu of an item - * + * * @param item */ public void hideChildMenu(CustomMenuItem item) { @@ -729,7 +729,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Returns the parent menu of this menu, or null if this is the top-level * menu - * + * * @return */ public VMenuBar getParentMenu() { @@ -738,7 +738,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Set the parent menu of this menu - * + * * @param parent */ public void setParentMenu(VMenuBar parent) { @@ -748,7 +748,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Returns the currently selected item of this menu, or null if nothing is * selected - * + * * @return */ public CustomMenuItem getSelected() { @@ -757,7 +757,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Set the currently selected item of this menu - * + * * @param item */ public void setSelected(CustomMenuItem item) { @@ -774,9 +774,9 @@ public class VMenuBar extends SimpleFocusablePanel implements } /** - * + * * A class to hold information on menu items - * + * */ public static class CustomMenuItem extends Widget implements HasHTML { @@ -795,7 +795,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Default menu item {@link Widget} constructor for GWT.create(). - * + * * Use {@link #setHTML(String)} and {@link #setCommand(Command)} after * constructing a menu item. */ @@ -805,7 +805,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Creates a menu item {@link Widget}. - * + * * @param html * @param cmd * @deprecated use the default constructor and {@link #setHTML(String)} @@ -1039,7 +1039,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Checks if the item can be selected. - * + * * @return true if it is possible to select this item, false otherwise */ public boolean isSelectable() { @@ -1157,7 +1157,14 @@ public class VMenuBar extends SimpleFocusablePanel implements */ @Override public void onKeyPress(KeyPressEvent event) { - if (handleNavigation(event.getNativeEvent().getKeyCode(), + // A bug fix for #14041 + // getKeyCode and getCharCode return different values for different + // browsers + int keyCode = event.getNativeEvent().getKeyCode(); + if (keyCode == 0) { + keyCode = event.getNativeEvent().getCharCode(); + } + if (handleNavigation(keyCode, event.isControlKeyDown() || event.isMetaKeyDown(), event.isShiftKeyDown())) { event.preventDefault(); @@ -1173,7 +1180,14 @@ public class VMenuBar extends SimpleFocusablePanel implements */ @Override public void onKeyDown(KeyDownEvent event) { - if (handleNavigation(event.getNativeEvent().getKeyCode(), + // A bug fix for #14041 + // getKeyCode and getCharCode return different values for different + // browsers + int keyCode = event.getNativeEvent().getKeyCode(); + if (keyCode == 0) { + keyCode = event.getNativeEvent().getCharCode(); + } + if (handleNavigation(keyCode, event.isControlKeyDown() || event.isMetaKeyDown(), event.isShiftKeyDown())) { event.preventDefault(); @@ -1184,7 +1198,7 @@ public class VMenuBar extends SimpleFocusablePanel implements * Get the key that moves the selection upwards. By default it is the up * arrow key but by overriding this you can change the key to whatever you * want. - * + * * @return The keycode of the key */ protected int getNavigationUpKey() { @@ -1195,7 +1209,7 @@ public class VMenuBar extends SimpleFocusablePanel implements * Get the key that moves the selection downwards. By default it is the down * arrow key but by overriding this you can change the key to whatever you * want. - * + * * @return The keycode of the key */ protected int getNavigationDownKey() { @@ -1206,7 +1220,7 @@ public class VMenuBar extends SimpleFocusablePanel implements * Get the key that moves the selection left. By default it is the left * arrow key but by overriding this you can change the key to whatever you * want. - * + * * @return The keycode of the key */ protected int getNavigationLeftKey() { @@ -1217,7 +1231,7 @@ public class VMenuBar extends SimpleFocusablePanel implements * Get the key that moves the selection right. By default it is the right * arrow key but by overriding this you can change the key to whatever you * want. - * + * * @return The keycode of the key */ protected int getNavigationRightKey() { @@ -1227,7 +1241,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Get the key that selects a menu item. By default it is the Enter key but * by overriding this you can change the key to whatever you want. - * + * * @deprecated use {@link #isNavigationSelectKey(int)} instead * @return */ @@ -1240,7 +1254,7 @@ public class VMenuBar extends SimpleFocusablePanel implements * Checks whether key code selects a menu item. By default it is the Enter * and Space keys but by overriding this you can change the keys to whatever * you want. - * + * * @since 7.2 * @param keycode * @return true if key selects menu item @@ -1253,7 +1267,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Get the key that closes the menu. By default it is the escape key but by * overriding this yoy can change the key to whatever you want. - * + * * @return */ protected int getCloseMenuKey() { @@ -1262,7 +1276,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Handles the keyboard events handled by the MenuBar - * + * * @param event * The keyboard event received * @return true iff the navigation event was handled @@ -1549,7 +1563,7 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Get menu item with given DOM element - * + * * @param element * Element used in search * @return Menu item or null if not found @@ -1578,11 +1592,11 @@ public class VMenuBar extends SimpleFocusablePanel implements /** * Get menu item with given DOM element - * + * * @param element * Element used in search * @return Menu item or null if not found - * + * * @since 7.2 */ public CustomMenuItem getMenuItemWithElement(Element element) { diff --git a/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java b/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java index 3e0053d0d1..a32a611d9a 100644 --- a/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java +++ b/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java @@ -1,12 +1,12 @@ /* * 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 @@ -23,10 +23,11 @@ import org.openqa.selenium.Keys; import org.openqa.selenium.WebElement; import com.vaadin.testbench.By; +import com.vaadin.testbench.elements.MenuBarElement; import com.vaadin.tests.tb3.MultiBrowserTest; /** - * + * * @since 7.2 * @author Vaadin Ltd */ @@ -36,19 +37,21 @@ public class SpaceMenuBarNavigationTest extends MultiBrowserTest { public void testEnableParentLayout() { openTestURL(); - WebElement menu = driver.findElement(By.className("menu-bar")); + MenuBarElement menu = $(MenuBarElement.class).get(0); + menu.focus(); menu.sendKeys(Keys.ARROW_RIGHT); - menu.sendKeys(Keys.SPACE); + menu.sendKeys(Keys.ENTER); List captions = driver.findElements(By .className("v-menubar-menuitem-caption")); boolean found = false; + for (WebElement caption : captions) { if ("subitem".equals(caption.getText())) { found = true; } } - Assert.assertTrue("Sub menu is not opened on SPACE key", found); + Assert.assertTrue("Sub menu is not opened on ENTER key", found); menu.sendKeys(Keys.SPACE); -- 2.39.5