From 3a1f2211e94e15a2c31f36ebae54f4199344a724 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Wed, 31 Aug 2016 12:42:55 -0500
Subject: [PATCH 1/8] attempt to namespace plot source

---
 R/plotly.R                 | 5 +++++
 R/shiny.R                  | 4 +++-
 inst/htmlwidgets/plotly.js | 6 ++----
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/R/plotly.R b/R/plotly.R
index 7de4c32198..af12180b38 100644
--- a/R/plotly.R
+++ b/R/plotly.R
@@ -118,6 +118,11 @@ plot_ly <- function(data = data.frame(), ..., type = NULL, group,
   id <- new_id()
   # avoid weird naming clashes
   plotlyVisDat <- data
+  # automatically namespace source
+  session <- shiny::getDefaultReactiveDomain()
+  if (!is.null(session)) {
+    source <- session$ns(source)
+  }
   p <- list(
     visdat = setNames(list(function() plotlyVisDat), id),
     cur_data = id,
diff --git a/R/shiny.R b/R/shiny.R
index 0c5e9e5c31..c8aa376feb 100644
--- a/R/shiny.R
+++ b/R/shiny.R
@@ -53,6 +53,8 @@ event_data <- function(event = c("plotly_hover", "plotly_click", "plotly_selecte
     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))
+  print(src)
+  val <- session$input[[src]]
   if (is.null(val)) val else jsonlite::fromJSON(val)
 }
diff --git a/inst/htmlwidgets/plotly.js b/inst/htmlwidgets/plotly.js
index 3df60ef50d..13dd8ee3bd 100644
--- a/inst/htmlwidgets/plotly.js
+++ b/inst/htmlwidgets/plotly.js
@@ -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));
       };
     };
     

From d2257d4747f4a5361e55c70c971579fd7b83b24b Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Wed, 31 Aug 2016 17:56:10 -0500
Subject: [PATCH 2/8] take advantage of new globalScope session method

---
 DESCRIPTION | 2 ++
 R/plotly.R  | 4 ++--
 R/shiny.R   | 7 +++----
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index c141cbe7d7..385d6a6ba2 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -56,3 +56,5 @@ Suggests:
 LazyData: true
 VignetteBuilder: knitr
 RoxygenNote: 5.0.1
+Remotes:
+  rstudio/shiny#1344
diff --git a/R/plotly.R b/R/plotly.R
index af12180b38..fe24bae100 100644
--- a/R/plotly.R
+++ b/R/plotly.R
@@ -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)
   }
@@ -119,7 +120,6 @@ plot_ly <- function(data = data.frame(), ..., type = NULL, group,
   # avoid weird naming clashes
   plotlyVisDat <- data
   # automatically namespace source
-  session <- shiny::getDefaultReactiveDomain()
   if (!is.null(session)) {
     source <- session$ns(source)
   }
diff --git a/R/shiny.R b/R/shiny.R
index c8aa376feb..1188faec90 100644
--- a/R/shiny.R
+++ b/R/shiny.R
@@ -47,14 +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.")
   }
   src <- sprintf(".clientValue-%s-%s", event[1], session$ns(source))
-  print(src)
-  val <- session$input[[src]]
+  val <- session$rootScope()$input[[src]]
   if (is.null(val)) val else jsonlite::fromJSON(val)
 }

From d66e2ca62df75010936873c4cfa5739010ac06a8 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Wed, 31 Aug 2016 18:09:45 -0500
Subject: [PATCH 3/8] add an official example

---
 inst/examples/plotlyShinyModules/app.R | 39 ++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 inst/examples/plotlyShinyModules/app.R

diff --git a/inst/examples/plotlyShinyModules/app.R b/inst/examples/plotlyShinyModules/app.R
new file mode 100644
index 0000000000..2dc8671413
--- /dev/null
+++ b/inst/examples/plotlyShinyModules/app.R
@@ -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)
+}
+
+shinyApp(ui, server)

From 308ff9210581896b48d5223aac62a754c717c252 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Thu, 1 Sep 2016 11:55:48 -0500
Subject: [PATCH 4/8] shiny issue was merged

---
 DESCRIPTION | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 385d6a6ba2..c28f747780 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -43,7 +43,7 @@ Suggests:
     testthat,
     knitr,
     devtools,
-    shiny,
+    shiny (>= 0.13.2.9005),
     htmltools,
     curl,
     rmarkdown,
@@ -57,4 +57,4 @@ LazyData: true
 VignetteBuilder: knitr
 RoxygenNote: 5.0.1
 Remotes:
-  rstudio/shiny#1344
+  rstudio/shiny

From ffef4919d1d93ea3edba36931f539b4c7ec0eb16 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Tue, 6 Sep 2016 10:54:47 -0500
Subject: [PATCH 5/8] don't namespace under-the-hood

---
 R/plotly.R                             |  8 ++------
 R/shiny.R                              |  2 +-
 inst/examples/plotlyShinyModules/app.R | 27 +++++++++++++++++---------
 man/event_data.Rd                      |  3 ++-
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/R/plotly.R b/R/plotly.R
index c2ece2eb1c..6ea848079b 100644
--- a/R/plotly.R
+++ b/R/plotly.R
@@ -78,8 +78,7 @@
 plot_ly <- function(data = data.frame(), ..., type = NULL, 
                     color, colors = NULL, alpha = 1, symbol, symbols = NULL, 
                     size, sizes = c(10, 100), linetype, linetypes = NULL,
-                    width = NULL, height = NULL, source = "A", 
-                    session = shiny::getDefaultReactiveDomain()) {
+                    width = NULL, height = NULL, source = "A") {
   if (!is.data.frame(data)) {
     stop("First argument, `data`, must be a data frame.", call. = FALSE)
   }
@@ -120,10 +119,7 @@ plot_ly <- function(data = data.frame(), ..., type = NULL,
   id <- new_id()
   # avoid weird naming clashes
   plotlyVisDat <- data
-  # automatically namespace source
-  if (!is.null(session)) {
-    source <- session$ns(source)
-  }
+  
   p <- list(
     visdat = setNames(list(function() plotlyVisDat), id),
     cur_data = id,
diff --git a/R/shiny.R b/R/shiny.R
index 11ae9ea442..6cb9768d83 100644
--- a/R/shiny.R
+++ b/R/shiny.R
@@ -54,7 +54,7 @@ event_data <- function(event = c("plotly_hover", "plotly_click", "plotly_selecte
     stop("No reactive domain detected. This function can only be called \n",
          "from within a reactive shiny context.")
   }
-  src <- sprintf(".clientValue-%s-%s", event[1], session$ns(source))
+  src <- sprintf(".clientValue-%s-%s", event[1], source)
   val <- session$rootScope()$input[[src]]
   if (is.null(val)) val else jsonlite::fromJSON(val)
 }
diff --git a/inst/examples/plotlyShinyModules/app.R b/inst/examples/plotlyShinyModules/app.R
index 2dc8671413..8f6eaf1808 100644
--- a/inst/examples/plotlyShinyModules/app.R
+++ b/inst/examples/plotlyShinyModules/app.R
@@ -7,23 +7,31 @@ reusableUI <- function(id = NULL) {
   fluidRow(
     column(4, plotlyOutput(ns("p1"))),
     column(4, plotlyOutput(ns("p2"))),
-    column(4, verbatimTextOutput(ns("ev")))
+    column(4, verbatimTextOutput(ns("ev1"))),
+    column(4, verbatimTextOutput(ns("ev2")))
   )
 }
 
-viz <- function(input, output, session) {
+viz <- function(input, output, scope, src) {
+  
+  # if you want, you can define multiple sources here
+  src2 <- paste0(src, "2")
+  
   output$p1 <- renderPlotly({
     plot_ly(mtcars, x = ~mpg, y = ~disp, 
-            key = row.names(mtcars), session = session)
+            key = row.names(mtcars), source = src)
   })
   output$p2 <- renderPlotly({
     plot_ly(mtcars, x = ~mpg, y = ~disp, 
-            key = row.names(mtcars), session = session)
+            key = row.names(mtcars), source = src2)
   })
-  output$ev <- renderPrint({
-    d <- event_data("plotly_hover", session = session)
-    if (is.null(d)) print(paste("Module", session$ns(NULL))) else d
+  output$ev1 <- renderPrint({
+    event_data("plotly_hover", source = src)
   })
+  output$ev2 <- renderPrint({
+    event_data("plotly_hover", source = src2)
+  })
+  
 }
 
 ui <- fluidPage(
@@ -32,8 +40,9 @@ ui <- fluidPage(
 )
 
 server <- function(input, output, session) {
-  callModule(viz, "one", session = session)
-  callModule(viz, "two", session = session)
+  # use the src argument to namespace plotly events
+  callModule(viz, "one", src = "A")
+  callModule(viz, "two", src = "B")
 }
 
 shinyApp(ui, server)
diff --git a/man/event_data.Rd b/man/event_data.Rd
index e70c7c9670..a398625205 100644
--- a/man/event_data.Rd
+++ b/man/event_data.Rd
@@ -5,7 +5,8 @@
 \title{Access plotly user input event data in shiny}
 \usage{
 event_data(event = c("plotly_hover", "plotly_click", "plotly_selected",
-  "plotly_relayout"), source = "A")
+  "plotly_relayout"), source = "A",
+  session = shiny::getDefaultReactiveDomain())
 }
 \arguments{
 \item{event}{The type of plotly event. Currently 'plotly_hover',

From dca6b82bd0d0d8bc1de11791064d6de5e21f03d6 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Tue, 6 Sep 2016 11:24:40 -0500
Subject: [PATCH 6/8] module function should always have a session argument

---
 R/plotly.R                             | 1 -
 inst/examples/plotlyShinyModules/app.R | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/R/plotly.R b/R/plotly.R
index 6ea848079b..44603a73d2 100644
--- a/R/plotly.R
+++ b/R/plotly.R
@@ -119,7 +119,6 @@ plot_ly <- function(data = data.frame(), ..., type = NULL,
   id <- new_id()
   # avoid weird naming clashes
   plotlyVisDat <- data
-  
   p <- list(
     visdat = setNames(list(function() plotlyVisDat), id),
     cur_data = id,
diff --git a/inst/examples/plotlyShinyModules/app.R b/inst/examples/plotlyShinyModules/app.R
index 8f6eaf1808..e96a82c51a 100644
--- a/inst/examples/plotlyShinyModules/app.R
+++ b/inst/examples/plotlyShinyModules/app.R
@@ -12,9 +12,9 @@ reusableUI <- function(id = NULL) {
   )
 }
 
-viz <- function(input, output, scope, src) {
+viz <- function(input, output, session, src) {
   
-  # if you want, you can define multiple sources here
+  # if you want, you can distinguish between events *within* a module
   src2 <- paste0(src, "2")
   
   output$p1 <- renderPlotly({

From 3536fe7362af02a3677d4137f38bb3e10c47675e Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Sun, 11 Sep 2016 16:27:51 -0500
Subject: [PATCH 7/8] bump version; update news

---
 DESCRIPTION       | 6 ++----
 NEWS.md           | 6 ++++++
 R/shiny.R         | 1 +
 man/event_data.Rd | 2 ++
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index d0ca676380..b56712d4b9 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: plotly
 Title: Create Interactive Web Graphics via 'plotly.js'
-Version: 4.3.6
+Version: 4.3.7
 Authors@R: c(person("Carson", "Sievert", role = c("aut", "cre"),
     email = "cpsievert1@gmail.com"),
     person("Chris", "Parmer", role = c("aut", "cph"),
@@ -44,7 +44,7 @@ Suggests:
     testthat,
     knitr,
     devtools,
-    shiny (>= 0.13.2.9005),
+    shiny (>= 0.14),
     htmltools,
     curl,
     rmarkdown,
@@ -57,5 +57,3 @@ Suggests:
 LazyData: true
 VignetteBuilder: knitr
 RoxygenNote: 5.0.1
-Remotes:
-  rstudio/shiny
diff --git a/NEWS.md b/NEWS.md
index d1ed52a7e8..00b15071c5 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,3 +1,9 @@
+# 4.3.7 -- 11 September 2016
+
+## BUG FIXES
+
+* `event_data()` now works inside shiny modules (#659). For an example, see <https://github.com/ropensci/plotly/tree/master/inst/examples/plotlyShinyModules>
+
 # 4.3.6 -- 9 September 2016
 
 ## CHANGES
diff --git a/R/shiny.R b/R/shiny.R
index 6cb9768d83..c540ef2ed9 100644
--- a/R/shiny.R
+++ b/R/shiny.R
@@ -41,6 +41,7 @@ renderPlotly <- function(expr, env = parent.frame(), quoted = FALSE) {
 #' @param source Which plot should the listener be tied to? This 
 #' (character string) should match the value of \code{source} in 
 #' \code{\link{plot_ly}()}.
+#' @param session a shiny session object (the default should almost always be used).
 #' @export
 #' @author Carson Sievert
 #' @examples \dontrun{
diff --git a/man/event_data.Rd b/man/event_data.Rd
index a398625205..1928f8c955 100644
--- a/man/event_data.Rd
+++ b/man/event_data.Rd
@@ -15,6 +15,8 @@ event_data(event = c("plotly_hover", "plotly_click", "plotly_selected",
 \item{source}{Which plot should the listener be tied to? This 
 (character string) should match the value of \code{source} in 
 \code{\link{plot_ly}()}.}
+
+\item{session}{a shiny session object (the default should almost always be used).}
 }
 \description{
 This function must be called within a reactive shiny context.

From e6f2a81449ea1fb8979f939d80072781e5f41120 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Sun, 11 Sep 2016 16:57:14 -0500
Subject: [PATCH 8/8] discard attributes with non-NULL dimensions

---
 R/plotly_build.R | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/R/plotly_build.R b/R/plotly_build.R
index 68181c81f1..b52aa34e4a 100644
--- a/R/plotly_build.R
+++ b/R/plotly_build.R
@@ -98,6 +98,8 @@ plotly_build.plotly <- function(p) {
     })
     dataArrayAttrs <- names(Attrs)[as.logical(isArray)]
     tr <- trace[names(trace) %in% c(npscales(), special_attrs(trace), dataArrayAttrs)]
+    # TODO: does it make sense to "train" matrices/2D-tables (e.g. z)?
+    tr <- tr[vapply(tr, function(x) is.null(dim(x)), logical(1))]
     builtData <- tibble::as_tibble(tr)
     
     # avoid clobbering I() (i.e., variables that shouldn't be scaled)