trafficonese / leaflet.extras

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

removeDrawToolbar clearFeatures doesn't work anymore #148

Closed jamiemkass closed 5 months ago

jamiemkass commented 6 years ago

leaflet.extras::removeDrawToolbar(clearFeatures = TRUE) doesn't seem to clear the features anymore. Anyone else notice this? I see that now there is "clear all features" option on the edit menu for draw. Was there ever a different way implemented to programmatically remove drawn features? Thanks.

jamiemkass commented 6 years ago

Also found that removeDrawToolbar doesn't work at all when a leaflet proxy is used within a shiny app. It used to work just fine. Please find below a simple example.

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

# Define UI for application that draws a histogram
ui <- fluidPage(

   # Sidebar with a slider input for number of bins 
   sidebarLayout(
      sidebarPanel(
         actionButton("drawOn", "Draw Toolbar On"),
         actionButton("drawOff", "Draw Toolbar Off")
      ),

      # Show a plot of the generated distribution
      mainPanel(
        leaflet::leafletOutput("map", height = 700)
      )
   )
)

# Define server logic required to draw a histogram
server <- function(input, output) {

  m <- leaflet() %>% setView(0, 0, zoom = 2) %>% addProviderTiles('Esri.WorldTopoMap')
  output$map <- renderLeaflet(m)

  # create map proxy to make further changes to existing map
  map <- leafletProxy("map")

  observeEvent(input$drawOn, map %>% addDrawToolbar())
  observeEvent(input$drawOff, map %>% removeDrawToolbar())
}

# Run the application 
shinyApp(ui = ui, server = server)
RobW101 commented 6 years ago

I was using the clearFeatures = TRUE functionality to allow the users to geofence points on the map one drawn shape at a time. 1/ User Draws a shape 2/ Code runs a query based upon the shape's co-ordinates. 3/ Shape is deleted automatically Unfortunately I have had to roll back to an earlier version of the package now the ability to programmatically remove the drawn items is no longer working :-(

RobW101 commented 6 years ago

Just to say the I was only using clearFeatures = TRUE as a workaround. The underlying need to remove features via the code is already covered in #96 (and @timelyportfolio 's demo code in WIP #98 is exactly how I was hoping it would behave) Cheers, Rob

nikkoc commented 6 years ago

I was having issues with removeDrawToolbar on a leaflet proxy today. I was getting "...t.drawToolbar.removeFrom is not a function" JS error in my browser's console.

I have been able to get it to work by replacing the "removeFrom" methods with just "remove" in inst/htmlwidgets/build/lfx-draw/lfx-draw-bindings.js.

However, as I just made this change, I have only verified that it has worked locally. I also haven't gone line-by-line to check if it breaks anything else.

(While the initial error was thrown on t.drawToolbar.removeFrom(t), I also did the replacement for 'o.drawToolbar.removeFrom(o)' in the same file as it seems to get triggered when I add a drawToolbar when there is already one)

laldew commented 5 years ago

Has anyone found a solution for this?

I have a process where the user can utilize the draw toolbar to draw a polygon, and then I take the features from it to hold the data in a reactiveValues dataframe, for the user to go on to do other things with it. Right now the polygon from the draw toolbar does not clear out of the way for the user to interact with the redrawn reactiveValues polygon.

jamiemkass commented 5 years ago

Was wondering if there was any progress on this issue. Is this package still being maintained? There doesn't seem to be any changes to the repo for a year. It would be great if these changes could be made.

nikkoc commented 5 years ago

@ndimhypervol @laldew if you need to get this working sooner rather than later, you can download this package from source, make the change, 'export' the package, then install it locally. That is what I did above to fix the issue. Here's a link to some basics on how to do it in RStudio (you may need to install some other packages to get it to work).

Similarly, it looks like @jeroenclaes made a fork called patch-2 where he made the fix (as referenced in the commit above). You could possibly use that as a quick fix.

I don't exactly know if the issue I experienced is the same as the issue you are experiencing. If you 'inspect element' on the browser's page after you perform the intended action, are there any errors in the console?

RemkoDuursma commented 4 years ago

I am quite sure there is a typo in inst/htmlwidgets/bindings/lfx-draw-bindings.js, lines 8:11 :

if(map.drawToolbar) {
      map.drawToolbar.removeFrom(map);
      delete map.drawToobar;
    }

should be map.drawToolbar, I assume!

I wanted to post this before I had the time to try the fix myself.

jamiemkass commented 4 years ago

@nikkoc Thank you for the fix, and sorry for the late reply.

kaheil commented 4 years ago

I am quite sure there is a typo in inst/htmlwidgets/bindings/lfx-draw-bindings.js, lines 8:11 :

if(map.drawToolbar) {
      map.drawToolbar.removeFrom(map);
      delete map.drawToobar;
    }

should be map.drawToolbar, I assume!

I wanted to post this before I had the time to try the fix myself.

I changed this in the source and reinstalled the package. i still can't get the code to remove the drawtoolbar.

trafficonese commented 4 years ago

The typo is not the source of that error.

One easy workaround would be to use shinyjs and this JS script: shinyjs::runjs("$('.leaflet-draw').remove()") This just removes the toolbar, but doesnt clearFeatures.

I think the error comes from this line: https://github.com/bhaskarvk/leaflet.extras/blob/05c7e4f2cd1cf9ef586f875271cf03673c081e1d/inst/htmlwidgets/bindings/lfx-draw-bindings.js#L197

If it is changed to t.drawToolbar.remove(t) it works as expected.