-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add renderCallback #126
Add renderCallback #126
Conversation
8a1c3da
to
4fe03d9
Compare
Such an event as this has actually been wanted, so this great! :-)
Yeah that will be the realm of a custom node.
It wouldn't necessarily be any slower as all it would entail would just be calling the send message callback with the event, as there is already an event being handled it will automatically be handled on the next event handling loop right after anyway.
I'll perform a review of this shortly and will think it through.
This is precisely one of the Elm annoyances that would be fixed. ^.^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah an event would be better than a new callback, let me think on this a short period...
@@ -117,6 +117,7 @@ let main = | |||
model = init (); (* Since model is a set value here, we call our init function to generate that value *) | |||
update; | |||
view; | |||
renderCallback = (fun _ -> ()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, actually thinking this over, perhaps a subscription would make more sense, that way it is fed into the normal even loop processing... It would have to touch some internals, but as it is defined within tea anyway that seems fine.
@@ -177,6 +179,7 @@ let programLoop update view subscriptions initModel initCmd = function | |||
| Some _id -> | |||
let newVdom = [view !latestModel] in | |||
let justRenderedVdom = Vdom.patchVNodesIntoElement callbacks parentNode !priorRenderedVdom newVdom in | |||
let () = renderCallback !latestModel in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this doesn't allow for any commands either, so a normal event might be a lot better.
The place I need to run this callback is after immediately the render which happens asynchronously and unpredictably. Subscriptions, cmds, and other events do not have this property as I understand it? |
Thinking on this more, another way to handle this would be to have a type of cmd that runs post-render only. I can make that work if you give me a rough idea of how you'd like it implemented. |
I think I have an idea, just just a list of events to pass to the application to be applied during and after each DOM update call, this list would be managed by a subscription. I'm working on the idea now but I ran into a typing bug with Cmd.map (in unrelated code...) that I'm needing to fix first. |
@pbiggar I just pushed a branch at https://github.com/OvermindDL1/bucklescript-tea/tree/renderevent with an added subscription function at If it works I'll merge it in to main. :-) EDIT: It should pretty well no-op if there is no subscription for a render_event at the time too, hopefully nothing broke there either. Everything compiles at least. ^.^; EDIT2: On a plus side, this also adds an event system for the central processing loop, so it is easy to add more things over time now as well. |
This looks pretty good. How do I use it? Suppose I have a function myRenderFn that I want to call after specific updates, how do I call that? Also, looking through the code am slightly confused by there being AddRenderMsg and RemoveRenderMsg? |
In your subscriptions you should be able to do something like
Purely internal system messages, ignore those. ^.^ |
Reading this again, think I'm wrong here. |
OK, so as I see it, the handler synchronously calls my update function before the repaint. Am I able to update my model and add Cmds? |
Yep! It's the full normal update loop! And as always they will run in the same render callback as well until there are no more events, upon which it will finally exit the render callback, so you will be able to do as much as you need before the screen actually repaints. :-) For note, RenderEvent will be passed to your update 'after' the DOM updates are applied and 'before' the repaint, this way you can define flickerless updates for things like cursor position. |
FYI I finally switched over to using the render_event subscription and it works great! |
This adds a renderCallback function, which runs on the model after the render. We use this to set the cursor (which the browser resets after updating the text of a node).
I looked at other ways of doing this, such as a custom node, and this seemed the simplest, most general, and most performant. The other way to do this would be a node that has a callback right after it updates that runs if-and-only-if it updates. That would be pretty cool but would be a lot more work, and would not necessarily be faster.
I wasn't sure about adding renderCallback to standardProgram and navigationProgram, but defaulted to yes.
Elm doesn't have this API, but this is exactly the sort of problem that Elm's view of the world causes, and it really does need to be run right after the render.
Fixes #125.