Fix incorrect color value handling in ColorPicker (#10812)

client/src/main/java/com/vaadin/client/ui/VColorPicker.java

@@ -47,6 +47,16 @@ public class VColorPicker extends VButton {
         this.color = color;
     }
 
+    /**
+     * Gets the color.
+     *
+     * @since
+     * @return the color
+     */
+    public String getColor() {
+        return color;
+    }
+
     /**
      * Mark the popup opened/closed.
      *

client/src/main/java/com/vaadin/client/ui/VColorPickerArea.java

@@ -77,6 +77,7 @@ public class VColorPickerArea extends Widget
     @Override
     public void onClick(ClickEvent event) {
         setOpen(!isOpen);
+        refreshColor();
     }
 
     @Override
@@ -163,6 +164,16 @@ public class VColorPickerArea extends Widget
         this.color = color;
     }
 
+    /**
+     * Gets the color.
+     *
+     * @since
+     * @return the color
+     */
+    public String getColor() {
+        return color;
+    }
+
     /**
      * Update the color area with the currently set color.
      */

client/src/main/java/com/vaadin/client/ui/colorpicker/ColorPickerConnector.java

@@ -66,6 +66,7 @@ public class ColorPickerConnector extends AbstractColorPickerConnector {
     @Override
     protected void refreshColor() {
         getWidget().refreshColor();
+        rpc.changeColor(getWidget().getColor());
     }
 
     @Override

client/src/main/java/com/vaadin/client/ui/colorpicker/VColorPickerGradient.java

@@ -176,8 +176,9 @@ public class VColorPickerGradient extends FocusPanel implements
         }
         if (y >= 0) {
             lowercross.getElement().getStyle().setHeight(height - y, Unit.PX);
+        } else {
+            lowercross.getElement().getStyle().setHeight(Math.abs(y), Unit.PX);
         }
-
         if (x >= 0) {
             highercross.getElement().getStyle().setWidth(width - x, Unit.PX);
         }
@@ -186,6 +187,8 @@ public class VColorPickerGradient extends FocusPanel implements
         }
         if (y >= 0) {
             highercross.getElement().getStyle().setHeight(y, Unit.PX);
+        } else {
+            highercross.getElement().getStyle().setHeight(height + y, Unit.PX);
         }
     }
 

server/src/main/java/com/vaadin/ui/AbstractColorPicker.java

@@ -88,8 +88,6 @@ public abstract class AbstractColorPicker extends AbstractField<Color> {
         }
     }
 
-    private ColorPickerServerRpc rpc = this::showPopup;
-
     protected static final String STYLENAME_DEFAULT = "v-colorpicker";
     protected static final String STYLENAME_BUTTON = "v-button";
     protected static final String STYLENAME_AREA = "v-colorpicker-area";
@@ -113,6 +111,21 @@ public abstract class AbstractColorPicker extends AbstractField<Color> {
     protected boolean historyVisible = true;
     protected boolean textfieldVisible = true;
 
+    private ColorPickerServerRpc rpc = new ColorPickerServerRpc() {
+        @Override
+        public void openPopup(boolean openPopup) {
+            showPopup(openPopup);
+        }
+
+        @Override
+        public void changeColor(String col) {
+            Color valueC = new Color(
+                    Integer.parseInt(col.substring(1, col.length()), 16));
+            color = valueC;
+            setValue(color,true);
+        }
+    };
+
     /**
      * Instantiates a new color picker.
      */
@@ -170,6 +183,9 @@ public abstract class AbstractColorPicker extends AbstractField<Color> {
     public void setValue(Color color) {
         Objects.requireNonNull(color, "color cannot be null");
         super.setValue(color);
+        if (window != null) {
+            window.setValue(color);
+        }
     }
 
     /**
@@ -446,13 +462,15 @@ public abstract class AbstractColorPicker extends AbstractField<Color> {
 
                 window.addCloseListener(
                         event -> getState().popupVisible = false);
-                window.addValueChangeListener(
-                        event -> setValue(event.getValue()));
-
+                window.addValueChangeListener(event -> {
+                    setValue(event.getValue());
+                    rpc.changeColor(event.getValue().getCSS());
+                });
                 window.getHistory().setValue(color);
                 window.setPositionX(positionX);
                 window.setPositionY(positionY);
                 window.setVisible(true);
+                window.setValue(color);
 
                 parent.addWindow(window);
                 window.focus();
@@ -472,6 +490,7 @@ public abstract class AbstractColorPicker extends AbstractField<Color> {
                 parent.addWindow(window);
                 window.focus();
             }
+            window.setValue(color);
 
         } else if (window != null) {
             window.setVisible(false);

server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java

@@ -28,19 +28,19 @@ import com.vaadin.data.HasValue;
 import com.vaadin.shared.Registration;
 import com.vaadin.shared.ui.MarginInfo;
 import com.vaadin.shared.ui.colorpicker.Color;
-import com.vaadin.ui.AbstractColorPicker.Coordinates2Color;
+import com.vaadin.ui.TabSheet;
+import com.vaadin.ui.Window;
+import com.vaadin.ui.VerticalLayout;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.Slider;
+import com.vaadin.ui.Layout;
 import com.vaadin.ui.Alignment;
 import com.vaadin.ui.Button;
-import com.vaadin.ui.Button.ClickEvent;
-import com.vaadin.ui.Component;
 import com.vaadin.ui.HasComponents;
-import com.vaadin.ui.HorizontalLayout;
-import com.vaadin.ui.Layout;
-import com.vaadin.ui.Slider;
+import com.vaadin.ui.AbstractColorPicker.Coordinates2Color;
+import com.vaadin.ui.Button.ClickEvent;
 import com.vaadin.ui.Slider.ValueOutOfBoundsException;
-import com.vaadin.ui.TabSheet;
-import com.vaadin.ui.VerticalLayout;
-import com.vaadin.ui.Window;
 
 /**
  * A component that represents color selection popup within a color picker.
@@ -160,8 +160,8 @@ public class ColorPickerPopup extends Window implements HasValue<Color> {
      */
     public ColorPickerPopup(Color initialColor) {
         this();
-        selectedColor = initialColor;
         initContents();
+        setValue(initialColor);
     }
 
     private void initContents() {
@@ -248,6 +248,7 @@ public class ColorPickerPopup extends Window implements HasValue<Color> {
         buttons.setComponentAlignment(ok, Alignment.MIDDLE_CENTER);
         buttons.setComponentAlignment(cancel, Alignment.MIDDLE_CENTER);
         layout.addComponent(buttons);
+        setRgbSliderValues(selectedColor);
     }
 
     /**
@@ -451,6 +452,9 @@ public class ColorPickerPopup extends Window implements HasValue<Color> {
 
     private void okButtonClick(ClickEvent event) {
         fireEvent(new ValueChangeEvent<>(this, previouslySelectedColor, true));
+        rgbPreview.setValue(getValue());
+        hsvPreview.setValue(getValue());
+        selPreview.setValue(getValue());
         close();
     }
 
@@ -516,6 +520,7 @@ public class ColorPickerPopup extends Window implements HasValue<Color> {
     }
 
     private void colorChanged(ValueChangeEvent<Color> event) {
+        event.getSource().setValue(event.getValue());
         setValue(event.getValue());
 
         updatingColors = true;

shared/src/main/java/com/vaadin/shared/ui/colorpicker/ColorPickerServerRpc.java

@@ -33,4 +33,12 @@ public interface ColorPickerServerRpc extends ServerRpc {
      */
     public void openPopup(boolean openPopup);
 
+    /**
+     * ColorPicker's selected color is changed
+     *
+     * @since
+     * @param color
+     */
+    public void changeColor(String color);
+
 }

uitest/src/main/java/com/vaadin/tests/components/colorpicker/ColorPickerTestUI.java

@@ -17,7 +17,7 @@ import com.vaadin.server.StreamResource;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.shared.ui.ContentMode;
 import com.vaadin.shared.ui.colorpicker.Color;
-import com.vaadin.tests.components.AbstractReindeerTestUI;
+import com.vaadin.tests.components.AbstractTestUI;
 import com.vaadin.ui.AbstractColorPicker;
 import com.vaadin.ui.Alignment;
 import com.vaadin.ui.CheckBox;
@@ -31,7 +31,7 @@ import com.vaadin.ui.Panel;
 import com.vaadin.ui.VerticalLayout;
 
 @Widgetset("com.vaadin.DefaultWidgetSet")
-public class ColorPickerTestUI extends AbstractReindeerTestUI {
+public class ColorPickerTestUI extends AbstractTestUI {
 
     @Override
     public String getTestDescription() {

uitest/src/main/java/com/vaadin/tests/components/colorpicker/ColorPickerValueTest.java

@@ -0,0 +1,31 @@
+package com.vaadin.tests.components.colorpicker;
+
+import com.vaadin.annotations.Widgetset;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.colorpicker.Color;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.AbstractColorPicker;
+
+import com.vaadin.ui.Button;
+import com.vaadin.ui.ColorPicker;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class ColorPickerValueTest extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        AbstractColorPicker colorpicker = new ColorPicker("ColorPicker",
+                Color.GREEN);
+        colorpicker.setPosition(250, 0);
+        colorpicker.setId("clp");
+        colorpicker.setRGBVisibility(true);
+        colorpicker.setHSVVisibility(true);
+        colorpicker.setTextfieldVisibility(true);
+        colorpicker.setSwatchesVisibility(true);
+        addComponent(colorpicker);
+        addComponent(new Button("Change Color", e -> {
+            colorpicker.setValue(Color.YELLOW);
+        }));
+    }
+}

uitest/src/test/java/com/vaadin/tests/components/colorpicker/ColorPickerValueTestTest.java

@@ -0,0 +1,33 @@
+package com.vaadin.tests.components.colorpicker;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.ColorPickerElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+
+import static org.junit.Assert.assertEquals;
+
+public class ColorPickerValueTestTest extends MultiBrowserTest {
+
+    @Test
+    public void testValue() throws Exception {
+        openTestURL();
+
+        // Open ColorPicker
+        findElement(By.id("clp")).click();
+        // Click Button to change color
+        ButtonElement button = $(ButtonElement.class).first();
+        button.click();
+        Thread.sleep(300);
+        assertEquals("#ffff00", getColorpickerValue());
+    }
+
+    private String getColorpickerValue() {
+        WebElement field = findElement(
+                By.className("v-colorpicker-preview-textfield"));
+        return field.getAttribute("value");
+    }
+}