Skip to content

Get event_data working on shiny modules #700

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

Merged
merged 10 commits into from
Sep 12, 2016
2 changes: 2 additions & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,5 @@ Suggests:
LazyData: true
VignetteBuilder: knitr
RoxygenNote: 5.0.1
Remotes:
rstudio/shiny#1344
7 changes: 6 additions & 1 deletion R/plotly.R
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@
plot_ly <- function(data = data.frame(), ..., type = NULL, group,
color, colors = NULL, alpha = 1, symbol, symbols = NULL,
size, sizes = c(10, 100), linetype, linetypes = NULL,
width = NULL, height = NULL, source = "A") {
width = NULL, height = NULL, source = "A",
session = shiny::getDefaultReactiveDomain()) {
if (!is.data.frame(data)) {
stop("First argument, `data`, must be a data frame.", call. = FALSE)
}
Expand Down Expand Up @@ -118,6 +119,10 @@ plot_ly <- function(data = data.frame(), ..., type = NULL, group,
id <- new_id()
# avoid weird naming clashes
plotlyVisDat <- data
# automatically namespace source
if (!is.null(session)) {
source <- session$ns(source)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you want the source namespaced? Don't you want to be able to have multiple modules share the same source?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking namespacing would be imposed by default by plotly, but I think I'm now in favor of the approach you describe below

}
p <- list(
visdat = setNames(list(function() plotlyVisDat), id),
cur_data = id,
Expand Down
7 changes: 4 additions & 3 deletions R/shiny.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ renderPlotly <- function(expr, env = parent.frame(), quoted = FALSE) {
#' }

event_data <- function(event = c("plotly_hover", "plotly_click", "plotly_selected",
"plotly_relayout"), source = "A") {
session <- shiny::getDefaultReactiveDomain()
"plotly_relayout"), source = "A",
session = shiny::getDefaultReactiveDomain()) {
if (is.null(session)) {
stop("No reactive domain detected. This function can only be called \n",
"from within a reactive shiny context.")
}
val <- session$input[[sprintf(".clientValue-%s-%s", event[1], source)]]
src <- sprintf(".clientValue-%s-%s", event[1], session$ns(source))
val <- session$rootScope()$input[[src]]
if (is.null(val)) val else jsonlite::fromJSON(val)
}
39 changes: 39 additions & 0 deletions inst/examples/plotlyShinyModules/app.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
library(shiny)
library(plotly)

reusableUI <- function(id = NULL) {
ns <- NS(id)

fluidRow(
column(4, plotlyOutput(ns("p1"))),
column(4, plotlyOutput(ns("p2"))),
column(4, verbatimTextOutput(ns("ev")))
)
}

viz <- function(input, output, session) {
output$p1 <- renderPlotly({
plot_ly(mtcars, x = ~mpg, y = ~disp,
key = row.names(mtcars), session = session)
})
output$p2 <- renderPlotly({
plot_ly(mtcars, x = ~mpg, y = ~disp,
key = row.names(mtcars), session = session)
})
output$ev <- renderPrint({
d <- event_data("plotly_hover", session = session)
if (is.null(d)) print(paste("Module", session$ns(NULL))) else d
})
}

ui <- fluidPage(
reusableUI("one"),
reusableUI("two")
)

server <- function(input, output, session) {
callModule(viz, "one", session = session)
callModule(viz, "two", session = session)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jcheng5 this example seems to still work without passing the session to plot_ly()/event_data(). Would u still suggest doing so?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, you generally don't want to be explicitly passing the session--just let it use the default.

Copy link
Contributor

@jcheng5 jcheng5 Sep 6, 2016

Choose a reason for hiding this comment

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

Same goes for the callModule calls, they shouldn't need explicit session arguments passed to them either.

Copy link
Contributor

@jcheng5 jcheng5 Sep 6, 2016

Choose a reason for hiding this comment

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

Oh I think what I said to you last week was that you should explicitly pass the source, not the session. So viz would have a source = "A" param that's passed to plot_ly and event_data; that way, the main UI can decide whether reusableUI("one") and reusableUI("two") are linked together or separately, by passing the same (or default) or different source values.

shinyApp(ui, server)
6 changes: 2 additions & 4 deletions inst/htmlwidgets/plotly.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ HTMLWidgets.widget({
attachKey("key");
return obj;
});
Shiny.onInputChange(
".clientValue-" + eventType + "-" + x.source,
JSON.stringify(d)
);
var src = ".clientValue-" + eventType + "-" + x.source;
Shiny.onInputChange(src, JSON.stringify(d));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you're doing JSON.stringify() and jsonlite::fromJSON explicitly? If you just pass d from here, and don't JSON-decode from R, I think it should work fine--we do the JSON encoding for you.

Copy link
Collaborator Author

@cpsievert cpsievert Sep 6, 2016

Choose a reason for hiding this comment

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

I'm pretty sure I did that because the default translates an array of objects to a named character vector, when I really want a data frame. Would you go about ensuring a data frame in another way?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair. It does mean you are sending double-encoded JSON data across the wire which will bloat things up a little, but I guess these are pretty small packets anyway.

};
};

Expand Down