Allow custom kinds of bindables and stores #154
Conversation
core/src/handle.rs
Outdated
| L: Lens, | ||
| <L as Lens>::Target: Data, | ||
| L: Bindable, | ||
| <L as Bindable>::Output: Data, |
There was a problem hiding this comment.
I'm wondering if this bound is required or not now. Previously the output of a lens needed to implement data because the store which kept a copy of the output would need it to compare for change. Now that a Bindable creates its own store which decides whether data has changed this bound may not be needed anymore.
| for store_req in bindable.requests() { | ||
| // walk the ancestry chain child-to-parent... | ||
| for entity in new_ancestors.iter() { |
There was a problem hiding this comment.
I think maybe these loops should be swapped becuase when we iterate up the tree to find the relevant stores we can break out early if they are all found.
|
|
||
| let model = model_data.downcast_ref::<L::Source>().unwrap(); | ||
| fn requests(&self) -> Vec<StoreId>; | ||
| fn make_store(&self, source: ModelOrView) -> Option<Box<dyn StoreHandler>>; |
There was a problem hiding this comment.
Could it be worth using an associated type for the returned store?
There was a problem hiding this comment.
I considered this, but I'd like to keep the flexibility of a single bindable being able to bind to multiple kinds of stores!
There was a problem hiding this comment.
But how would a single bindable choose what store to use?
There was a problem hiding this comment.
The thing I was envisioning is that there would be a MultiBindable which contains two child bindables. It will then dispatch its method calls to BOTH of the children, and take the first result that succeeds.
There was a problem hiding this comment.
Oh okay, that's an interesting way to do it. I was thinking that this would be done with just the MultiBindable type which would have an associated store that holds on to two lenses and two pieces of state but one list of observers. Then the output of the Bindable would be a tuple of the output types for the lenses.
There was a problem hiding this comment.
I think that approach would require that the two lenses both be on models which are built on the same entity, which is not desirable
There was a problem hiding this comment.
This is why I was thinking that stores should be separated from models. Though your idea of having child bindables is probably a better one, I just need to get my head around what that would look like in terms of stores and whether we end up with redundant observers.
| }; | ||
|
|
||
| // ...so that we can register the observer with the appropriate store. | ||
| bindable.add_to_store(store.as_mut(), id); |
There was a problem hiding this comment.
Why is this a method on bindable rather than on store? Would it not make more sense to have store.add_observer(id)?
There was a problem hiding this comment.
the problem is that Store can need more than just an id in order to do registration - this is the main motivation behind bindable, that is, being able to store data which the store can use in order to more selectively dispatch a rebuild. As a result, there's no one concrete type the store would be able to take - we would need to do a dynamic dispatch to store.add_observer and then it would need to downcast the bindable in order to extract that information.
There was a problem hiding this comment.
Ah I see! But is the observer not always an entity?
There was a problem hiding this comment.
The observer will always be an entity, but the observer's metadata (e.g. the list index being bound to) can be of arbitrary type.
| model_data_store.lenses_dup.push(state); | ||
| } | ||
| } | ||
| fn requests(&self) -> Vec<StoreId> { |
There was a problem hiding this comment.
I'm a bit confused how this works because the make_store method seems to only be able to create a single store but this method suggests that a bindable can link to many stores.
There was a problem hiding this comment.
make_store will be called once for each request!
There was a problem hiding this comment.
Ah okay, so the requests vector defines what stores the binable links to and then the make_store functions creates the store for each item?
| fn element(&self) -> Option<&'static str> { | ||
| Some("binding") | ||
| #[derive(Clone, Debug)] | ||
| pub struct ConstantBindable<T> { |
There was a problem hiding this comment.
This is StaticLens but cleaner. It doesn't cover all the use cases that StaticLens does, but it will be more efficient since it will create no observers.
| let top = ((data.child_y - data.parent_y) * data.scroll_y).round(); | ||
| handle.left(Units::Pixels(-left)).top(Units::Pixels(-top)); | ||
| }) | ||
| .left(data.clone().map_shallow(|data| { |
There was a problem hiding this comment.
So does this mean that map_shallow replaces the need for .bind() when using it to set multiple properties?
No description provided.