-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Bindings #1002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
cc @chriddyp |
After brief discussion with @etpinard, going to opt for Option 2 since I think Option 1 has much more complicated side effects, is more challenging to specify, and is more functionality than is actually required for this need. |
Don't we already have that information in the definition of what each control does? ie if a control sets Also it's possible for a control to be bound to more than one attribute, right? Like a button could change both |
Good point @alexcjohnson. That might be a good approach. It's possible to inspect the method and args and try to determine what's being set, though it's not an entirely trivial thing to do. For example, then we get into |
As part of this, I'd like to factor out the |
Sounds great! Complicated is fine ( ❤️ 🐅 ! ) if it makes the resulting experience more robust and simpler, which this seems like it will do. |
@alexcjohnson Thanks for the input! I still haven't ruled out the possibility of weird corner cases in my head, but this feels reasonable and in particular I like it because it avoids any additional API surface area. I'll go for it and see what happens. |
Here are the difficulties I can come up with:
It's not insurmountable since it's self-contained and well-defined; it just needs a lot of testing and enumeration of cases in order to work reliably. |
Yeah, |
I think there's been some step down that path with |
As part of #1016, I've implemented the logic to work out which properties are affected by a bindings = [
'data[0].marker.width',
'data[1].marker.width'
]; So in that sense, things are good. The tricky part of automating these bindings is then updating them. Figuring out the properties have changed is easy, but matching that with a component state is not straightforward except for the simple case of a single property. Do you think it's the right move to automate these bindings but only for the relatively simple case where it's very straightforward to match plot state to component state? I can add logic to accomplish this; my concern is that magic that works only in certain conditions is maybe worse than specifying it manually since it's not clear to the end-user precisely what's going on. |
first iteration completed in #1016 |
As controls get more capable in plotly, two-way binding becomes necessary. That is, a slider can change the frame, but the slider should also reflect the current frame when something else (e.g. a play button) changes it. This could also be the case for a marker size, a size ref, or a transform parameter. My current slider PR does this in sort of an ad hoc manner, but I really think it should be more general.
Proposal:
bindings
Option 1:
Bindings are defined in the layout. For example:
In this example,
bind
is the data that gets changed,to
is the item that's observed for changes, andlookup
is an optional translation table that maps one to the other. Whenever a plotly event is emitted or when a relayout/restyle/update occurs, we quickly look through the bindings to see what's changed (which shouldn't be expensive enough that it's a problem). If an item has changed, that item is written to the state of the plot. Aplotly_bindingchange
event is emitted. All components (that's the best I can do right now) must then look through their data to see if anything has changed and update accordingly.Pros:
Cons:
Option 2:
Bindings are defined in the component that needs them, e.g. a slider. The slider has:
Most of the logic is the same. (I've omitted the lookup table since, though it could be added, we should probably go to great lengths to generally avoid needing.) When a component binds to a variable, we register it as needing a lookup. We scan through those liberally since it's fast, and when one changes, we emit an event. This case is a little simpler since we can emit
plotly_bindingchange
with data{binding: 'data[0].marker.color', value: 'red'}
. The component sees its binding has changed and updates accordingly.Pros:
updatemenus[0].active
Cons:
cc: @etpinard @chriddyp @alexcjohnson
The text was updated successfully, but these errors were encountered: