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

Implement support for clientside callbacks in Dash for R #130

Merged
merged 20 commits into from
Oct 1, 2019

Conversation

rpkyle
Copy link
Contributor

@rpkyle rpkyle commented Sep 22, 2019

This PR proposes modifications to support clientside callbacks in Dash for R, as they currently exist in Dash for Python (plotly/dash/issues/266, plotly/dash/pull/672, plotly/dash-renderer/pull/143).

Specifically, the following changes are proposed:

  • add a function called clientsideFunction which allows app developers to reference a JavaScript function and its namespace
  • modify insertIntoCallbackMap to accept either an R function or a JavaScript function
  • modify assert_valid_callbacks to permit either a function to be passed into the callback_map, or a reference to a JavaScript function (provided that the object's names are namespace and function_name)
  • modify the _dash-dependencies route handler to add clientside_function to the JSON payload

Closes #129.

@rpkyle rpkyle added enhancement parity Modifications to improve parity across Dash implementations labels Sep 22, 2019
@rpkyle rpkyle added this to the Dash v1.4.0 milestone Sep 22, 2019
@rpkyle rpkyle self-assigned this Sep 22, 2019
@rpkyle rpkyle requested a review from alexcjohnson September 22, 2019 17:22
@rpkyle
Copy link
Contributor Author

rpkyle commented Sep 22, 2019

Sample app (cf. https://community.plot.ly/t/dash-0-41-0-released/22131):

library(dash)
library(dashCoreComponents)
library(dashHtmlComponents)

app <- Dash$new()

app$layout(htmlDiv(list(
    dccInput(id='input'),
    htmlDiv(id='output-clientside'),
    htmlDiv(id='output-serverside')
    )
    )
)

app$callback( 
  output(id = "output-serverside", property = "children"),
  params = list(
    input(id = "input", property = "value")
  ),
  function(value) {
    sprintf("Server says %s", value)
  }
)

app$clientside_callback(
  output('output-clientside', 'children'),
  params=list(input('input', 'value')),
  clientsideFunction(
    namespace = 'clientside',
    function_name = 'display'
    )
)

app$run_server(showcase = TRUE, debug = TRUE)

namespace = 'clientside',
function_name = 'display'
)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if we allow app$callback to accept either an R function or a clientsideFunction, determine which it is based on type, and do away with app$clientside_callback? It's a little different from Python, but that's just because of Python's decorators vs R's anonymous functions. It's still plenty clear, I think, as you've still got an explicit call to clientsideFunction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered about that as I was working on this PR; using the same syntax for both seems perfectly fine to me. I'll go ahead and make this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in e4fffaf

@rpkyle rpkyle requested a review from alexcjohnson September 24, 2019 04:59
R/utils.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Looks great! just one small suggestion https://github.com/plotly/dashR/pull/130/files#r330202703 and trailing whitespace, then 💃

rpkyle and others added 2 commits October 1, 2019 15:10
Co-Authored-By: alexcjohnson <johnson.alex.c@gmail.com>
@rpkyle
Copy link
Contributor Author

rpkyle commented Oct 1, 2019

Looks great! just one small suggestion https://github.com/plotly/dashR/pull/130/files#r330202703 and trailing whitespace, then 💃

Right, sorry about that. RStudio has been configured to 🔪 trailing whitespace, but my vim settings weren't working as effectively. I had configured

autocmd FileType R,Rd,py,js autocmd BufWritePre <buffer> %s/\s\+$//e

Using a keystroke to make the change manually before save is working, so will use that going forward:

nnoremap <F5> :let _s=@/<Bar>:%s/\s\+$//e<Bar>:let @/=_s<Bar><CR>

@rpkyle rpkyle merged commit 2e09789 into dev Oct 1, 2019
@rpkyle rpkyle deleted the clientside-callbacks-129 branch October 1, 2019 19:39
@rpkyle rpkyle mentioned this pull request Jan 3, 2020
rpkyle added a commit that referenced this pull request Jan 4, 2020
* Provide support for no_update in Dash for R (#111)

* Use dev_tools_prune_errors instead of pruned_errors (#113)

* Better handling for user-defined error conditions in debug mode (#116)

* Provide support for multiple outputs (#119)

* Provide support for hot reloading in Dash for R (#127)

* Implement support for clientside callbacks in Dash for R (#130)

* Add line number context to stack traces when srcrefs are available (#133)

* Update dash-renderer to 1.2.2 and fix dev tools UI display of stack traces (#137)

* Support for meta tags in Dash for R (#142)

* Fixes for hot reloading interval handling and refreshing apps within viewer pane (#148)

* Support for asynchronous loading/compression in Dash for R (#157)

* Support returning asset URLs via public method within Dash class (#160)

* Minor fix for get_asset_url + docs, add url_base_pathname (#161)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dash-type-enhancement parity Modifications to improve parity across Dash implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants