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

Docs: add walk-through examples on Canvas redraws #75

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

timholy
Copy link
Member

@timholy timholy commented Nov 11, 2015

Note: this PR is against the "next" branch.

FRP is new to me, so as I struggle to wrap my head around it I thought I'd create a "diary." If that diary is useful to others, great; otherwise, feel free to close this.

@timholy timholy mentioned this pull request Nov 11, 2015
3 tasks
You can cause these updates to be processed at regular intervals---if
julia isn't busy doing something else---as follows:
```jl
eventloop = Timer(_->Reactive.run_till_now(), 0.001, 0.001)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the latest commits, I put this in Reactive.init.

The reason is, if one is using multiple packages that run Reactive.run, then they might end up stepping on each other's run calls. I also made it so that push!(...) takes an optional third argument which is a callback that will be called in case of an error... By default this callback prints the error to STDERR.

I was thinking the reason for exposing Reactive.run is to allow packages to watch for crashes, but this seems like a worse idea than having the onerror callback.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the latest commits, I put this in Reactive.init.

I was going to suggest that (for the reason you cite), glad you've already thought of it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and if you think this has any tutorial value at all, I can make changes as you change the API. (I won't bother if you think it won't be useful, though.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely update this for the new changes and merge this. This will be very useful for library writers using Reactive.

@shashi
Copy link
Member

shashi commented Nov 12, 2015

Reactive is meant to be orthogonal to other packages. But when you are making a GUI-like package such as ImageView, you will need to think about precisely these things. It will be nice to have this documentation.

@timholy
Copy link
Member Author

timholy commented Nov 12, 2015

Glad it seems useful. I will definitely revise this and resubmit.

c = new(io, n1, n2)
combined = merge(n1, n2)
throttled = throttle(1/60, combined)
Reactive.preserve(map(x->println(c.io, "state1: $(value(c.state1)); state2: $(value(c.state2))"), throttled))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the use of the value slightly awkward.... But it's really OK. Here's an alternative

show_canvas(c) = map(render, sampleon(throttle(1/60, merge(c.state1, state2)), Node(c)))

(I find it better to take out the rendering from the type definition as a personal preference)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, you'd still need value inside render. Never mind me.

@timholy timholy force-pushed the teh/imageview_warmups branch from 67e6cc0 to 6241b42 Compare November 13, 2015 12:24
@timholy
Copy link
Member Author

timholy commented Nov 13, 2015

Indeed it's much simpler now.

@musm
Copy link

musm commented Jan 28, 2017

bump

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

Successfully merging this pull request may close these issues.

3 participants