search

r-spatial / mapedit

Interactive editing of spatial data in R
https://www.r-spatial.org/r/2019/03/31/mapedit_leafpm.html
Other
218 stars 33 forks source link

make sure drawing happens in a sane place #79

Open tim-salabim opened 6 years ago

tim-salabim commented 6 years ago

See https://stackoverflow.com/questions/51580551/convert-xy-to-lat-long-in-r

We should probably add a check whether drawing happened / is happening within acceptable longlat bounds (-180/180 & -90/90)

tim-salabim commented 6 years ago

@timelyportfolio an attempt at signaling when features are drawn in a location outside the standard longitude range between -180 & 180. This now emits a warning which is possibly highlighted using crayon (if available) in nasty red background with white text :-) . In my opinion, this is sufficient, but we could make sure to mention somewhere that this warning is generated if features are drawn outside sane bounds (which, for whatever reason, is allowed by leaflet). Any other suggestions? Thoughts?

mdsumner commented 6 years ago

Very happy it's just a signal and not an error :)

timelyportfolio commented 6 years ago

@tim-salabim would this require a dependency on crayon? Are you thinking an error from JavaScript or R? Assuming R, and in this case we could add a test when we mutate the state based on Leaflet.draw events. Or would you rather delay the test until completion?

tim-salabim commented 6 years ago

@timelyportfolio crayon is in Suggests: with a if (requireNamespace("crayon")) conditional on the warning. In my experience, I pay even less attention to console output in shiny apps than in interactive mode, that's why I thought highlighting is helpful in this case. With the above commit, this warning is emitted every time (part of) a feature is drawn outside -180 / 180 longitude. I think this is what you mean with "a test when we mutate the state based on Leaflet.draw events", right? If not, I think I need more clarification. Regarding a JS warning, I think it would be great as it would provide more direct feedback especially when working in the browser. Though, I would only do this if we can assure that it doesn't interrupt interaction too much (people may intentionally want to draw outside standard bounds, who knows). I first thought of a alert call, but this would be really annoying I think. If we can find a good place in the window/map to place such a warning, then I would say it is desirable to emit directly via JS (potentially keeping the R warnings as well). But this is something that I would not know how/where to implement.

timelyportfolio commented 6 years ago

@tim-salabim I see now. I missed your commit and misunderstood thinking we were discussing in theory. Adding a JavaScript warning should be fairly straightfoward. I will leave this open as a reminder to work on this once we have finished the prioritized open items.