trafficonese / leaflet.extras

Extra functionality for leaflet R package.
https://trafficonese.github.io/leaflet.extras/
GNU General Public License v3.0
211 stars 74 forks source link

Adding and removing draw toolbar causes duplicate draw events #215

Closed dfriend21 closed 2 months ago

dfriend21 commented 3 months ago

First off, thanks for the updates that you've done on this package. I've noticed a bug in the development version related to adding and removing the draw toolbar. If you toggle the draw toolbar on and off, it causes the draw events to be fired multiple times. For example, if you toggle the draw toolbar off and on five times and then draw a new feature, the "draw_new_feature" event will get fired 5 times. Here's an Shiny app that demonstrates this.

library(shiny)
library(leaflet)
library(leaflet.extras)

ui <- fluidPage(
  leafletOutput("map"),
  actionButton("add_draw_toolbar", "Add draw toolbar"),
  actionButton("remove_draw_toolbar", "Remove draw toolbar")
)

server <- function(input, output, session) {
  output$map <- renderLeaflet({
    leaflet() |> addTiles()
  })

  observeEvent(input$add_draw_toolbar, {
    leafletProxy("map") |>
      addDrawToolbar()
  })

  observeEvent(input$remove_draw_toolbar, {
    leafletProxy("map") |>
      removeDrawToolbar()
  })

  observeEvent(input$map_draw_new_feature, {
    showNotification(
      paste0("new feature id: ",
             input$map_draw_new_feature$properties$`_leaflet_id`)
    )
  })
}

shinyApp(ui, server)

To reproduce the issue, toggle the draw toolbar on and off using the buttons, and then draw a new feature - I have an observer set on input$map_draw_new_feature that shows a notification, and you'll notice that the number of notifications is equal to the number of times you toggled the toolbar.

This is a bug that appears to have been introduced in the development version - it doesn't happen with the CRAN version (1.0.0). Below I have code creating the same example as above (the code is slightly different because I had to use a work-around to remove the toolbar since removeDrawToolbar() doesn't work in the CRAN version). If you run this code with the CRAN version, the results are as expected - the draw event only gets triggered once, regardless of how many times you toggle the draw toolbar.

Code for CRAN version (1.0.0) ```r library(leaflet) library(leaflet.extras) library(shiny) library(shinyjs) ui <- fluidPage( shinyjs::useShinyjs(), leafletOutput("map"), actionButton("add_draw_toolbar", "Add draw toolbar"), actionButton("remove_draw_toolbar", "Remove draw toolbar") ) server <- function(input, output, session) { output$map <- renderLeaflet({ leaflet() |> addTiles() |> htmlwidgets::onRender(" function(el,x) { map = this; } ") }) observeEvent(input$add_draw_toolbar, { leafletProxy("map") |> addDrawToolbar() }) observeEvent(input$remove_draw_toolbar, { shinyjs::runjs( "map.drawToolbar.remove(map); delete map.drawToolbar;") }) observeEvent(input$map_draw_new_feature, { showNotification( paste0("new feature id: ", input$map_draw_new_feature$properties$`_leaflet_id`) ) }) } shinyApp(ui, server) ```
trafficonese commented 3 months ago

Thanks for reporting! I will look into it!

trafficonese commented 2 months ago

This should be fixed with this commit. Please reopen if its not the case.