From: Leif Åstrand Date: Tue, 24 Mar 2015 08:46:39 +0000 (+0200) Subject: Revert "Moved accessibility shortcut handling to server-side. [#14843]" X-Git-Tag: 7.5.0.beta1~25 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=0a7d5bcd585dcfc08ea7c4339dc03f29cea6fde4;p=vaadin-framework.git Revert "Moved accessibility shortcut handling to server-side. [#14843]" This reverts commit e88f71dd6d1d7634e3a90a7e53859ff6dc028e21. Change-Id: I1d4ed60ec4a194f6ed18fa5506134ef3a185a6cf --- diff --git a/client/src/com/vaadin/client/ui/VWindow.java b/client/src/com/vaadin/client/ui/VWindow.java index d28686e777..c5cab8ff6b 100644 --- a/client/src/com/vaadin/client/ui/VWindow.java +++ b/client/src/com/vaadin/client/ui/VWindow.java @@ -1354,7 +1354,9 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner, @Override public void onKeyUp(KeyUpEvent event) { - // do nothing + if (isClosable() && event.getNativeKeyCode() == KeyCodes.KEY_ESCAPE) { + onCloseClick(); + } } @Override diff --git a/server/src/com/vaadin/event/ShortcutAction.java b/server/src/com/vaadin/event/ShortcutAction.java index 32b909e9f2..09accae1c7 100644 --- a/server/src/com/vaadin/event/ShortcutAction.java +++ b/server/src/com/vaadin/event/ShortcutAction.java @@ -17,8 +17,6 @@ package com.vaadin.event; import java.io.Serializable; -import java.util.ArrayList; -import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -238,42 +236,6 @@ public class ShortcutAction extends Action { return modifiers; } - /** - * Checks whether the shortcut can be triggered with the given combination - * of keys. - * - * @param keyCode - * potential match for the {@link KeyCode} that this shortcut - * reacts to - * @param modifierKeys - * (optional) potential matches for the {@link ModifierKey}s - * required for this shortcut to react - * @return true if keyCode and modifierKeys are a match, - * false otherwise - */ - public boolean isTriggeredBy(int keyCode, int... modifierKeys) { - boolean result = false; - if (keyCode == this.keyCode) { - if (modifierKeys == null) { - result = (modifiers == null); - } else if (modifiers != null) { - List modifierList = new ArrayList(); - for (int modifier : modifiers) { - modifierList.add(modifier); - } - for (int modifierKey : modifierKeys) { - if (modifierList.contains(modifierKey)) { - modifierList.remove(modifierKey); - } else { - return false; - } - } - result = modifierList.isEmpty(); - } - } - return result; - } - /** * Key codes that can be used for shortcuts * diff --git a/server/src/com/vaadin/ui/Window.java b/server/src/com/vaadin/ui/Window.java index e7764ffd8d..61664fc95d 100644 --- a/server/src/com/vaadin/ui/Window.java +++ b/server/src/com/vaadin/ui/Window.java @@ -21,9 +21,6 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -133,7 +130,6 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier, super(caption, content); registerRpc(rpc); setSizeUndefined(); - setCloseShortcut(ShortcutAction.KeyCode.ESCAPE); } /* ********************************************************************* */ @@ -820,48 +816,14 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier, /* * Actions */ - private LinkedHashSet closeShortcuts = new LinkedHashSet(); - - protected Collection getCloseShortcuts() { - return Collections.unmodifiableCollection(closeShortcuts); - } - - /** - * Adds a keyboard shortcut for closing the window when user presses the - * given {@link KeyCode} and (optional) {@link ModifierKey}s.
- * Note that this shortcut only reacts while the window has focus, closing - * itself - if you want to close a window from a UI, use - * {@link UI#addAction(com.vaadin.event.Action)} of the UI instead. - *

- * If there is a prior CloseShortcut with the same keycode and modifiers, - * that gets removed before the new one is added. Prior CloseShortcuts with - * differing keycodes or modifiers are not affected. - * - * @param keyCode - * the keycode for invoking the shortcut - * @param modifiers - * the (optional) modifiers for invoking the shortcut, null for - * none - */ - public void addCloseShortcut(int keyCode, int... modifiers) { - // make sure there are no duplicates - removeCloseShortcut(keyCode, modifiers); - CloseShortcut closeShortcut = new CloseShortcut(this, keyCode, - modifiers); - closeShortcuts.add(closeShortcut); - addAction(closeShortcut); - } + protected CloseShortcut closeShortcut; /** - * Sets the keyboard shortcut for closing the window when user presses the - * given {@link KeyCode} and (optional) {@link ModifierKey}s.
+ * Makes is possible to close the window by pressing the given + * {@link KeyCode} and (optional) {@link ModifierKey}s.
* Note that this shortcut only reacts while the window has focus, closing * itself - if you want to close a window from a UI, use * {@link UI#addAction(com.vaadin.event.Action)} of the UI instead. - *

- * If there are any prior CloseShortcuts when this method is called those - * get removed before the new one is added. NOTE: this also removes the - * default shortcut that is added for accessibility purposes. * * @param keyCode * the keycode for invoking the shortcut @@ -870,61 +832,22 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier, * none */ public void setCloseShortcut(int keyCode, int... modifiers) { - removeCloseShortcuts(); - addCloseShortcut(keyCode, modifiers); - } - - /** - * Removes a keyboard shortcut previously set with - * {@link #setCloseShortcut(int, int...)} or - * {@link #addCloseShortcut(int, int...)}. - * - * @param keyCode - * the keycode for invoking the shortcut - * @param modifiers - * the (optional) modifiers for invoking the shortcut, null for - * none - */ - public void removeCloseShortcut(int keyCode, int... modifiers) { - for (CloseShortcut closeShortcut : closeShortcuts) { - if (closeShortcut.isTriggeredBy(keyCode, modifiers)) { - removeAction(closeShortcut); - closeShortcuts.remove(closeShortcut); - break; - } + if (closeShortcut != null) { + removeAction(closeShortcut); } + closeShortcut = new CloseShortcut(this, keyCode, modifiers); + addAction(closeShortcut); } /** - * @deprecated use {@link #resetCloseShortcuts()} instead, or - * {@link #removeCloseShortcuts()} if you also want to get rid - * of the default shortcut + * Removes the keyboard shortcut previously set with + * {@link #setCloseShortcut(int, int...)}. */ - @Deprecated public void removeCloseShortcut() { - resetCloseShortcuts(); - } - - /** - * Removes all the keyboard shortcuts previously set with - * {@link #setCloseShortcut(int, int...)} or - * {@link #addCloseShortcut(int, int...)} and re-adds the default shortcut - * {@link KeyCode.ESCAPE}. - */ - public void resetCloseShortcuts() { - setCloseShortcut(ShortcutAction.KeyCode.ESCAPE); - } - - /** - * Removes all the keyboard shortcuts previously set with - * {@link #setCloseShortcut(int, int...)} or - * {@link #addCloseShortcut(int, int...)}. - */ - public void removeCloseShortcuts() { - for (CloseShortcut closeShortcut : closeShortcuts) { + if (closeShortcut != null) { removeAction(closeShortcut); + closeShortcut = null; } - closeShortcuts.clear(); } /** @@ -1391,8 +1314,7 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier, } private CloseShortcut getCloseShortcut() { - Iterator i = getCloseShortcuts().iterator(); - return i.hasNext() ? i.next() : null; + return closeShortcut; } @Override diff --git a/uitest/src/com/vaadin/tests/components/window/CloseShortcut.java b/uitest/src/com/vaadin/tests/components/window/CloseShortcut.java deleted file mode 100644 index c7499134e0..0000000000 --- a/uitest/src/com/vaadin/tests/components/window/CloseShortcut.java +++ /dev/null @@ -1,121 +0,0 @@ -/* - * 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.window; - -import com.vaadin.data.Property; -import com.vaadin.data.Property.ValueChangeEvent; -import com.vaadin.event.ShortcutAction.KeyCode; -import com.vaadin.event.ShortcutAction.ModifierKey; -import com.vaadin.server.VaadinRequest; -import com.vaadin.tests.components.AbstractTestUI; -import com.vaadin.ui.Button; -import com.vaadin.ui.Button.ClickEvent; -import com.vaadin.ui.CheckBox; -import com.vaadin.ui.TextField; -import com.vaadin.ui.VerticalLayout; -import com.vaadin.ui.Window; - -/** - * Tests close shortcuts for Window. - * - * @author Vaadin Ltd - */ -public class CloseShortcut extends AbstractTestUI { - - @Override - protected void setup(VaadinRequest request) { - final Window w = new Window(); - w.setWidth("300px"); - w.setHeight("300px"); - w.center(); - addWindow(w); - w.focus(); - - // add textfield to the window to give TestBench something to send the - // keys to - VerticalLayout content = new VerticalLayout(); - content.setMargin(true); - TextField textField = new TextField(); - textField.setSizeFull(); - content.addComponent(textField); - w.setContent(content); - - final CheckBox cbDefault = new CheckBox("Use default (ESC) shortcut"); - cbDefault.setId("default"); - addComponent(cbDefault); - final CheckBox cbOther = new CheckBox("Use R shortcut"); - cbOther.setId("other"); - addComponent(cbOther); - final CheckBox cbCtrl = new CheckBox("Use CTRL+A shortcut"); - cbCtrl.setId("control"); - addComponent(cbCtrl); - final CheckBox cbShift = new CheckBox("Use SHIFT+H shortcut"); - cbShift.setId("shift"); - addComponent(cbShift); - - cbOther.setValue(true); - cbCtrl.setValue(true); - cbShift.setValue(true); - - Property.ValueChangeListener listener = new Property.ValueChangeListener() { - - @Override - public void valueChange(ValueChangeEvent event) { - if (Boolean.TRUE.equals(cbDefault.getValue())) { - w.resetCloseShortcuts(); - } else { - w.removeCloseShortcuts(); - } - if (Boolean.TRUE.equals(cbOther.getValue())) { - w.addCloseShortcut(KeyCode.R); - } - if (Boolean.TRUE.equals(cbCtrl.getValue())) { - w.addCloseShortcut(KeyCode.A, ModifierKey.CTRL); - } - if (Boolean.TRUE.equals(cbShift.getValue())) { - w.addCloseShortcut(KeyCode.H, ModifierKey.SHIFT); - } - } - }; - cbDefault.addValueChangeListener(listener); - cbOther.addValueChangeListener(listener); - cbCtrl.addValueChangeListener(listener); - cbShift.addValueChangeListener(listener); - - cbDefault.setValue(true); // trigger value change - - Button button = new Button("Reopen window", new Button.ClickListener() { - - @Override - public void buttonClick(ClickEvent event) { - w.close(); - addWindow(w); - w.focus(); - } - }); - addComponent(button); - } - - @Override - protected String getTestDescription() { - return "It should be possible to have multiple shortcuts at the same time, and to remove the default shortcut ESC."; - } - - @Override - protected Integer getTicketNumber() { - return 14843; - } -} diff --git a/uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java b/uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java deleted file mode 100644 index df7b7cad0c..0000000000 --- a/uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java +++ /dev/null @@ -1,219 +0,0 @@ -/* - * 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.window; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import org.junit.Before; -import org.junit.Test; -import org.openqa.selenium.By; -import org.openqa.selenium.Keys; -import org.openqa.selenium.WebDriver; -import org.openqa.selenium.interactions.Actions; -import org.openqa.selenium.remote.DesiredCapabilities; -import org.openqa.selenium.support.ui.ExpectedCondition; - -import com.vaadin.testbench.elements.CheckBoxElement; -import com.vaadin.testbench.elements.TextFieldElement; -import com.vaadin.testbench.elements.WindowElement; -import com.vaadin.testbench.parallel.BrowserUtil; -import com.vaadin.tests.tb3.MultiBrowserTest; - -/** - * Tests close shortcuts for Window. - * - * @author Vaadin Ltd - */ -public class CloseShortcutTest extends MultiBrowserTest { - - private WindowElement window; - private CheckBoxElement cbDefault; - private CheckBoxElement cbOther; - private CheckBoxElement cbCtrl; - private CheckBoxElement cbShift; - - @Override - @Before - public void setup() throws Exception { - super.setup(); - openTestURL(); - waitForElementPresent(By.className("v-window")); - - window = $(WindowElement.class).first(); - cbDefault = $(CheckBoxElement.class).id("default"); - cbOther = $(CheckBoxElement.class).id("other"); - cbCtrl = $(CheckBoxElement.class).id("control"); - cbShift = $(CheckBoxElement.class).id("shift"); - } - - @Test - public void testAllCheckBoxesSelected() { - assertTrue("Default wasn't selected initially.", isChecked(cbDefault)); - assertTrue("Other wasn't selected initially.", isChecked(cbOther)); - assertTrue("Ctrl+A wasn't selected initially.", isChecked(cbCtrl)); - assertTrue("Shift+H wasn't selected initially.", isChecked(cbShift)); - } - - @Test - public void testAllCheckBoxesClickable() { - click(cbDefault); - click(cbOther); - click(cbCtrl); - click(cbShift); - - assertFalse("Default was selected when it shouldn't have been.", - isChecked(cbDefault)); - assertFalse("Other was selected when it shouldn't have been.", - isChecked(cbOther)); - assertFalse("Ctrl+A was selected when it shouldn't have been.", - isChecked(cbCtrl)); - assertFalse("Shift+H was selected when it shouldn't have been.", - isChecked(cbShift)); - } - - @Test - public void testDefaultWithAll() { - attemptDefaultShortcut(); - ensureWindowClosed(); - } - - @Test - public void testDefaultWithoutSelection() { - click(cbDefault); - - attemptDefaultShortcut(); - ensureWindowOpen(); - } - - @Test - public void testOtherWithAll() { - attemptOtherShortcut(); - // TODO: remove this check once #14902 has been fixed - DesiredCapabilities cap = getDesiredCapabilities(); - if ((BrowserUtil.isIE(cap) && !BrowserUtil.isIE8(cap)) - || BrowserUtil.isPhantomJS(cap)) { - ensureWindowClosed(); - } - } - - @Test - public void testOtherWithoutSelection() { - click(cbOther); - - attemptOtherShortcut(); - ensureWindowOpen(); - } - - @Test - public void testCtrlWithAll() { - attemptCtrlShortcut(); - // TODO: remove this check once #14902 has been fixed - if (BrowserUtil.isPhantomJS(getDesiredCapabilities())) { - ensureWindowClosed(); - } - } - - @Test - public void testCtrlWithoutSelection() { - click(cbCtrl); - - attemptCtrlShortcut(); - ensureWindowOpen(); - } - - @Test - public void testShiftWithAll() { - attemptShiftShortcut(); - // TODO: remove this check once #14902 has been fixed - DesiredCapabilities capabilities = getDesiredCapabilities(); - if (!BrowserUtil.isIE(capabilities) || BrowserUtil.isIE8(capabilities)) { - ensureWindowClosed(); - } - } - - @Test - public void testShiftWithoutSelection() { - click(cbShift); - - attemptShiftShortcut(); - ensureWindowOpen(); - } - - private boolean isChecked(CheckBoxElement cb) { - String checked = cb.findElement(By.tagName("input")).getAttribute( - "checked"); - if ("true".equals(checked)) { - return true; - } else if (checked == null) { - return false; - } - throw new IllegalStateException( - "Unexpected attribute value for 'checked': " + checked); - } - - @Override - protected void click(final CheckBoxElement cb) { - final boolean initial = isChecked(cb); - super.click(cb); - waitUntil(new ExpectedCondition() { - @Override - public Boolean apply(WebDriver input) { - return initial != isChecked(cb); - } - - @Override - public String toString() { - // Timed out after 10 seconds waiting for ... - return "checked state to change"; - } - }); - } - - private void attemptDefaultShortcut() { - window.focus(); - $(TextFieldElement.class).first().sendKeys(Keys.ESCAPE); - } - - private void attemptOtherShortcut() { - window.focus(); - $(TextFieldElement.class).first().sendKeys("R"); - } - - private void attemptCtrlShortcut() { - window.focus(); - new Actions(driver).keyDown(Keys.CONTROL).perform(); - $(TextFieldElement.class).first().sendKeys("A"); - new Actions(driver).keyUp(Keys.CONTROL).perform(); - } - - private void attemptShiftShortcut() { - window.focus(); - new Actions(driver).keyDown(Keys.SHIFT).perform(); - $(TextFieldElement.class).first().sendKeys("H"); - new Actions(driver).keyUp(Keys.SHIFT).perform(); - } - - private void ensureWindowClosed() { - assertTrue("Window didn't close as expected.", $(WindowElement.class) - .all().isEmpty()); - } - - private void ensureWindowOpen() { - assertFalse("Window closed when it shouldn't have.", - $(WindowElement.class).all().isEmpty()); - } -}