fix: set style property priority using proper API method (#12019)

fixes #11981 Co-authored-by: Denis Anisimov <denis@vaadin.com> Co-authored-by: Pekka Hyvönen <pekka@vaadin.com>
vaadin · Nov 4, 2021 · 24b8063 · 24b8063
1 parent 6b885bd
commit 24b8063
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 14 deletions.
diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java
@@ -22,6 +22,7 @@
 
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.Scheduler;
+
 import com.vaadin.client.ApplicationConfiguration;
 import com.vaadin.client.Command;
 import com.vaadin.client.Console;
@@ -751,7 +752,24 @@ private void updateStyleProperty(MapProperty mapProperty, Element element) {
         String name = mapProperty.getName();
         CSSStyleDeclaration styleElement = element.getStyle();
         if (mapProperty.hasValue()) {
-            styleElement.setProperty(name, (String) mapProperty.getValue());
+            String value = (String) mapProperty.getValue();
+            boolean styleIsSet = false;
+            if (value.contains("!important")) {
+                Element temp = Browser.getDocument()
+                        .createElement(element.getTagName());
+                CSSStyleDeclaration tmpStyle = temp.getStyle();
+                tmpStyle.setCssText(name + ": " + value + ";");
+                String priority = "important";
+                if (priority
+                        .equals(temp.getStyle().getPropertyPriority(name))) {
+                    styleElement.setProperty(name,
+                            temp.getStyle().getPropertyValue(name), priority);
+                    styleIsSet = true;
+                }
+            }
+            if (!styleIsSet) {
+                styleElement.setProperty(name, value);
+            }
         } else {
             styleElement.removeProperty(name);
         }

diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java
@@ -80,11 +80,11 @@ public void testBindExistingProperty() {
     }
 
     public void testBindExistingPropertyWithDifferentType() {
-        //set number as a property value for DOM elememnt
-        double value= setNumberValue(element, "bar");
-        
+        // set number as a property value for DOM elememnt
+        double value = setNumberValue(element, "bar");
+
         // set string as a StateTree property value
-        properties.getProperty("bar").setValue(""+value);
+        properties.getProperty("bar").setValue("" + value);
 
         Binder.bind(node, element);
 
@@ -818,11 +818,24 @@ public void testAddStylesBeforeBind() {
         node.getMap(NodeFeatures.ELEMENT_STYLE_PROPERTIES).getProperty("color")
                 .setValue("green");
 
-        Reactive.flush();
-        Binder.bind(node, element);
+        node.getMap(NodeFeatures.ELEMENT_STYLE_PROPERTIES)
+                .getProperty("display").setValue("none !important");
 
+        node.getMap(NodeFeatures.ELEMENT_STYLE_PROPERTIES)
+                .getProperty("background-color").setValue("!importantfoo");
+
+        Binder.bind(node, element);
         Reactive.flush();
+
         assertEquals("green", element.getStyle().getColor());
+
+        assertEquals("none", element.getStyle().getDisplay());
+        assertEquals("important",
+                element.getStyle().getPropertyPriority("display"));
+
+        assertEquals("!importantfoo", element.getStyle().getBackgroundColor());
+        assertEquals("",
+                element.getStyle().getPropertyPriority("background-color"));
     }
 
     public void testAddStylesAfterBind() {
@@ -831,8 +844,22 @@ public void testAddStylesAfterBind() {
         node.getMap(NodeFeatures.ELEMENT_STYLE_PROPERTIES).getProperty("color")
                 .setValue("green");
 
+        node.getMap(NodeFeatures.ELEMENT_STYLE_PROPERTIES)
+                .getProperty("display").setValue("none !important");
+
+        node.getMap(NodeFeatures.ELEMENT_STYLE_PROPERTIES)
+                .getProperty("background-color").setValue("!importantfoo");
+
         Reactive.flush();
         assertEquals("green", element.getStyle().getColor());
+
+        assertEquals("none", element.getStyle().getDisplay());
+        assertEquals("important",
+                element.getStyle().getPropertyPriority("display"));
+
+        assertEquals("!importantfoo", element.getStyle().getBackgroundColor());
+        assertEquals("",
+                element.getStyle().getPropertyPriority("background-color"));
     }
 
     public void testRemoveStyles() {
@@ -844,27 +871,33 @@ public void testRemoveStyles() {
         styleMap.getProperty("color").setValue("white");
 
         Reactive.flush();
-        assertEquals("background: blue;color: white;",
-                element.getAttribute("style"));
+        assertEquals("blue", element.getStyle().getPropertyValue("background"));
+        assertEquals("white", element.getStyle().getColor());
 
         styleMap.getProperty("color").removeValue();
 
         Reactive.flush();
-        assertEquals("background: blue;", element.getAttribute("style"));
+        assertEquals("blue", element.getStyle().getPropertyValue("background"));
+        assertEquals("", element.getStyle().getColor());
     }
 
     private native void polyfillStyleSetProperty(Element element)
     /*-{
          // This polyfills just enough to make the tests pass and nothing else
-         element.style.__proto__.setProperty = function(key,value) {
+         element.style.__proto__.setProperty = function(key,value, priority) {
              var newValue = element.getAttribute("style");
              if (!newValue) {
                  newValue = "";
              }
              else if (!newValue.endsWith(";")) {
                  newValue +=";"
              }
-             element.setAttribute("style", newValue + key+": "+value+";");
+             if ( priority ){
+                 element.setAttribute("style", newValue + key+": "+value+" !"+priority+";");
+             }
+             else {
+                 element.setAttribute("style", newValue + key+": "+value+";");
+             }
          };
      }-*/;
 
@@ -883,7 +916,7 @@ public void testAddStylesAfterUnbind() {
         styleMap.getProperty("font-size").setValue("12px");
 
         Reactive.flush();
-        assertEquals("color: red;", element.getAttribute("style"));
+        assertEquals("red", element.getStyle().getColor());
     }
 
     public void testAttachExistingElement() {
@@ -1975,7 +2008,7 @@ private native String getPropertyType(Object obj, String name)
     /*-{
        return typeof obj[name];
     }-*/;
-    
+
     private native double setNumberValue(Object obj, String name)
     /*-{
        obj[name] =2;

diff --git a/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/StylePriorityView.java b/flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/StylePriorityView.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2000-2021 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.flow.uitest.ui;
+
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.router.Route;
+
+@Route("com.vaadin.flow.uitest.ui.StylePriorityView")
+public class StylePriorityView extends Div {
+
+    public StylePriorityView() {
+        Div div = new Div();
+        div.getElement().getStyle().set("display", "block !important");
+        div.setText("Priority style");
+        div.setId("priority-style");
+        add(div);
+    }
+
+}
diff --git a/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/StylePriorityIT.java b/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/StylePriorityIT.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2000-2021 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.flow.uitest.ui;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.flow.testutil.ChromeBrowserTest;
+
+public class StylePriorityIT extends ChromeBrowserTest {
+
+    @Test
+    public void noParameters() {
+        open();
+        WebElement div = findElement(By.id("priority-style"));
+
+        Assert.assertEquals("display: block !important;",
+                div.getAttribute("style"));
+    }
+}