Table / TreeTable multiselect disabling of touch detection (#11641)

* Fixes #11601. Add toggle for disabling touch detection on table multiselect.

* fix tests

9 files changed, 248 insertions, 6 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/VScrollTable.java b/client/src/main/java/com/vaadin/client/ui/VScrollTable.java
index 3659bf6bb7..26155c5bd6 100644
--- a/client/src/main/java/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/main/java/com/vaadin/client/ui/VScrollTable.java
@@ -338,6 +338,8 @@ public class VScrollTable extends FlowPanel
 
     private SelectMode selectMode = SelectMode.NONE;
 
+    private boolean multiSelectTouchDetectionEnabled = true;
+
     public final HashSet<String> selectedRowKeys = new HashSet<String>();
 
     /*
@@ -1483,6 +1485,9 @@ public class VScrollTable extends FlowPanel
             } else {
                 selectMode = SelectMode.NONE;
             }
+            if (uidl.hasAttribute("touchdetection")) {
+                multiSelectTouchDetectionEnabled = uidl.getBooleanAttribute("touchdetection");
+            }
         }
     }
 
@@ -1936,9 +1941,9 @@ public class VScrollTable extends FlowPanel
     }
 
     private void setMultiSelectMode(int multiselectmode) {
-        if (BrowserInfo.get().isTouchDevice()) {
+        if (BrowserInfo.get().isTouchDevice() && multiSelectTouchDetectionEnabled) {
             // Always use the simple mode for touch devices that do not have
-            // shift/ctrl keys
+            // shift/ctrl keys (unless this feature is explicitly disabled)
             this.multiselectmode = MULTISELECT_MODE_SIMPLE;
         } else {
             this.multiselectmode = multiselectmode;
diff --git a/server/src/main/java/com/vaadin/ui/Table.java b/server/src/main/java/com/vaadin/ui/Table.java
index a2496a5c4f..930a152c61 100644
--- a/server/src/main/java/com/vaadin/ui/Table.java
+++ b/server/src/main/java/com/vaadin/ui/Table.java
@@ -577,6 +577,8 @@ public class Table extends AbstractSelect implements Action.Container,
 
     private MultiSelectMode multiSelectMode = MultiSelectMode.DEFAULT;
 
+    private boolean multiSelectTouchDetectionEnabled = true;
+
     private boolean rowCacheInvalidated;
 
     private RowGenerator rowGenerator = null;
@@ -3774,6 +3776,9 @@ public class Table extends AbstractSelect implements Action.Container,
         if (isSelectable()) {
             target.addAttribute("selectmode",
                     (isMultiSelect() ? "multi" : "single"));
+            if (isMultiSelect()) {
+                target.addAttribute("touchdetection", isMultiSelectTouchDetectionEnabled());
+            }
         } else {
             target.addAttribute("selectmode", "none");
         }
@@ -5185,7 +5190,10 @@ public class Table extends AbstractSelect implements Action.Container,
      * <p>
      * Note, that on some clients the mode may not be respected. E.g. on touch
      * based devices CTRL/SHIFT base selection method is invalid, so touch based
-     * browsers always use the {@link MultiSelectMode#SIMPLE}.
+     * browsers always use the {@link MultiSelectMode#SIMPLE} unless touch multi
+     * select is explicitly disabled .
+     *
+     * @see #setMultiSelectTouchDetectionEnabled(boolean)
      *
      * @param mode
      *            The select mode of the table
@@ -5205,6 +5213,31 @@ public class Table extends AbstractSelect implements Action.Container,
     }
 
     /**
+     * Default behavior on touch-reporting devices is to switch from CTRL/SHIFT
+     * based multi-selection to simple mode, but you can use this method to
+     * explicitly disable the touch device detection. Thus you can keep using
+     * keyboard-based multi selection on hybrid devices that have both a touch
+     * screen and a keyboard.
+     *
+     * @param multiSelectTouchDetectionEnabled
+     *               Whether to enable or disable touch screen detection
+     */
+    public void setMultiSelectTouchDetectionEnabled(boolean multiSelectTouchDetectionEnabled) {
+        this.multiSelectTouchDetectionEnabled = multiSelectTouchDetectionEnabled;
+        markAsDirty();
+    }
+
+    /**
+     * Returns if touch screen detection is used to toggle multi select mode.
+     *
+     * @return
+     *              If touch screen detection for multi select is enabled
+     */
+    public boolean isMultiSelectTouchDetectionEnabled() {
+        return multiSelectTouchDetectionEnabled;
+    }
+
+    /**
      * Lazy loading accept criterion for Table. Accepted target rows are loaded
      * from server once per drag and drop operation. Developer must override one
      * method that decides on which rows the currently dragged data can be
diff --git a/uitest/src/main/java/com/vaadin/tests/components/table/CtrlShiftMultiselect.java b/uitest/src/main/java/com/vaadin/tests/components/table/CtrlShiftMultiselect.java
index b9e23930e4..04838b9a88 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/table/CtrlShiftMultiselect.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/table/CtrlShiftMultiselect.java
@@ -10,10 +10,10 @@ import com.vaadin.ui.Table.TableDragMode;
 @SuppressWarnings("serial")
 public class CtrlShiftMultiselect extends TestBase {
 
+    protected final Table table = new Table("Multiselectable table");
+
     @Override
     protected void setup() {
-        final Table table = new Table("Multiselectable table");
-
         table.setContainerDataSource(createContainer());
         table.setImmediate(true);
 
diff --git a/uitest/src/main/java/com/vaadin/tests/components/table/CtrlShiftMultiselectTouchDetectionDisabled.java b/uitest/src/main/java/com/vaadin/tests/components/table/CtrlShiftMultiselectTouchDetectionDisabled.java
new file mode 100644
index 0000000000..9b410af276
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/table/CtrlShiftMultiselectTouchDetectionDisabled.java
@@ -0,0 +1,47 @@
+package com.vaadin.tests.components.table;
+
+import com.vaadin.data.Container;
+import com.vaadin.data.Item;
+import com.vaadin.data.Property;
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.Table.TableDragMode;
+
+import java.util.Set;
+
+@SuppressWarnings("serial")
+public class CtrlShiftMultiselectTouchDetectionDisabled extends CtrlShiftMultiselect {
+
+    protected Label label;
+
+    @Override
+    protected void setup() {
+        super.setup();
+        label = new Label("0");
+        label.setId("count");
+        label.setCaption("Amount of selected items");
+        table.setMultiSelectTouchDetectionEnabled(false);
+        table.addValueChangeListener(new Property.ValueChangeListener() {
+            @Override
+            public void valueChange(Property.ValueChangeEvent event) {
+                Property property = event.getProperty();
+                Set set = (Set) property.getValue();
+                label.setValue("" + set.size());
+            }
+        });
+        addComponent(label);
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Allow disabling multi selection's touch screen detection for hybrid devices";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11601;
+    }
+
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/components/treetable/TreeTableMultiselect.java b/uitest/src/main/java/com/vaadin/tests/components/treetable/TreeTableMultiselect.java
new file mode 100644
index 0000000000..7b19d29d2e
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/treetable/TreeTableMultiselect.java
@@ -0,0 +1,64 @@
+package com.vaadin.tests.components.treetable;
+
+import com.vaadin.data.Property;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TreeTable;
+
+import java.util.Set;
+
+import static com.vaadin.server.Sizeable.Unit.PIXELS;
+
+public class TreeTableMultiselect extends AbstractTestUI {
+
+    protected final TreeTable tt = new TreeTable("Multiselectable treetable");
+    protected Label label;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        label = new Label("0");
+        label.setCaption("Amount of selected items");
+        label.setId("count");
+
+        tt.setImmediate(true);
+        tt.addContainerProperty("Foo", String.class, "");
+        tt.setColumnWidth("Foo", 100);
+        tt.addContainerProperty("Bar", String.class, "");
+        tt.setColumnWidth("Bar", 100);
+        tt.setHeight(400, PIXELS);
+        Object item1 = tt.addItem(new Object[]{"Foo", "Bar"}, null);
+        Object item2 = tt.addItem(new Object[]{"Foo2", "Bar2"}, null);
+        Object item3 = tt.addItem(new Object[]{"Foo3", "Bar3"}, null);
+        tt.setParent(item2, item1);
+        tt.setParent(item3, item1);
+        tt.setCollapsed(item1, false);
+        tt.setSelectable(true);
+        tt.setMultiSelect(true);
+        tt.setMultiSelectTouchDetectionEnabled(false);
+        tt.setWidth("400px");
+        tt.setHeight("400px");
+        tt.addValueChangeListener(new Property.ValueChangeListener() {
+            @Override
+            public void valueChange(Property.ValueChangeEvent event) {
+                Property property = event.getProperty();
+                Set set = (Set) property.getValue();
+                label.setValue("" + set.size());
+            }
+        });
+
+        addComponent(tt);
+        addComponent(label);
+    }
+
+    @Override
+    public String getDescription() {
+        return "Allow disabling multi selection's touch screen detection for hybrid devices";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11601;
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxClosePopupRetainTextTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxClosePopupRetainTextTest.java
index 532b6929ae..d71d954868 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxClosePopupRetainTextTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxClosePopupRetainTextTest.java
@@ -34,7 +34,7 @@ public class ComboBoxClosePopupRetainTextTest extends MultiBrowserTest {
         ComboBoxElement cb = $(ComboBoxElement.class).first();
         cb.selectByText("Item 3");
         WebElement textbox = cb.findElement(By.vaadin("#textbox"));
-        textbox.clear();
+        cb.clear();
         textbox.sendKeys("I");
         cb.openPopup();
         // Entered value should remain in the text field even though the popup
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxMenuBarAutoopenTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxMenuBarAutoopenTest.java
index 98651437e0..bdb47da998 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxMenuBarAutoopenTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxMenuBarAutoopenTest.java
@@ -20,6 +20,7 @@ public class ComboboxMenuBarAutoopenTest extends MultiBrowserTest {
 
     @Test
     public void closeComboboxPopupOnClickToMenuBar() {
+        setDebug(true);
         openTestURL();
 
         openPopup();
diff --git a/uitest/src/test/java/com/vaadin/tests/components/table/CtrlShiftMultiselectTouchDetectionDisabledTest.java b/uitest/src/test/java/com/vaadin/tests/components/table/CtrlShiftMultiselectTouchDetectionDisabledTest.java
new file mode 100644
index 0000000000..485c257bd1
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/table/CtrlShiftMultiselectTouchDetectionDisabledTest.java
@@ -0,0 +1,46 @@
+package com.vaadin.tests.components.table;
+
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.testbench.elements.TableElement;
+import com.vaadin.testbench.parallel.Browser;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.interactions.Actions;
+import org.openqa.selenium.remote.DesiredCapabilities;
+
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+public class CtrlShiftMultiselectTouchDetectionDisabledTest extends MultiBrowserTest {
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        return getBrowserCapabilities(Browser.IE11);
+    }
+
+    @Override
+    protected boolean requireWindowFocusForIE() {
+        return true;
+    }
+
+    @Test
+    public void testSelectedCount() {
+        openTestURL();
+        clickRow(3);
+        new Actions(driver).keyDown(Keys.SHIFT).perform();
+        clickRow(8);
+        new Actions(driver).keyUp(Keys.SHIFT).perform();
+        new Actions(driver).release().perform();
+        LabelElement labelElement = $(LabelElement.class).id("count");
+        assertEquals("6", labelElement.getText());
+
+    }
+
+    private void clickRow(int index) {
+        TableElement tableElement = $(TableElement.class).first();
+        tableElement.getRow(index).click();
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/treetable/TreeTableMultiselectTest.java b/uitest/src/test/java/com/vaadin/tests/components/treetable/TreeTableMultiselectTest.java
new file mode 100644
index 0000000000..7ba8149189
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/treetable/TreeTableMultiselectTest.java
@@ -0,0 +1,46 @@
+package com.vaadin.tests.components.treetable;
+
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.testbench.elements.TreeTableElement;
+import com.vaadin.testbench.parallel.Browser;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.interactions.Actions;
+import org.openqa.selenium.remote.DesiredCapabilities;
+
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+public class TreeTableMultiselectTest extends MultiBrowserTest {
+
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        return getBrowserCapabilities(Browser.IE11);
+    }
+
+    @Override
+    protected boolean requireWindowFocusForIE() {
+        return true;
+    }
+
+    @Test
+    public void testSelectedCount() {
+        openTestURL();
+        clickRow(0);
+        new Actions(driver).keyDown(Keys.SHIFT).perform();
+        clickRow(1);
+        new Actions(driver).keyUp(Keys.SHIFT).perform();
+        new Actions(driver).release().perform();
+        LabelElement labelElement = $(LabelElement.class).id("count");
+        assertEquals("2", labelElement.getText());
+    }
+
+    private void clickRow(int index) {
+        TreeTableElement treeTable = $(TreeTableElement.class).first();
+        treeTable.getRow(index).click();
+    }
+
+}