Streamline crosstalk support; fix tests

Stop passing Crosstalk options for every single method; instead, each
method that wants Crosstalk options must explicitly pass them to
invokeMethod(), just like any other argument.

Also remove addSelect(), we haven't fully thought through how one might
use it for non-Crosstalk scenarios.

Author: jcheng5

DESCRIPTION

@@ -44,7 +44,6 @@ Imports:
     raster,
     scales (>= 0.2.5),
     sp,
-    jsonlite,
     crosstalk
 Suggests:
     knitr,

NAMESPACE

@@ -33,7 +33,6 @@ export(addProviderTiles)
 export(addRasterImage)
 export(addRectangles)
 export(addScaleBar)
-export(addSelect)
 export(addSimpleGraticule)
 export(addTerminator)
 export(addTiles)
@@ -103,7 +102,6 @@ export(removeMarkerFromCluster)
 export(removeMeasure)
 export(removePopup)
 export(removeScaleBar)
-export(removeSelect)
 export(removeShape)
 export(removeTiles)
 export(removeTopoJSON)

R/layers.R

@@ -535,10 +535,19 @@ addMarkers = function(
   invokeMethod(
     map, data, 'addMarkers', pts$lat, pts$lng, icon, layerId, group, options,
     popup, popupOptions, clusterOptions, clusterId,
-    safeLabel(label, data), labelOptions
+    safeLabel(label, data), labelOptions,
+    getCrosstalkOptions(data)
   ) %>% expandLimits(pts$lat, pts$lng)
 }
 
+getCrosstalkOptions <- function(data) {
+  if (is.SharedData(data)) {
+    list(ctKey = data$key(), ctGroup = data$groupName())
+  } else {
+    NULL
+  }
+}
+
 #' @describeIn map-layers Add Label only markers to the map
 #' @export
 addLabelOnlyMarkers = function(
@@ -838,7 +847,8 @@ addCircleMarkers = function(
   pts = derivePoints(data, lng, lat, missing(lng), missing(lat), "addCircleMarkers")
   invokeMethod(map, data, 'addCircleMarkers', pts$lat, pts$lng, radius,
                layerId, group, options, clusterOptions, clusterId,
-               popup, popupOptions, safeLabel(label, data), labelOptions) %>%
+               popup, popupOptions, safeLabel(label, data), labelOptions,
+               getCrosstalkOptions(data)) %>%
     expandLimits(pts$lat, pts$lng)
 }
 

R/plugin-awesomeMarkers.R

@@ -274,7 +274,8 @@ addAwesomeMarkers = function(
   invokeMethod(
     map, data, 'addAwesomeMarkers', pts$lat, pts$lng, icon, layerId,
     group, options, popup, popupOptions,
-    clusterOptions, clusterId, safeLabel(label, data), labelOptions
+    clusterOptions, clusterId, safeLabel(label, data), labelOptions,
+    getCrosstalkOptions(data)
   ) %>% expandLimits(pts$lat, pts$lng)
 }
 

R/selection.R

@@ -10,17 +10,17 @@ locationFilter2Dependencies <- function() {
   )
 }
 
-#' @export
-addSelect <- function(map) {
+addSelect <- function(map, data = getMapData(map)) {
   map$dependencies <- c(map$dependencies,
     leafletEasyButtonDependencies(),
     locationFilter2Dependencies())
   map <- addIonIcon(map)
 
-  invokeMethod(map, getMapData(map), "addSelect")
+  invokeMethod(map, data, "addSelect",
+    getCrosstalkOptions(data)[["ctGroup"]]
+  )
 }
 
-#' @export
 removeSelect <- function(map) {
   invokeMethod(map, NULL, "removeSelect")
 }

R/utils.R

@@ -49,17 +49,12 @@ filterNULL = function(x) {
 invokeMethod = function(map, data, method, ...) {
   crosstalkOptions <- if (crosstalk::is.SharedData(data)) {
     map$dependencies <- c(map$dependencies, crosstalk::crosstalkLibs())
-    sd <- data
-    data <- sd$data()
-    list(
-      ctKey = sd$key(),
-      ctGroup = sd$groupName()
-    )
+    data <- data$data()
   } else {
     NULL
   }
 
-  args = c(evalFormula(list(...), data), list(crosstalkOptions))
+  args = evalFormula(list(...), data)
 
   dispatch(map,
     method,

inst/htmlwidgets/leaflet.js

@@ -1391,7 +1391,7 @@ methods.addMarkers = function (lat, lng, icon, layerId, group, options, popup, p
   }
 };
 
-methods.addAwesomeMarkers = function (lat, lng, icon, layerId, group, options, popup, popupOptions, clusterOptions, clusterId, label, labelOptions) {
+methods.addAwesomeMarkers = function (lat, lng, icon, layerId, group, options, popup, popupOptions, clusterOptions, clusterId, label, labelOptions, crosstalkOptions) {
   var icondf = void 0;
   var getIcon = void 0;
   if (icon) {
@@ -1413,7 +1413,7 @@ methods.addAwesomeMarkers = function (lat, lng, icon, layerId, group, options, p
 
   if (!(_jquery2.default.isEmptyObject(lat) || _jquery2.default.isEmptyObject(lng)) || _jquery2.default.isNumeric(lat) && _jquery2.default.isNumeric(lng)) {
 
-    var df = new _dataframe2.default().col("lat", lat).col("lng", lng).col("layerId", layerId).col("group", group).col("popup", popup).col("popupOptions", popupOptions).col("label", label).col("labelOptions", labelOptions).cbind(options);
+    var df = new _dataframe2.default().col("lat", lat).col("lng", lng).col("layerId", layerId).col("group", group).col("popup", popup).col("popupOptions", popupOptions).col("label", label).col("labelOptions", labelOptions).cbind(options).cbind(crosstalkOptions || {});
 
     if (icon) icondf.effectiveLength = df.nrow();
 
@@ -1515,9 +1515,9 @@ methods.addCircles = function (lat, lng, radius, layerId, group, options, popup,
   }
 };
 
-methods.addCircleMarkers = function (lat, lng, radius, layerId, group, options, clusterOptions, clusterId, popup, popupOptions, label, labelOptions) {
+methods.addCircleMarkers = function (lat, lng, radius, layerId, group, options, clusterOptions, clusterId, popup, popupOptions, label, labelOptions, crosstalkOptions) {
   if (!(_jquery2.default.isEmptyObject(lat) || _jquery2.default.isEmptyObject(lng)) || _jquery2.default.isNumeric(lat) && _jquery2.default.isNumeric(lng)) {
-    var df = new _dataframe2.default().col("lat", lat).col("lng", lng).col("radius", radius).col("layerId", layerId).col("group", group).col("popup", popup).col("popupOptions", popupOptions).col("label", label).col("labelOptions", labelOptions).cbind(options);
+    var df = new _dataframe2.default().col("lat", lat).col("lng", lng).col("radius", radius).col("layerId", layerId).col("group", group).col("popup", popup).col("popupOptions", popupOptions).col("label", label).col("labelOptions", labelOptions).cbind(crosstalkOptions || {}).cbind(options);
 
     addMarkers(this, df, group, clusterOptions, clusterId, function (df, i) {
       return _leaflet2.default.circleMarker([df.get(i, "lat"), df.get(i, "lng")], df.get(i));
@@ -2203,7 +2203,7 @@ methods.removeMeasure = function () {
   delete this.measureControl;
 };
 
-methods.addSelect = function (crosstalkOptions) {
+methods.addSelect = function (ctGroup) {
   var _this8 = this;
 
   methods.removeSelect.call(this);
@@ -2217,9 +2217,9 @@ methods.addSelect = function (crosstalkOptions) {
         btn.state("select-active");
         _this8._locationFilter = new _leaflet2.default.LocationFilter2();
 
-        if (crosstalkOptions && crosstalkOptions.ctGroup) {
+        if (ctGroup) {
           (function () {
-            var selectionHandle = new global.crosstalk.SelectionHandle(crosstalkOptions.ctGroup);
+            var selectionHandle = new global.crosstalk.SelectionHandle(ctGroup);
             selectionHandle.on("change", function (e) {
               if (e.sender !== selectionHandle) {
                 if (_this8._locationFilter) {
@@ -2258,7 +2258,7 @@ methods.addSelect = function (crosstalkOptions) {
   this._selectButton.addTo(this);
 };
 
-methods.removeSelect = function (crosstalkOptions) {
+methods.removeSelect = function () {
   if (this._locationFilter) {
     this._locationFilter.disable();
   }

javascript/src/methods.js

@@ -262,7 +262,7 @@ methods.addMarkers = function(lat, lng, icon, layerId, group, options, popup, po
 };
 
 methods.addAwesomeMarkers = function(lat, lng, icon, layerId, group, options, popup, popupOptions,
-clusterOptions, clusterId, label, labelOptions) {
+clusterOptions, clusterId, label, labelOptions, crosstalkOptions) {
   let icondf;
   let getIcon;
   if (icon) {
@@ -294,7 +294,8 @@ clusterOptions, clusterId, label, labelOptions) {
       .col("popupOptions", popupOptions)
       .col("label", label)
       .col("labelOptions", labelOptions)
-      .cbind(options);
+      .cbind(options)
+      .cbind(crosstalkOptions || {});
 
     if (icon) icondf.effectiveLength = df.nrow();
 
@@ -401,7 +402,7 @@ methods.addCircles = function(lat, lng, radius, layerId, group, options, popup,
   }
 };
 
-methods.addCircleMarkers = function(lat, lng, radius, layerId, group, options, clusterOptions, clusterId, popup, popupOptions, label, labelOptions) {
+methods.addCircleMarkers = function(lat, lng, radius, layerId, group, options, clusterOptions, clusterId, popup, popupOptions, label, labelOptions, crosstalkOptions) {
   if(!($.isEmptyObject(lat) || $.isEmptyObject(lng)) ||
       ($.isNumeric(lat) && $.isNumeric(lng))) {
     let df = new DataFrame()
@@ -414,6 +415,7 @@ methods.addCircleMarkers = function(lat, lng, radius, layerId, group, options, c
       .col("popupOptions", popupOptions)
       .col("label", label)
       .col("labelOptions", labelOptions)
+      .cbind(crosstalkOptions || {})
       .cbind(options);
 
     addMarkers(this, df, group, clusterOptions, clusterId, function(df, i) {
@@ -1148,7 +1150,7 @@ methods.removeMeasure = function() {
   delete this.measureControl;
 };
 
-methods.addSelect = function(crosstalkOptions) {
+methods.addSelect = function(ctGroup) {
   methods.removeSelect.call(this);
 
   this._selectButton = L.easyButton({
@@ -1161,8 +1163,8 @@ methods.addSelect = function(crosstalkOptions) {
           btn.state("select-active");
           this._locationFilter = new L.LocationFilter2();
 
-          if (crosstalkOptions && crosstalkOptions.ctGroup) {
-            let selectionHandle = new global.crosstalk.SelectionHandle(crosstalkOptions.ctGroup);
+          if (ctGroup) {
+            let selectionHandle = new global.crosstalk.SelectionHandle(ctGroup);
             selectionHandle.on("change", (e) => {
               if (e.sender !== selectionHandle) {
                 if (this._locationFilter) {
@@ -1204,7 +1206,7 @@ methods.addSelect = function(crosstalkOptions) {
   this._selectButton.addTo(this);
 };
 
-methods.removeSelect = function(crosstalkOptions) {
+methods.removeSelect = function() {
   if (this._locationFilter) {
     this._locationFilter.disable();
   }

tests/testit/test-remote.R

@@ -150,10 +150,9 @@ assert(
   identical(mockSession$.calls, list())
 )
 mockSession$.flush()
-expected <- list(
-  list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"addPolygons\",\"args\":[[[{\"lng\":[1,2,3,4,5],\"lat\":[1,2,3,4,5]}]],null,null,{\"lineCap\":null,\"lineJoin\":null,\"clickable\":true,\"pointerEvents\":null,\"className\":\"\",\"stroke\":true,\"color\":\"#03F\",\"weight\":5,\"opacity\":0.5,\"fill\":true,\"fillColor\":\"#03F\",\"fillOpacity\":0.2,\"dashArray\":null,\"smoothFactor\":1,\"noClip\":false},null,null,null,null,null]}]}", class = "json"))
-)
+expected <- list(structure(list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"addPolygons\",\"args\":[[[{\"lng\":[1,2,3,4,5],\"lat\":[1,2,3,4,5]}]],null,null,{\"lineCap\":null,\"lineJoin\":null,\"clickable\":true,\"pointerEvents\":null,\"className\":\"\",\"stroke\":true,\"color\":\"#03F\",\"weight\":5,\"opacity\":0.5,\"fill\":true,\"fillColor\":\"#03F\",\"fillOpacity\":0.2,\"dashArray\":null,\"smoothFactor\":1,\"noClip\":false},null,null,null,null,null]}]}", class = "json")), .Names = c("type",  "message")))
 # cat(deparse(mockSession$.calls), "\n")
+dput(mockSession$.calls)
 assert(identical(mockSession$.calls, expected))
 
 
@@ -167,7 +166,7 @@ remote2 <- leafletProxy("map", mockSession,
 )
 # Check that addMarkers() takes effect immediately, no flush required
 remote2 %>% addMarkers()
-expected2 <- list(structure(list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"addMarkers\",\"args\":[[10,9,8,7,6,5,4,3,2,1],[10,9,8,7,6,5,4,3,2,1],null,null,null,{\"clickable\":true,\"draggable\":false,\"keyboard\":true,\"title\":\"\",\"alt\":\"\",\"zIndexOffset\":0,\"opacity\":1,\"riseOnHover\":false,\"riseOffset\":250},null,null,null,null,null,null]}]}", class = "json")), .Names = c("type",  "message")))
+expected2 <- list(list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"addMarkers\",\"args\":[[10,9,8,7,6,5,4,3,2,1],[10,9,8,7,6,5,4,3,2,1],null,null,null,{\"clickable\":true,\"draggable\":false,\"keyboard\":true,\"title\":\"\",\"alt\":\"\",\"zIndexOffset\":0,\"opacity\":1,\"riseOnHover\":false,\"riseOffset\":250},null,null,null,null,null,null,null]}]}", class = "json")))
 # cat(deparse(mockSession$.calls), "\n")
 assert(identical(mockSession$.calls, expected2))
 # Flushing should do nothing
@@ -184,6 +183,7 @@ remote3 <- leafletProxy("map", mockSession,
 remote3 %>% clearShapes() %>% addMarkers()
 assert(identical(mockSession$.calls, list()))
 mockSession$.flush()
-expected3 <- list(structure(list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"clearShapes\",\"args\":[]}]}", class = "json")), .Names = c("type",  "message")), structure(list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"addMarkers\",\"args\":[[10,9,8,7,6,5,4,3,2,1],[10,9,8,7,6,5,4,3,2,1],null,null,null,{\"clickable\":true,\"draggable\":false,\"keyboard\":true,\"title\":\"\",\"alt\":\"\",\"zIndexOffset\":0,\"opacity\":1,\"riseOnHover\":false,\"riseOffset\":250},null,null,null,null,null,null]}]}", class = "json")), .Names = c("type",  "message")))
+expected3 <- list(structure(list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"clearShapes\",\"args\":[]}]}", class = "json")), .Names = c("type", "message")), structure(list(type = "leaflet-calls", message = structure("{\"id\":\"map\",\"calls\":[{\"dependencies\":[],\"method\":\"addMarkers\",\"args\":[[10,9,8,7,6,5,4,3,2,1],[10,9,8,7,6,5,4,3,2,1],null,null,null,{\"clickable\":true,\"draggable\":false,\"keyboard\":true,\"title\":\"\",\"alt\":\"\",\"zIndexOffset\":0,\"opacity\":1,\"riseOnHover\":false,\"riseOffset\":250},null,null,null,null,null,null,null]}]}", class = "json")), .Names = c("type", "message")))
+
 # Check that multiple calls are invoked in order
 assert(identical(mockSession$.calls, expected3))