slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
16.93k stars 566 forks source link

two way binding to struct fields #556

Closed tronical closed 1 year ago

tronical commented 2 years ago

Modify the Todo example like this:

diff --git a/examples/todo/cpp/main.cpp b/examples/todo/cpp/main.cpp
index bc614ca8..d203f353 100644
--- a/examples/todo/cpp/main.cpp
+++ b/examples/todo/cpp/main.cpp
@@ -30,13 +26,10 @@ int main()
     });

     demo->on_remove_done([todo_model] {
-        int offset = 0;
-        int count = todo_model->row_count();
-        for (int i = 0; i < count; ++i) {
-            if (todo_model->row_data(i - offset).checked) {
-                todo_model->erase(i - offset);
-                offset += 1;
-            }
+        for (auto i = 0; i < todo_model->row_count(); ++i) {
+            auto todo = todo_model->row_data(i);
+            todo.checked = true;
+            todo_model->set_row_data(i, todo);
         }
     });

diff --git a/examples/todo/rust/main.rs b/examples/todo/rust/main.rs
index b29ef759..9738caf6 100644
--- a/examples/todo/rust/main.rs
+++ b/examples/todo/rust/main.rs
@@ -42,12 +42,10 @@ pub fn main() {
     main_window.on_remove_done({
         let todo_model = todo_model.clone();
         move || {
-            let mut offset = 0;
             for i in 0..todo_model.row_count() {
-                if todo_model.row_data(i - offset).checked {
-                    todo_model.remove(i - offset);
-                    offset += 1;
-                }
+                let mut bulb = todo_model.row_data(i);
+                bulb.checked = true;
+                todo_model.set_row_data(i, bulb);
             }
         }
     });

Then when running, pressing the "remove all done" button will toggle all todos. That works the first time. Then turn off a few toggles in the UI and press the "remove all done" button again.

Expected Result: All todos are checked again. Actual Result: Nothing changes in the UI.

tronical commented 2 years ago

At the heart of the issue lies that the binding for the checked property is destroyed:

CheckBox {
    text: todo.title;
    checked: todo.checked;
    toggled => {
        todo.checked = checked;
    }
}

Initially the binding checked: todo.checked; correctly fetches the data out of the model (model_data property). But when the user clicks on the CheckBox, the style runs this code in the TouchArea:

touch := TouchArea {
        clicked => {
            if (root.enabled) {
                root.checked = !root.checked;
                root.toggled();
            }
        }
    }

and the assignment to root.checked destroys the data model binding.

ogoffart commented 2 years ago

We would somehow need to support two ways binding with model properties this is not easy because the runtime really expect properties so they can be merged into one property.

tronical commented 2 years ago

I've begun prototyping an approach that tries to detect two-way bindings in model data structures, synthesise a property for it and update it in the model update function.

Diagnostics wise it might be interesting to also have a pass that tries to detect cases where we know that a property has a binding and is written to somewhere else - in order to issue a warning that perhaps a two-way binding is desired.

ogoffart commented 2 years ago

Diagnostics wise it might be interesting to also have a pass that tries to detect cases where we know that a property has a binding and is written to somewhere else

I think we really need to be able to declare property are meant to be "readonly" or something (like the pressed property of a TouchArea) or read-write (like the checked property of a checkbox) or meant to be set like the background property for example. Issue https://github.com/sixtyfpsui/sixtyfps/issues/191

For example the compiler can't know if the NativeCheckBox assignthe checked property. However we already annotate some property as being set. But it'd be better to be exlicit about it i think.

tronical commented 2 years ago

We discussed the two-way bindings separately a bit further and with regards to the model we could implement tow-way bindings for model data by using the existing callback functionality of struct PropertyTracker<ChangeHandler = ()> in generated code to write changes back into the model.

levrik commented 2 years ago

@ogoffart I was just hit by this as well. Is there some suggested workaround for now? I can't control the checked property of a CheckBox from Rust anymore as soon as it was toggled once which basically makes it impossible to implement my desired UX.

ogoffart commented 2 years ago

Is there some suggested workaround for now?

It depends a bit of what you're doing. But we can use a workaround similar to the missing changed event https://github.com/slint-ui/slint/issues/112#issuecomment-1065866865

      CheckBox {
            text: {
                checked = the_property;    // <- Ugly workaround right there        
                " Check me ";
            }
            checked: the_property;
            toggled => {
                the_property = checked;
            }
        }

code editor link of example

This works because the text property is evaluated (indirectly) by the rendering engine. And it depends on the the_property and do side effect in a binding (which is supposed to be pure, but nobody is checking).

levrik commented 2 years ago

@ogoffart I see. I worked around by using my local fork and slightly modifying CheckBoxs implementation. I could propose this change in a PR when I have a free moment but it might be a breaking change so unsure about it.

ogoffart commented 1 year ago

Closing this as a duplicate of https://github.com/slint-ui/slint/issues/814