selectFeatures draws on world map even if crs = NA (for mode = "click" only)

[tim-salabim]

When using selectFeatures with sf features with no crs (i.e. NA) only mode = "draw" renders the features correctly without a background map. mode = "click" tries to plot things on a world map in web mercator. Should behave as mode = "draw"

library(mapedit)
library(mapview)
library(leaflet)
library(sf)

fran = st_set_crs(franconia, NA)

## as expected
selectFeatures(fran, mode = "draw")

## drawing on world map - not what should happen
selectFeatures(fran)

[timelyportfolio]

@tim-salabim I think the only difference is in lines for "click" and no extent in line for "draw". If I understand correctly, you prefer "draw", which would mean we remove lines 72-80. Is this correct?

[tim-salabim]

I'll look into this soon. We simply need to adhere to the crs of the data, should be an easy fix

[tim-salabim]

OK, this is fixed and merged in master

[timelyportfolio]

@tim-salabim thanks! do we also need the change here

mapedit/R/select.R

Line 107 in 8328411

drawn = editMap(mapview::mapView(x, map = map, layer.name = nm, ...), title = title)

?

[tim-salabim]

@timelyportfolio no, the mapview call will automatically set up the map according to the projection of x.

[timelyportfolio]

@tim-salabim after rhub::check_for_cran I discovered this uses unexported mapview:::initMap. I added a copy in mapedit similar to other unexported. Is this ok with you? Will you verify that this copy works on your side?

This was the only issue from rhub::check_for_cran(). Did you see anything on winbuilder?

Thanks!

[tim-salabim]

@timelyportfolio good catch! Sorry for the oversight... I just had a closer look at the package code (it's been a while) and I think we have a similar issue to this for editFeatures. I will investigate shortly.

Turns out, we're handling this correctly, as we can only allow editing if projection is 4326 (longlat). So we reproject if data is in different projection or through a descriptive error if no projection information is present.

I just tested on winbuilder (R-devel-ATC, normal R-devel wasn't accepting upload) and status is OK. See https://win-builder.r-project.org/B54RObZ26z92/ for details (only valid for about 72hrs).

Likely not for the next CRAN release of mapedit but I will also investigate whether it makes sense to export initMap from mapview. I don't quite like the code duplication as a long-term solution (I have similar issues throughout other packages).