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

JavaScript conveniences #172

Merged
merged 4 commits into from Dec 7, 2015

Conversation

Projects
None yet
4 participants
@jcheng5
Collaborator

jcheng5 commented Nov 30, 2015

In #171 I added features to make widget instances more easy to manipulate from JavaScript. However, it's not obvious how you actually get your custom JavaScript code onto a web page alongside a widget (assuming you're doing a standalone render of a widget, not in R Markdown or Shiny).

I think there are actually three related problems we should solve for this release:

  1. Inject JavaScript (and other invisible elements like CSS stylesheets) alongside a widget when it's rendered standalone. Don't modify anything about the page layout (for example the standalone (re)sizing rules should apply, which is not generally the case when you combine a widget with other HTML like in R Markdown and Shiny)
  2. Put multiple widgets, plus additional JS/CSS, into a single page (without R Markdown or Shiny).
  3. Custom JavaScript that wants to access widget instance objects need to defer execution until after the widgets have actually been rendered; if they run at page load time, the page will only contain <script type="application/json"> and empty <div>s.

This PR addresses 1 (via prependContent and appendContent) and 3 (onStaticRenderComplete). (I think 2 will be a bigger project and should go into a different PR.)

Here's an example of how you might use the two together.

library(htmlwidgets)
library(leaflet)

leaflet() %>% addTiles() %>% appendContent(
  onStaticRenderComplete("
    var map = HTMLWidgets.find('.leaflet');
    map.on('click', function(e) {
      alert(e.latlng);
    });
  ")
)

The prependContent and appendContent functions insert HTML between the #htmlwidget_container and the widget element itself.

<div id="htmlwidget_container">
  <!-- prepended content goes here -->
  <div class="leaflet"> ... </div>
  <!-- appended content goes here -->
</div>

The content can be any htmltools objects (tags, escaped text, unescaped HTML, lists, and NULL) and will simply be stored on the widget itself as additional list elements (x$prependedContent and x$appendedContent or whatever). If the prepended/appended content has HTML dependencies, those will be correctly handled.

jcheng5 added some commits Nov 30, 2015

Add onStaticRenderComplete
Provides a convenient way to schedule arbitrary JS code for execution
after staticRender() finishes executing.
@jcheng5

This comment has been minimized.

Show comment
Hide comment
@jcheng5

jcheng5 Nov 30, 2015

Collaborator

I got an example from @timelyportfolio showing how he handles this:

https://github.com/hrbrmstr/taucharts/blob/2cc90a4ead0a9ceadc1524b67462a91fb85faaf4/R/tasks.r
https://github.com/hrbrmstr/taucharts/blob/2cc90a4ead0a9ceadc1524b67462a91fb85faaf4/inst/htmlwidgets/taucharts.js#L100-L114

This is quite nice, better than the append/prepend mechanism for most scenarios I think. I'm going to hold off on this PR until I've had a chance to think about this.

Collaborator

jcheng5 commented Nov 30, 2015

I got an example from @timelyportfolio showing how he handles this:

https://github.com/hrbrmstr/taucharts/blob/2cc90a4ead0a9ceadc1524b67462a91fb85faaf4/R/tasks.r
https://github.com/hrbrmstr/taucharts/blob/2cc90a4ead0a9ceadc1524b67462a91fb85faaf4/inst/htmlwidgets/taucharts.js#L100-L114

This is quite nice, better than the append/prepend mechanism for most scenarios I think. I'm going to hold off on this PR until I've had a chance to think about this.

@jcheng5 jcheng5 closed this Nov 30, 2015

@ramnathv

This comment has been minimized.

Show comment
Hide comment
@ramnathv

ramnathv Nov 30, 2015

Owner

@jcheng5 @timelyportfolio I like the tasks mechanism too. The only suggestion I would have is that the callback function signature should include access to el, width, height and value (basically what the current version of renderValue has). This would allow one to make use of these parameters while specifying the callback.

Owner

ramnathv commented Nov 30, 2015

@jcheng5 @timelyportfolio I like the tasks mechanism too. The only suggestion I would have is that the callback function signature should include access to el, width, height and value (basically what the current version of renderValue has). This would allow one to make use of these parameters while specifying the callback.

@jcheng5

This comment has been minimized.

Show comment
Hide comment
@jcheng5

jcheng5 Nov 30, 2015

Collaborator

I think append/prepend still makes sense for 1) htmlwidgets that don't expose a tasks parameter (especially legacy ones), 2) CSS customizations, and 3) non-intrusively adding additional HTML dependencies. So it looks like the two approaches are at least somewhat complementary.

Collaborator

jcheng5 commented Nov 30, 2015

I think append/prepend still makes sense for 1) htmlwidgets that don't expose a tasks parameter (especially legacy ones), 2) CSS customizations, and 3) non-intrusively adding additional HTML dependencies. So it looks like the two approaches are at least somewhat complementary.

@jcheng5

This comment has been minimized.

Show comment
Hide comment
@jcheng5

jcheng5 Dec 1, 2015

Collaborator

OK so scratch "1) htmlwidgets that don't expose a tasks parameter (especially legacy ones)". It's going to work like this:

leaflet() %>% addTiles() %>%
  htmlwidgets::addRenderTask("function(el, x) { this.setZoom(4); }")

so all widgets, old and new, will magically gain this ability!

Collaborator

jcheng5 commented Dec 1, 2015

OK so scratch "1) htmlwidgets that don't expose a tasks parameter (especially legacy ones)". It's going to work like this:

leaflet() %>% addTiles() %>%
  htmlwidgets::addRenderTask("function(el, x) { this.setZoom(4); }")

so all widgets, old and new, will magically gain this ability!

New addRenderHook function (thanks @timelyportfolio, @ramnathv)
For adding custom JS rendering code onto a particular widget instance.

@jcheng5 jcheng5 reopened this Dec 1, 2015

@jcheng5

This comment has been minimized.

Show comment
Hide comment
@jcheng5

jcheng5 Dec 1, 2015

Collaborator

Anyone have a problem with the function name addRenderHook? Hook is a slightly ugly-sounding word to me, but I thought this was more generic than "task" because I could see us expanding this notion in the future:

widget %>%
  addHook("initialize", ...js code...) %>%
  addHook("pre-render", ...js code...) %>%
  addHook("render", ...js code...) %>%
  addHook("pre-resize", ...js code...) %>%
  addHook("resize", ...js code...)
Collaborator

jcheng5 commented Dec 1, 2015

Anyone have a problem with the function name addRenderHook? Hook is a slightly ugly-sounding word to me, but I thought this was more generic than "task" because I could see us expanding this notion in the future:

widget %>%
  addHook("initialize", ...js code...) %>%
  addHook("pre-render", ...js code...) %>%
  addHook("render", ...js code...) %>%
  addHook("pre-resize", ...js code...) %>%
  addHook("resize", ...js code...)
@timelyportfolio

This comment has been minimized.

Show comment
Hide comment
@timelyportfolio

timelyportfolio Dec 1, 2015

Collaborator

I have no problems with the word "hook" nor the pull request as changed. I very much like this ability.

Collaborator

timelyportfolio commented Dec 1, 2015

I have no problems with the word "hook" nor the pull request as changed. I very much like this ability.

@ramnathv

This comment has been minimized.

Show comment
Hide comment
@ramnathv

ramnathv Dec 1, 2015

Owner

I feel comfortable with the function name addRenderHook. Another option is to use somethin like on or onEvent.

Owner

ramnathv commented Dec 1, 2015

I feel comfortable with the function name addRenderHook. Another option is to use somethin like on or onEvent.

@jcheng5

This comment has been minimized.

Show comment
Hide comment
@jcheng5

jcheng5 Dec 1, 2015

Collaborator
widget %>%
  onRender("function(el, x) { ... }")

Not bad at all... I think I'll do that...

Collaborator

jcheng5 commented Dec 1, 2015

widget %>%
  onRender("function(el, x) { ... }")

Not bad at all... I think I'll do that...

@jjallaire

This comment has been minimized.

Show comment
Hide comment
@jjallaire

jjallaire Dec 2, 2015

Collaborator

I like hook / addRenderHook as well

On Tue, Dec 1, 2015 at 6:01 PM, Ramnath Vaidyanathan <
notifications@github.com> wrote:

I feel comfortable with the function name addRenderHook. Another option
is to use somethin like on or onEvent.


Reply to this email directly or view it on GitHub
#172 (comment).

Collaborator

jjallaire commented Dec 2, 2015

I like hook / addRenderHook as well

On Tue, Dec 1, 2015 at 6:01 PM, Ramnath Vaidyanathan <
notifications@github.com> wrote:

I feel comfortable with the function name addRenderHook. Another option
is to use somethin like on or onEvent.


Reply to this email directly or view it on GitHub
#172 (comment).

@jcheng5

This comment has been minimized.

Show comment
Hide comment
@jcheng5

jcheng5 Dec 7, 2015

Collaborator

Are we good with the name onRender? If so, I think this is ready to merge.

Collaborator

jcheng5 commented Dec 7, 2015

Are we good with the name onRender? If so, I think this is ready to merge.

@timelyportfolio

This comment has been minimized.

Show comment
Hide comment
@timelyportfolio

timelyportfolio Dec 7, 2015

Collaborator

Works for me. Name is sort-of intuitive and JS-like. Also, I can't think of any conflicts with other R packages.

Collaborator

timelyportfolio commented Dec 7, 2015

Works for me. Name is sort-of intuitive and JS-like. Also, I can't think of any conflicts with other R packages.

@jjallaire

This comment has been minimized.

Show comment
Hide comment
@jjallaire

jjallaire Dec 7, 2015

Collaborator

Yes, agreed. Good to merge!

J

On Mon, Dec 7, 2015 at 1:21 PM, timelyportfolio notifications@github.com
wrote:

Works for me. Name is sort-of intuitive and JS-like. Also, I can't think
of any conflicts with other R packages.


Reply to this email directly or view it on GitHub
#172 (comment).

Collaborator

jjallaire commented Dec 7, 2015

Yes, agreed. Good to merge!

J

On Mon, Dec 7, 2015 at 1:21 PM, timelyportfolio notifications@github.com
wrote:

Works for me. Name is sort-of intuitive and JS-like. Also, I can't think
of any conflicts with other R packages.


Reply to this email directly or view it on GitHub
#172 (comment).

jcheng5 added a commit that referenced this pull request Dec 7, 2015

@jcheng5 jcheng5 merged commit 083f018 into master Dec 7, 2015

@jcheng5 jcheng5 deleted the joe/feature/js-callbacks branch Dec 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment