Conversation
ogoffart
force-pushed
the
olivier/two_way_bindings
branch
from
cf14c0c to
8b34b8d
Compare
tronical
left a comment
tronical
left a comment
There was a problem hiding this comment.
Very nice series :). Some comments inside. My main criticism is that documentation wise this is poorly documented / advertised :)
|
|
||
| The right hand side of the `<=>` must be a reference to a property of the same type, | ||
| or a field of the same type within a property of struct type. | ||
| Inside a `for` loop, it can also reference the current row, which lets widgets write |
There was a problem hiding this comment.
Deep inside the guide I find it a little odd to refer to widgets and basically let that be the documentation for a key feature. One could argue that it just works as expected, but I'd prefer if a paragraph was dedicated to this feature. When is it used? (When reading from a model and wanting to write changes back after for example the user made changes) How is it used? (Two way binding)
Perhaps this belongs into the model section?
There was a problem hiding this comment.
The sentence here could maybe be rewritten, but i don't agree we need a full paragraph for it. This is not really a new feature, but rather the lack of a feature that was missing before.
| property: NamedReference, | ||
| /// If property is a struct, this is the fields. | ||
| /// So if you have `foo <=> element.property.baz.xyz`, then `field_access` is `vec!["baz", "xyz"]` | ||
| field_access: Vec<SmolStr>, |
There was a problem hiding this comment.
Not quite part of this PR, but I'm inclined to suggest handling this as another enum variant to make the code more readable (instead of the extra ifs after the match)
| } | ||
| } | ||
|
|
||
| struct TwoWayBindingModel<T, ItemTree, Getter, Setter> { |
There was a problem hiding this comment.
I think this may be the wrong name for the type. We used it to implement two-way bindings to model data, but I'd say this is more like a two-way binding with custom setter/getter or property with custom setter/getter.
However, until we have a second user we could also keep this called as this.
It shouls always be a local reference
Add a BindingCallable that reads from a closure (registering a model row dependency) and writes back via another closure on `intercept_set`. The rtti trait and the C++ Property helper expose it to code generators.
Represent two-way bindings as an enum: the existing form binding to another property becomes `TwoWayBinding::Property`, and a new `TwoWayBinding::ModelData` variant binds to a row of the enclosing `for` model (optionally through struct fields). The resolve pass now walks each element in three phases so that the model expression of a `for` is resolved with the parent scope before two-way bindings in its body reference it. Field accesses on the model row are type-checked against the resolved row type, and a property declared as `property foo <=> item.field` has its type inferred from the bound field. Fixes: #2013
Replace the anonymous `(prop1, prop2, field_access)` tuple with a `TwoWayBinding` struct, and add an `is_model` discriminator plus a `resolve_model` helper that walks to the enclosing `for`'s sub-component and returns the data/index/repeater references used by every code generator.
For a two-way binding targeting a model row, emit a `link_two_way_to_model_data` call whose getter reads the row through the body sub-component's `model_data` property and whose setter writes back via `model_set_row_data` on the parent's repeater.
Emit a `link_two_way_to_model_data` call for model-row two-way bindings, mirroring the Rust generator. Extract a `lower_field_access_chain` helper (also mirroring Rust) and use it from the non-model `link_two_way_with_map` path and from both accessors inside `generate_model_two_way_binding`.
Add an rtti entry point and an interpreter-side helper that builds a (getter, setter) pair walking to the enclosing repeater and reading or writing the row data through the `model_data` property. Ships tests that cover plain, length-struct and nested-for bindings across all four code generators, as well as type inference and the model->UI direction after a native `set_row_data` call. Fixes: #2013
Replace the manual `checked: todo.checked` / `toggled => { todo.checked
= self.checked; }` dance with `checked <=> todo.checked`.
Add a test with two nested `for` loops over separate models, a `Base` component wrapping a `Compo` component with Text, and use-site two-way bindings that reach both the parent-model row (parent_level == 1) and the inner-model row (parent_level == 0). Covers UI-to-model propagation via TouchAreas and model-to-UI propagation via set_row_data, exercised from Rust, C++, and JS. Add two_way_binding_model_global_chain.slint exercising a chain through Glob.global_prop.xxx (struct-field alias) combined with a model-row binding in a for loop. This relies on the BindingMapper intercept_set fix. Enable the full assertion set in two_way_binding_structs2 for C++ and JS now that BindingMapper propagates writes correctly.
ogoffart
force-pushed
the
olivier/two_way_bindings
branch
from
6eff896 to
d5bc0c4
Compare
2 participants
Closes #2013