Skip to content
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

Why not InputWidget{T} <: Signal{T}? #1

Closed
stevengj opened this issue Jun 13, 2014 · 12 comments
Closed

Why not InputWidget{T} <: Signal{T}? #1

stevengj opened this issue Jun 13, 2014 · 12 comments

Comments

@stevengj
Copy link
Contributor

I don't see why it you have a separation between InputWidget and Input, i.e. you create a widget and then attach! an input to it.

Why not simply make the InputWidget a type of Signal? Then I could simply do e.g.

s = Slider(....)
lift(x -> RGB(x,x,x), s)
@shashi
Copy link
Member

shashi commented Jun 13, 2014

Ah. Right now, it's actually possible to create a signal of InputWidget values. i.e. I can have the backend update a displayed InputWidget by simply making it a Signal{InputWidget}. Making InputWidget a signal will stop this from making sense / will have us create cycles in the signal graph?

@stevengj
Copy link
Contributor Author

Detecting cycles in the graph and throwing an exception should be pretty easy.

Since most InputWidgets will also be used as input signals, it seems a shame to require the extra two steps of creating an Input and calling attach! every single time in the common case.

@stevengj
Copy link
Contributor Author

Why not just have add_child! throw an exception if the rank of the parent is >= the rank of the child?

@shashi
Copy link
Member

shashi commented Jun 14, 2014

But the question is: If you made InputWidget something like Input, how can the backend update its value after it's been displayed without having the update come back to the backend?

One thing we could do is have an Input as a field of InputWidgets and directly update the field. We can then have Signal{InputWidget}. You had suggested this before, IIRC. It seems OK now.

In this case, we can have lift methods correctly deal with the InputWidget objects as in

s = Slider(....)
lift(x -> RGB(x,x,x), s)

having the rank check in add_child! is a very nice idea. It guarantees topological order. Will do this 👍

@stevengj
Copy link
Contributor Author

I don't understand the difference between having an Input as a field of InputWidget vs. just having value::T, rank::Uint, and children::Vector{Signal} fields directly in InputWidget <: Signal and having Input-like methods for InputWidget.

@shashi
Copy link
Member

shashi commented Jun 14, 2014

My thinking is that the value field in InputWidget represents the value that should be used when the Widget is being drawn in the browser. (e.g. the position of the slider), and the input :: Input field is the input node in the signal graph and may not be always in sync with the value shown in the browser. (e.g. I might want it to update only when the user explicitly changes the value in a widget). I'd like to be able to create inputs that can themselves change like signals do (e.g. A slider whose limits or current value changing with other signals.) It would be nice if I could just create a Signal{Slider} in such a scenario. Consider:

s1 = Slider("x1", 0.0, 0.0, 1.0, 0.0001, Input(0.0))
input2 = Input(0.0) # Let's say I want to constrain this value to be <= s1^2
s2 = lift(x -> Slider("x2", min(input2.value, x*x), 0.0, x*x, 0.0001, input2), s1.input)

If Slider were a Signal, we'd have a Signal{Signal} on our hands and that kind of explodes my mind.

s1 = Slider("x", 0.0, 0.0, 1.0, 0.001)
s2 = lift(x->Slider("x2", min(x*x, s2.value), 0.0, x*x, 0.001), s1)
# The type of s2 ends up being Signal{Signal},
# s2.value is not defined in the beginning so this would crash

Elm has had a few iterations on how it deals with Input elements. My design above is roughly what they have settled for. I hope I was clear. In general, I think it pays to separate the concept of a widget from that of an input node in the signal graph. The InputWidget is the view, and the Input field is the model it updates. There can be any logic that updates the view.

@stevengj
Copy link
Contributor Author

I don't see the problem with Signal{Signal}. Initializing the value (e.g. to the initial slider value) does not seem to be difficult.

Entia non sunt multiplicanda.

@shashi
Copy link
Member

shashi commented Jun 15, 2014

Signal{Signal} is quite complicated I think. It may not be an entirely new entity, but it doesn't make sense immediately. We defined Signal{T} as a time-varying value of type T. Is Signal{Signal} a time-varying value of a time-varying value? That barely makes sense. How does it look in the Signal graph? Does Signal{Signal} keep producing signal nodes? What happens to nodes that are dependent on the value of a Signal{Signal}? There are immediately these questions. I feel the separation between a Widget and its Signal is necessary for simplicity. There has been debate about this in the Elm mailing list ("signal of signals") Elm avoids Signal (Signal a).

Also, how do you initialize the value to the initial slider value though? I could't figure it out.

@one-more-minute What do you think about this?

@MikeInnes
Copy link

It would definitely make sense to be able to implicitly treat things like InputWidgets as signals, but I don't think that we need to force them to be children of Signal or reimplement Input functionality within them to do that.

I think the way to do this would be to have a function which returns the concrete Signal associated with an object, e.g.

signal(x::Input) = x
signal(x::InputWidget) = x.input
# then
lift(f :: Function, output_type :: Type, inputs...) =
    Lift{output_type}(f, map(signal, inputs)...)

That would be a really easy way to get the nicer interface:

s = Slider(1:10)
lift(x->x^2, s)

which is very close to what we ultimately want to have.

If you wanted to be even more general you could dispense with the Signal abstract type and treat it as a protocol – like iteration. But I don't know if we really need that.

@stevengj
Copy link
Contributor Author

I still feel that there is something fundamentally broken and confusing about an abstraction in which an InputWidget is not a kind of Input/Signal, but requires some intermediary object. Your recv function seems to be clearly duplicating the React functionality, because in everything but name the inputs[widget] are effectively children of the widget in the signal graph.

(By the way, here is yet another global dictionary that should really be a type field. If you call that field children, you should notice a strong resemblance to Signal.)

As for creating inputs that affect widgets, a Signal{InputWidget} has exactly the same problem that Signal{Signal} would have: if the signal produces a new InputWidget, how will you update all the inputs[widget] that you previously attach!ed? Will you update inputs[widget] to the new widget key, or will the user have to call attach! again?

One option, if you have an InputWidget whose fields can be updated by another Signal, is to just make those fields signals. e.g. the Checkbox could have value::Signal{Bool}.

shashi added a commit to JuliaGizmos/Reactive.jl that referenced this issue Jun 17, 2014
@shashi
Copy link
Member

shashi commented Jul 22, 2014

There is no more an attach! function, and there is a one-to-one correspondence from a widget to a signal. Each widget has a .input (or in the case of output widgets .signal) field which is an Input node, signal(::Widget) will give you the Signal that the widget's input manifests as.

x=Slider(0:0.01:1)
@lift RGB(x,x,x)

works!

@stevengj
Copy link
Contributor Author

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants