Clicking on slider makes handler move (#11519)

* Clicking on slider makes handler move

Fixes #1496

* Introducing control over clicking behaviour

- User will have to enable process of the click event on handle calling `slider.setEnableClickHandler(true);`
- Clean-up the handler logic in VSlider.java
- Renaming the property to `updateValueOnClick`
- Added JavaDocs
- Fixing tests

6 files changed, 147 insertions, 8 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/VSlider.java b/client/src/main/java/com/vaadin/client/ui/VSlider.java
index de805ba423..2f3e56ae61 100644
--- a/client/src/main/java/com/vaadin/client/ui/VSlider.java
+++ b/client/src/main/java/com/vaadin/client/ui/VSlider.java
@@ -58,6 +58,8 @@ public class VSlider extends SimpleFocusablePanel
     protected double max;
     protected int resolution;
     protected Double value;
+
+    private boolean updateValueOnClick;
     protected SliderOrientation orientation = SliderOrientation.HORIZONTAL;
 
     private final HTML feedback = new HTML("", false);
@@ -266,12 +268,15 @@ public class VSlider extends SimpleFocusablePanel
             processMouseWheelEvent(event);
         } else if (dragging || targ == handle) {
             processHandleEvent(event);
+        } else if (targ.equals(base)
+                && DOM.eventGetType(event) == Event.ONMOUSEUP
+                && updateValueOnClick) {
+            processBaseEvent(event);
+            feedbackPopup.show();
         } else if (targ == smaller) {
             decreaseValue(true);
         } else if (targ == bigger) {
             increaseValue(true);
-        } else if (DOM.eventGetType(event) == Event.MOUSEEVENTS) {
-            processBaseEvent(event);
         } else if (isNavigationEvent(event)) {
 
             if (handleNavigation(event.getKeyCode(), event.getCtrlKey(),
@@ -300,7 +305,8 @@ public class VSlider extends SimpleFocusablePanel
     }
 
     private boolean isNavigationEvent(Event event) {
-        if (BrowserInfo.get().isGecko() && BrowserInfo.get().getGeckoVersion() < 65) {
+        if (BrowserInfo.get().isGecko()
+                && BrowserInfo.get().getGeckoVersion() < 65) {
             return DOM.eventGetType(event) == Event.ONKEYPRESS;
         } else {
             return DOM.eventGetType(event) == Event.ONKEYDOWN;
@@ -363,11 +369,9 @@ public class VSlider extends SimpleFocusablePanel
     }
 
     private void processBaseEvent(Event event) {
-        if (DOM.eventGetType(event) == Event.ONMOUSEDOWN) {
-            if (!disabled && !readonly && !dragging) {
-                setValueByEvent(event, true);
-                DOM.eventCancelBubble(event, true);
-            }
+        if (!disabled && !readonly && !dragging) {
+            setValueByEvent(event, true);
+            DOM.eventCancelBubble(event, true);
         }
     }
 
@@ -671,4 +675,13 @@ public class VSlider extends SimpleFocusablePanel
         }
         return null;
     }
+
+    /**
+     * Specifies whether or not click event should update the Slider's value.
+     *
+     * @param updateValueOnClick
+     */
+    public void setUpdateValueOnClick(boolean updateValueOnClick) {
+        this.updateValueOnClick = updateValueOnClick;
+    }
 }
diff --git a/client/src/main/java/com/vaadin/client/ui/slider/SliderConnector.java b/client/src/main/java/com/vaadin/client/ui/slider/SliderConnector.java
index a5be9b3a2d..f5f220f43f 100644
--- a/client/src/main/java/com/vaadin/client/ui/slider/SliderConnector.java
+++ b/client/src/main/java/com/vaadin/client/ui/slider/SliderConnector.java
@@ -82,6 +82,7 @@ public class SliderConnector extends AbstractFieldConnector
         getWidget().setMaxValue(getState().maxValue);
         getWidget().setResolution(getState().resolution);
         getWidget().setValue(getState().value, false);
+        getWidget().setUpdateValueOnClick(getState().updateValueOnClick);
 
         getWidget().buildBase();
         getWidget().setTabIndex(getState().tabIndex);
diff --git a/server/src/main/java/com/vaadin/ui/Slider.java b/server/src/main/java/com/vaadin/ui/Slider.java
index c488ecca52..2c1ec4afa1 100644
--- a/server/src/main/java/com/vaadin/ui/Slider.java
+++ b/server/src/main/java/com/vaadin/ui/Slider.java
@@ -258,6 +258,31 @@ public class Slider extends AbstractField<Double> {
         getState().resolution = resolution;
     }
 
+    /**
+     * Sets the slider to update its value when the user clicks on it.
+     * By default, the slider value is updated by dragging the slider's handle
+     * or clicking arrows.
+     *
+     * @param updateValueOnClick
+     *            {@code true} to update the value of the slider on click,
+     *            {@code false} otherwise.
+     * @since
+     */
+    public void setUpdateValueOnClick(boolean updateValueOnClick) {
+        getState().updateValueOnClick = updateValueOnClick;
+    }
+
+    /**
+     * Returns whether the slider updates its value on user click.
+     *
+     * @return {@code true} if the Slider updates its value on click. By
+     *         default, returns {@code false}
+     * @since
+     */
+    public boolean isUpdateValueOnClick() {
+        return getState(false).updateValueOnClick;
+    }
+
     private double getRoundedValue(Double value) {
         final double v = value.doubleValue();
         final int resolution = getResolution();
diff --git a/shared/src/main/java/com/vaadin/shared/ui/slider/SliderState.java b/shared/src/main/java/com/vaadin/shared/ui/slider/SliderState.java
index 1749e6afe2..6787dc4d53 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/slider/SliderState.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/slider/SliderState.java
@@ -38,6 +38,9 @@ public class SliderState extends AbstractFieldState {
     @NoLayout
     public int resolution = 0;
 
+    @NoLayout
+    public boolean updateValueOnClick = false;
+
     public SliderOrientation orientation = SliderOrientation.HORIZONTAL;
 
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/slider/SliderHandleBaseClick.java b/uitest/src/main/java/com/vaadin/tests/components/slider/SliderHandleBaseClick.java
new file mode 100644
index 0000000000..837f22056a
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/slider/SliderHandleBaseClick.java
@@ -0,0 +1,38 @@
+package com.vaadin.tests.components.slider;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Slider;
+import com.vaadin.ui.Button;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class SliderHandleBaseClick extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Slider slider = new Slider();
+        slider.setWidth(500, Unit.PIXELS);
+        slider.setMin(0);
+        slider.setMax(10);
+        slider.setValue(3.0);
+        slider.setUpdateValueOnClick(true);
+        addComponent(slider);
+        Button toggleHandling = new Button("Apply/remove click action", e -> {
+            slider.setUpdateValueOnClick(!slider.isUpdateValueOnClick());
+        });
+        toggleHandling.setId("toggleHandling");
+        addComponent(toggleHandling);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Slider should update its value, when clicked on the base";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 1496;
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/slider/SliderHandleBaseClickTest.java b/uitest/src/test/java/com/vaadin/tests/components/slider/SliderHandleBaseClickTest.java
new file mode 100644
index 0000000000..8de82a590f
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/slider/SliderHandleBaseClickTest.java
@@ -0,0 +1,59 @@
+package com.vaadin.tests.components.slider;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Before;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.Actions;
+
+import static org.junit.Assert.assertEquals;
+
+public class SliderHandleBaseClickTest extends MultiBrowserTest {
+    private WebElement base;
+    private int offsetStep;
+
+    @Before
+    public void setUp() throws Exception {
+        super.setup();
+        openTestURL();
+        base = findElement(By.className("v-slider-base"));
+        offsetStep = base.getSize().getWidth() / 10;
+    }
+
+    @Test
+    public void testHandlerHasMoved() {
+        // dragAndDropBy function starts calculating click position from the
+        // middle of the component .
+        // So the click will always be at the position (center + offsetStep)px
+
+        // Will move by one from the middle
+        new Actions(driver).dragAndDropBy(base, offsetStep, 0).perform();
+        sleep(100);
+        assertEquals("The Slider value should be 6 after moving by one offset",
+                6, getSliderValue(), 0);
+        // Will move by two from the middle, in this case from 5
+        new Actions(driver).dragAndDropBy(base, offsetStep * 2, 0).perform();
+        sleep(100);
+        assertEquals("The Slider value should be 7 after moving by two offsets",
+                7, getSliderValue(), 0);
+    }
+
+    private double getSliderValue() {
+        return Double.valueOf(
+                findElement(By.className("v-slider-feedback")).getText());
+    }
+
+    @Test
+    public void testHandlerNotMoved() {
+        // Disable click event handling
+        findElement(By.id("toggleHandling")).click();
+        new Actions(driver).dragAndDropBy(base, offsetStep, 0).perform();
+        sleep(100);
+        assertEquals(String.format(
+                "Slider value should not have changed. Expected 3.0 , but was %f",
+                getSliderValue()), 3.0,getSliderValue(), 0.0);
+        // Enable click event handling
+        findElement(By.id("toggleHandling")).click();
+    }
+}