Allow using modifier + enter as shorcuts in TextArea (#13811)

Change-Id: I99db3d6280f0066ed1249cb348da7e82381b45c2

client/src/com/vaadin/client/ui/VTextArea.java

@@ -262,10 +262,13 @@ public class VTextArea extends VTextField implements DragImageModifier {
 
         @Override
         public void onKeyDown(KeyDownEvent event) {
-            // Fix for #12424 - if the key being pressed is enter, we stop
-            // propagation of the KeyDownEvents. This prevents shortcuts that
-            // are bound to the enter key from being processed.
-            if (event.getNativeKeyCode() == KeyCodes.KEY_ENTER) {
+            // Fix for #12424/13811 - if the key being pressed is enter, we stop
+            // propagation of the KeyDownEvents if there were no modifier keys
+            // also pressed. This prevents shortcuts that are bound to only the
+            // enter key from being processed but allows usage of e.g.
+            // shift-enter or ctrl-enter.
+            if (event.getNativeKeyCode() == KeyCodes.KEY_ENTER
+                    && !event.isAnyModifierKeyDown()) {
                 event.stopPropagation();
             }
         }

uitest/src/com/vaadin/tests/components/ui/TextAreaEventPropagation.java

@@ -16,13 +16,13 @@
 package com.vaadin.tests.components.ui;
 
 import com.vaadin.event.ShortcutAction.KeyCode;
+import com.vaadin.event.ShortcutAction.ModifierKey;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.tests.components.AbstractTestUIWithLog;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
 import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.FormLayout;
-import com.vaadin.ui.Label;
 import com.vaadin.ui.TextArea;
 import com.vaadin.ui.TextField;
 
@@ -34,24 +34,12 @@ import com.vaadin.ui.TextField;
  */
 public class TextAreaEventPropagation extends AbstractTestUIWithLog {
 
-    protected static final String BUTTON_PRESSED = "Button Pressed";
-
-    protected static final String NO_BUTTON_PRESSED = "No Button Pressed";
-
-    private Label enterButtonPressed;
-
-    private Label escapeButtonPressed;
-
     @Override
     protected void setup(VaadinRequest request) {
 
         FormLayout form = new FormLayout();
         TextArea textArea = new TextArea("Text input");
         TextField textField = new TextField("Text field input");
-        enterButtonPressed = new Label(NO_BUTTON_PRESSED);
-        enterButtonPressed.setCaption("Enter Label");
-        escapeButtonPressed = new Label(NO_BUTTON_PRESSED);
-        escapeButtonPressed.setCaption("Escape Label");
 
         Button enterButton = new Button("Enter");
         enterButton.setClickShortcut(KeyCode.ENTER);
@@ -60,7 +48,29 @@ public class TextAreaEventPropagation extends AbstractTestUIWithLog {
             @Override
             public void buttonClick(ClickEvent event) {
 
-                enterButtonPressed.setValue(BUTTON_PRESSED);
+                log("Enter button pressed");
+            }
+        });
+
+        Button shiftEnterButton = new Button("Shift-Enter");
+        shiftEnterButton.setClickShortcut(KeyCode.ENTER, ModifierKey.SHIFT);
+        shiftEnterButton.addClickListener(new ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+
+                log("Shift-Enter button pressed");
+            }
+        });
+
+        Button ctrlEnterButton = new Button("Ctrl-Enter");
+        ctrlEnterButton.setClickShortcut(KeyCode.ENTER, ModifierKey.CTRL);
+        ctrlEnterButton.addClickListener(new ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+
+                log("Ctrl-Enter button pressed");
             }
         });
 
@@ -70,8 +80,7 @@ public class TextAreaEventPropagation extends AbstractTestUIWithLog {
 
             @Override
             public void buttonClick(ClickEvent event) {
-
-                escapeButtonPressed.setValue(BUTTON_PRESSED);
+                log("Escape button pressed");
             }
         });
 
@@ -79,15 +88,15 @@ public class TextAreaEventPropagation extends AbstractTestUIWithLog {
         form.addComponent(textField);
         form.addComponent(enterButton);
         form.addComponent(escapeButton);
-        form.addComponent(enterButtonPressed);
-        form.addComponent(escapeButtonPressed);
+        form.addComponent(shiftEnterButton);
+        form.addComponent(ctrlEnterButton);
         addComponent(form);
 
     }
 
     @Override
     protected String getTestDescription() {
-        return "Currently if enter key is set as a shortcut for some component, it won't be possible for the user to enter newline in a textarea.";
+        return "Enter as s shortcut in a TextArea should not trigger shortcuts as enter is handled internally. Enter + modifier key should trigger shortcut.";
     }
 
     @Override

uitest/src/com/vaadin/tests/components/ui/TextAreaEventPropagationModifierKeysTest.java

@@ -0,0 +1,87 @@
+/*
+ * 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.ui;
+
+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.Actions;
+import org.openqa.selenium.remote.DesiredCapabilities;
+
+import com.vaadin.testbench.elements.TextAreaElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class TextAreaEventPropagationModifierKeysTest extends MultiBrowserTest {
+    @Test
+    public void textAreaShiftEnterEventPropagation()
+            throws InterruptedException {
+        openTestURL();
+
+        WebElement textArea = $(TextAreaElement.class).first();
+        Actions builder = new Actions(driver);
+        builder.click(textArea);
+        builder.sendKeys(textArea, "first line asdf");
+        builder.keyDown(Keys.SHIFT);
+        builder.sendKeys(Keys.ENTER);
+        builder.keyUp(Keys.SHIFT);
+        builder.sendKeys(textArea, "second line jkl;");
+        builder.perform();
+
+        // Should have triggered shortcut
+        Assert.assertEquals("1. Shift-Enter button pressed", getLogRow(0));
+    }
+
+    @Test
+    public void textAreaCtrlEnterEventPropagation() throws InterruptedException {
+        openTestURL();
+
+        WebElement textArea = $(TextAreaElement.class).first();
+        Actions builder = new Actions(driver);
+        builder.click(textArea);
+        builder.sendKeys(textArea, "first line asdf");
+        builder.keyDown(Keys.CONTROL);
+        builder.sendKeys(Keys.ENTER);
+        builder.keyUp(Keys.CONTROL);
+        builder.sendKeys(textArea, "second line jkl;");
+        builder.perform();
+
+        // Should have triggered shortcut
+        Assert.assertEquals("1. Ctrl-Enter button pressed", getLogRow(0));
+    }
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        List<DesiredCapabilities> browsers = super.getBrowsersToTest();
+        // Can't handle ctrl
+        browsers.remove(Browser.IE8.getDesiredCapabilities());
+        browsers.remove(Browser.FIREFOX.getDesiredCapabilities());
+
+        // Can't handle shift or ctrl
+        browsers.remove(Browser.IE9.getDesiredCapabilities());
+        browsers.remove(Browser.IE10.getDesiredCapabilities());
+        browsers.remove(Browser.IE11.getDesiredCapabilities());
+        return browsers;
+
+    }
+
+    @Override
+    protected Class<?> getUIClass() {
+        return TextAreaEventPropagation.class;
+    }
+}

uitest/src/com/vaadin/tests/components/ui/TextAreaEventPropagationTest.java

@@ -22,7 +22,6 @@ import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.Actions;
 
-import com.vaadin.testbench.elements.LabelElement;
 import com.vaadin.testbench.elements.TextAreaElement;
 import com.vaadin.testbench.elements.TextFieldElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
@@ -35,7 +34,7 @@ import com.vaadin.tests.tb3.MultiBrowserTest;
 public class TextAreaEventPropagationTest extends MultiBrowserTest {
 
     @Test
-    public void testTextAreaEnterEventPropagation() throws InterruptedException {
+    public void textAreaEnterEventPropagation() throws InterruptedException {
         openTestURL();
         WebElement textArea = $(TextAreaElement.class).first();
         Actions builder = new Actions(driver);
@@ -45,23 +44,8 @@ public class TextAreaEventPropagationTest extends MultiBrowserTest {
         builder.sendKeys(textArea, "second line jkl;");
         builder.perform();
 
-        String text = $(LabelElement.class).caption("Enter Label").first()
-                .getText();
-        assertEquals(TextAreaEventPropagation.NO_BUTTON_PRESSED, text);
-
-        WebElement textField = $(TextFieldElement.class).first();
-        Actions builder2 = new Actions(driver);
-        builder2.click(textField);
-
-        builder2.sendKeys("third line");
-        builder2.sendKeys(Keys.ENTER);
-
-        builder2.perform();
-
-        text = $(LabelElement.class).caption("Enter Label").first().getText();
-
-        assertEquals(TextAreaEventPropagation.BUTTON_PRESSED, text);
-
+        // Should not have triggered shortcut
+        assertEquals(" ", getLogRow(0));
     }
 
     @Test
@@ -72,35 +56,16 @@ public class TextAreaEventPropagationTest extends MultiBrowserTest {
         Actions builder = new Actions(driver);
         builder.click(textArea);
         builder.sendKeys(textArea, "first line asdf");
-        builder.sendKeys(Keys.ENTER);
-        builder.sendKeys(textArea, "second line jkl;");
         builder.sendKeys(Keys.ESCAPE);
+        builder.sendKeys(textArea, "second line jkl;");
         builder.perform();
 
-        String text = $(LabelElement.class).caption("Enter Label").first()
-                .getText();
-        assertEquals(TextAreaEventPropagation.NO_BUTTON_PRESSED, text);
-        text = $(LabelElement.class).caption("Escape Label").first().getText();
-        assertEquals(TextAreaEventPropagation.BUTTON_PRESSED, text);
-
+        assertEquals("1. Escape button pressed", getLogRow(0));
     }
 
     @Test
-    public void testTextFieldEscapeEventPropagation()
-            throws InterruptedException {
+    public void testTextFieldEscapeEventPropagation() {
         openTestURL();
-        WebElement textArea = $(TextAreaElement.class).first();
-        Actions builder = new Actions(driver);
-        builder.click(textArea);
-        builder.sendKeys(textArea, "first line asdf");
-        builder.sendKeys(Keys.ENTER);
-        builder.sendKeys(textArea, "second line jkl;");
-        builder.perform();
-
-        String text = $(LabelElement.class).caption("Enter Label").first()
-                .getText();
-        assertEquals(TextAreaEventPropagation.NO_BUTTON_PRESSED, text);
-
         WebElement textField = $(TextFieldElement.class).first();
         Actions builder2 = new Actions(driver);
         builder2.click(textField);
@@ -111,10 +76,22 @@ public class TextAreaEventPropagationTest extends MultiBrowserTest {
 
         builder2.perform();
 
-        text = $(LabelElement.class).caption("Enter Label").first().getText();
-        assertEquals(TextAreaEventPropagation.BUTTON_PRESSED, text);
-        text = $(LabelElement.class).caption("Escape Label").first().getText();
-        assertEquals(TextAreaEventPropagation.BUTTON_PRESSED, text);
+        assertEquals("1. Enter button pressed", getLogRow(1));
+        assertEquals("2. Escape button pressed", getLogRow(0));
+    }
+
+    @Test
+    public void testTextFieldEnterEventPropagation() {
+        openTestURL();
+        WebElement textField = $(TextFieldElement.class).first();
+        Actions builder2 = new Actions(driver);
+        builder2.click(textField);
+
+        builder2.sendKeys("third line");
+        builder2.sendKeys(Keys.ENTER);
+
+        builder2.perform();
 
+        assertEquals("1. Enter button pressed", getLogRow(0));
     }
 }