diff options
author | Leif Åstrand <leif@vaadin.com> | 2015-03-24 10:46:39 +0200 |
---|---|---|
committer | Leif Åstrand <leif@vaadin.com> | 2015-03-24 10:47:21 +0200 |
commit | e00c161317fb5ff47e8e850baf90e8f595107966 (patch) | |
tree | e43f6bd9ae7da8e80b05ae6d08ff53fecac8a2bf | |
parent | b21af83dd731d7aba662cac5a04e9553723ddfe6 (diff) | |
download | vaadin-framework-e00c161317fb5ff47e8e850baf90e8f595107966.tar.gz vaadin-framework-e00c161317fb5ff47e8e850baf90e8f595107966.zip |
Revert "Moved accessibility shortcut handling to server-side. [#14843]"
This reverts commit e88f71dd6d1d7634e3a90a7e53859ff6dc028e21.
Conflicts:
WebContent/release-notes.html
uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java
Change-Id: I1d4ed60ec4a194f6ed18fa5506134ef3a185a6cf
-rw-r--r-- | client/src/com/vaadin/client/ui/VWindow.java | 4 | ||||
-rw-r--r-- | server/src/com/vaadin/event/ShortcutAction.java | 38 | ||||
-rw-r--r-- | server/src/com/vaadin/ui/Window.java | 99 | ||||
-rw-r--r-- | uitest/src/com/vaadin/tests/components/window/CloseShortcut.java | 121 | ||||
-rw-r--r-- | uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java | 219 |
5 files changed, 14 insertions, 467 deletions
diff --git a/client/src/com/vaadin/client/ui/VWindow.java b/client/src/com/vaadin/client/ui/VWindow.java index 501dedbaa8..6977cf9e7f 100644 --- a/client/src/com/vaadin/client/ui/VWindow.java +++ b/client/src/com/vaadin/client/ui/VWindow.java @@ -1338,7 +1338,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; @@ -239,42 +237,6 @@ public class ShortcutAction extends Action { } /** - * 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 <code>true</code> if keyCode and modifierKeys are a match, - * <code>false</code> 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<Integer> modifierList = new ArrayList<Integer>(); - 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 653b620746..35583c6052 100644 --- a/server/src/com/vaadin/ui/Window.java +++ b/server/src/com/vaadin/ui/Window.java @@ -18,9 +18,6 @@ package com.vaadin.ui; import java.io.Serializable; import java.lang.reflect.Method; -import java.util.Collection; -import java.util.Collections; -import java.util.LinkedHashSet; import java.util.Map; import com.vaadin.event.FieldEvents.BlurEvent; @@ -123,7 +120,6 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier, super(caption, content); registerRpc(rpc); setSizeUndefined(); - setCloseShortcut(ShortcutAction.KeyCode.ESCAPE); } /* ********************************************************************* */ @@ -810,48 +806,14 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier, /* * Actions */ - private LinkedHashSet<CloseShortcut> closeShortcuts = new LinkedHashSet<CloseShortcut>(); - - protected Collection<CloseShortcut> 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.<br/> - * 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. - * <p> - * 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.<br/> + * Makes is possible to close the window by pressing the given + * {@link KeyCode} and (optional) {@link ModifierKey}s.<br/> * 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. - * <p> - * If there are any prior CloseShortcuts when this method is called those - * get removed before the new one is added. <b>NOTE: this also removes the - * default shortcut that is added for accessibility purposes.</b> * * @param keyCode * the keycode for invoking the shortcut @@ -860,61 +822,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(); } /** 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<Boolean>() { - @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()); - } -} |