Fix the default Text color overriding a color specified in a two way …

…binding We just need to adjust the priority of the default binding to be a high value (eg, less priority) since the other values must always win. This fixes the placeholder text color
slint-ui · Sep 30, 2021 · fd435ec · fd435ec
1 parent 465857c
commit fd435ec
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 31 deletions.
diff --git a/sixtyfps_compiler/object_tree.rs b/sixtyfps_compiler/object_tree.rs
@@ -394,14 +394,18 @@ impl BindingsMap {
     pub fn set_binding_if_not_set(
         &mut self,
         property_name: String,
-        expression_fn: impl FnOnce() -> BindingExpression,
+        expression_fn: impl FnOnce() -> Expression,
     ) {
         match self.0.entry(property_name) {
             Entry::Vacant(vacant_entry) => {
-                vacant_entry.insert(expression_fn());
+                let mut binding: BindingExpression = expression_fn().into();
+                binding.priority = i32::MAX;
+                vacant_entry.insert(binding);
             }
             Entry::Occupied(mut existing_entry) if !existing_entry.get().has_binding() => {
-                existing_entry.get_mut().merge_with(&expression_fn());
+                let mut binding: BindingExpression = expression_fn().into();
+                binding.priority = i32::MAX;
+                existing_entry.get_mut().merge_with(&binding);
             }
             _ => {}
         };

diff --git a/sixtyfps_compiler/passes/apply_default_properties_from_style.rs b/sixtyfps_compiler/passes/apply_default_properties_from_style.rs
@@ -42,31 +42,24 @@ pub async fn apply_default_properties_from_style(
                             &style_metrics.root_element,
                             "text-cursor-width",
                         ))
-                        .into()
                     });
-                    elem.bindings.set_binding_if_not_set("color".into(), || {
-                        Expression::Cast {
-                            from: Expression::PropertyReference(NamedReference::new(
-                                &style_metrics.root_element,
-                                "default-text-color",
-                            ))
-                            .into(),
-                            to: Type::Brush,
-                        }
-                        .into()
+                    elem.bindings.set_binding_if_not_set("color".into(), || Expression::Cast {
+                        from: Expression::PropertyReference(NamedReference::new(
+                            &style_metrics.root_element,
+                            "default-text-color",
+                        ))
+                        .into(),
+                        to: Type::Brush,
                     });
                 }
                 "Text" => {
-                    elem.bindings.set_binding_if_not_set("color".into(), || {
-                        Expression::Cast {
-                            from: Expression::PropertyReference(NamedReference::new(
-                                &style_metrics.root_element,
-                                "default-text-color",
-                            ))
-                            .into(),
-                            to: Type::Brush,
-                        }
-                        .into()
+                    elem.bindings.set_binding_if_not_set("color".into(), || Expression::Cast {
+                        from: Expression::PropertyReference(NamedReference::new(
+                            &style_metrics.root_element,
+                            "default-text-color",
+                        ))
+                        .into(),
+                        to: Type::Brush,
                     });
                 }
                 "Dialog" | "Window" | "WindowItem" => {
@@ -75,7 +68,6 @@ pub async fn apply_default_properties_from_style(
                             &style_metrics.root_element,
                             "window-background",
                         ))
-                        .into()
                     });
                 }
 

diff --git a/sixtyfps_compiler/passes/default_geometry.rs b/sixtyfps_compiler/passes/default_geometry.rs
@@ -97,7 +97,6 @@ pub fn default_geometry(root_component: &Rc<Component>, diag: &mut BuildDiagnost
                                                 .try_value_from_string("contain")
                                                 .unwrap(),
                                         )
-                                        .into()
                                     },
                                 );
                             }
@@ -208,14 +207,13 @@ fn make_default_100(elem: &ElementRc, parent_element: &ElementRc, property: &str
     }
     elem.borrow_mut().bindings.set_binding_if_not_set(resolved_name.to_string(), || {
         Expression::PropertyReference(NamedReference::new(parent_element, resolved_name.as_ref()))
-            .into()
     });
 }
 
 fn make_default_implicit(elem: &ElementRc, property: &str, orientation: Orientation) {
     let base = crate::layout::implicit_layout_info_call(elem, orientation).into();
     elem.borrow_mut().bindings.set_binding_if_not_set(property.into(), || {
-        Expression::StructFieldAccess { base, name: "preferred".into() }.into()
+        Expression::StructFieldAccess { base, name: "preferred".into() }
     });
 }
 

diff --git a/tests/cases/text/default_color.60 b/tests/cases/text/default_color.60
@@ -10,19 +10,28 @@ LICENSE END */
 
 import { StyleMetrics } from "sixtyfps_widgets.60";
 
+SubElement := Rectangle {
+    property sub_color <=> sub.color;
+    sub := Text { text: "sub"; }
+}
+
 TestCase := Rectangle {
     // This binding allow the test to set the style's default color so that we can compare it
     property<color> binding_to_default_text_color <=> StyleMetrics.default-text-color;
 
-    default_text := Text {
-    }
+    default_text := Text { text: "default"; }
     text_with_color := Text {
+        text: "yellow";
         color: #ffff00ff;
     }
+    text_in_sub_element := SubElement { sub_color: #ff0000; }
+
     property <color> default_text_color: default_text.color;
     property <color> color_of_initialized_text: text_with_color.color;
+    property <color> color_of_sub_element_text: text_in_sub_element.sub_color;
 
-    property <bool> test: default_text_color == StyleMetrics.default-text-color && color-of-initialized-text == #ffff00ff;
+    property <bool> test: default_text_color == StyleMetrics.default-text-color && color-of-initialized-text == #ffff00ff
+        && color_of_sub_element_text == #ff0000;
 }
 
 
@@ -34,13 +43,15 @@ const TestCase &instance = *handle;
 instance.set_binding_to_default_text_color(sixtyfps::Color::from_rgb_uint8(0, 0, 255));
 assert_eq(instance.get_default_text_color(), sixtyfps::Color::from_rgb_uint8(0, 0, 255));
 assert_eq(instance.get_color_of_initialized_text(), sixtyfps::Color::from_rgb_uint8(255, 255, 0));
+assert_eq(instance.get_color_of_sub_element_text(), sixtyfps::Color::from_rgb_uint8(255, 0, 0));
 ```
 
 ```rust
 let instance = TestCase::new();
 instance.set_binding_to_default_text_color(sixtyfps::Color::from_rgb_u8(0, 0, 133));
 assert_eq!(instance.get_default_text_color(), sixtyfps::Color::from_rgb_u8(0, 0, 133));
 assert_eq!(instance.get_color_of_initialized_text(), sixtyfps::Color::from_rgb_u8(255, 255, 0));
+assert_eq!(instance.get_color_of_sub_element_text(), sixtyfps::Color::from_rgb_uint8(255, 0, 0));
 ```
 
 */