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

Add didrender hook #382

Merged
merged 2 commits into from
May 5, 2016
Merged

Add didrender hook #382

merged 2 commits into from
May 5, 2016

Conversation

joshfrench
Copy link
Contributor

Fires once card has been rendered, for use by cards that need processing after they've been added to the DOM (e.g. 3rd-party widgets.)

This is my first time in this codebase, so I don't expect this to be complete. Specifically:

  • Did I overlook any cases where this hook should (or should not) fire?
  • Would it be useful for the callback take any args, such as the current env?
  • I based this on the onTeardown hook. Is the env.didRender pattern preferable to adding another optional didRender property to the top-level card object?

Thanks for your feedback!

Fires after card has been rendered, for use by cards that need
processing after they've been added to the DOM (e.g. 3rd-party widgets.)
@bantic
Copy link
Collaborator

bantic commented May 5, 2016

This looks great! I restarted the travis build.

@bantic
Copy link
Collaborator

bantic commented May 5, 2016

@joshfrench This looks great, thank you for the PR.
I'm going to merge and then update the docs to mention this hook.
Because it's possible for folks to reuse cards between editor and display-only rendering contexts, I want to be clear in the docs that the hook is only available for cards that are rendered by the editor (and not by, e.g., mobiledoc-dom-renderer). I'm not sure it makes as much sense for that renderer, anyway, because the mobiledoc-dom-renderer builds the entire dom fragment for the mobiledoc before appending anything.

@bantic bantic merged commit 8349f65 into bustle:master May 5, 2016
@joshfrench
Copy link
Contributor Author

Agreed, I wouldn't expect the downstream renderer to have to handle this. As for callback args, am I right in thinking env and friends are unnecessary since they're available via closure when the callback is defined?

@bantic
Copy link
Collaborator

bantic commented May 5, 2016

am I right in thinking env and friends are unnecessary since they're available via closure when the callback is

Yeah, I think's exactly right. All possible variables they might need are in scope already, so they can close over them in their callback function. 👍

@bantic
Copy link
Collaborator

bantic commented May 10, 2016

released in v0.9.6

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.

2 participants