Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

addFgb fillColor not working #24

Open
DavZim opened this issue Jul 10, 2020 · 11 comments
Open

addFgb fillColor not working #24

DavZim opened this issue Jul 10, 2020 · 11 comments

Comments

@DavZim
Copy link

DavZim commented Jul 10, 2020

When trying to set fillColor in addFgb(), the polygons are not colored...

library(leaflet)
library(leafem)
library(sf)
#> Linking to GEOS 3.8.0, GDAL 3.1.2, PROJ 7.0.0

fgb_file <- tempfile(fileext = ".fgb")
st_write(mapview::franconia, fgb_file)

leaflet() %>% 
  addTiles() %>% 
  leafem::addFgb(fgb_file, fillColor = "red", color = "black", weight = 1) %>% 
  leafem::addMouseCoordinates() %>% 
  setView(10.8, 49.8, 7)

image

I would expect to see a map like this

# expected
leaflet() %>% 
  addTiles() %>% 
  addPolygons(data = mapview::franconia, fillColor = "red", color = "black", weight = 1)

image

Ps. Loving the fgb functionality, really helpful for larger datasets!

pps. If fgb_file points to an empty file, the r session crashes (at least in RStudio).

leaflet() %>%
  addFgb(file = "totally-not-existing.fgb")
@tim-salabim
Copy link
Member

Thanks! Will look into it shortly.

pps. If fgb_file points to an empty file, the r session crashes (at least in RStudio).

Just to be clear, are you saying that the file does not exist or that it exists but is empty?

@DavZim
Copy link
Author

DavZim commented Jul 10, 2020

The crash happens if the file does not exist.

Regarding the coloring mapview works.

library(mapview)
#> GDAL version >= 3.1.0 | setting mapviewOptions(fgb = TRUE)
mapview(franconia, col.regions = "red")

image

But I cant find the color/fill argument in the @map slot.

@tim-salabim
Copy link
Member

Yeah, the fillColor is currently written to the fgb file and then used to color the features when rendering. That may be the culprit wh it's not working natively. I'll investigate tonight

@DavZim
Copy link
Author

DavZim commented Jul 10, 2020

If I can be of any help, let me know!

@tim-salabim
Copy link
Member

tim-salabim commented Jul 10, 2020

Thanks for the offer!

This is all pretty alpha still. Long term plan is to supply some sort of colorOptions() containing e.g. the palette definition and breaks or colum name (zcol) and then have the coloring done on render using chroma.js. Would save creating and writing a color vector, but I am not quite sure what these options need to have to cover all cases. It's basically an iterative process with bugs caught in the process... :-)

Do you know any JavaScript?

@DavZim
Copy link
Author

DavZim commented Jul 10, 2020

Unfortunately I haven't looked into JS yet.

Some colorOptions would be great, especially if it enables to recolor the polygons instead of redrawing them (along the lines of this leaflet issue/solution).

@tim-salabim
Copy link
Member

When trying to set fillColor in addFgb(), the polygons are not colored...

This can be solved with setting fill = TRUE. The documentation says

fill whether to fill the path with color (e.g. filling on polygons or circles).

If you have any ideas on how to improve that I'm all ears.
The problem here is that we're using the same function for all types of features without knowing ahead of time what features to expect (unless we'd use gdalinfo or something - which I'd like to avoid). I made the default to be FALSE because in case of linestrings this is the only reasonable choice.

pps. If fgb_file points to an empty file, the r session crashes (at least in RStudio).

I've now included a file.exists call, if FALSE we stop with an error message.

@DavZim
Copy link
Author

DavZim commented Jul 11, 2020

This closes the issue for me. The not existing file throws an error and doesnt hang the session anymore.

I didn't know I had to set fill=TRUE when fillColors are present, seems kinda unintuitive for me.

Maybe something like addFgb <- function(<...>, fillColor = NULL, fill = !is.null(fillColor), ...) { <...> if (is.null(fillColor)) fillColor = color would help. If fillColor is set, fill is automatically set to TRUE, but if not, the old color argument is still used.
Additionally, mentioning to use fill = TRUE when using fillColor in the documentation would be helpful as well.

Otherwise thank you for the fast fixes!

@tim-salabim
Copy link
Member

Thanks, makes sense. We now have default fillColor = NULL. If set, we set fill = TRUE. If fillColor = NULL and fill = TRUE we set fillColor = color.

@tim-salabim
Copy link
Member

@DavZim heads-up, I had to revert this, as it messed up things in mapview. I need a bit more time to figure out a proper way of handling this on both sides but am pressed for time a little as I have a mapview course to teach end of August and need this to work from that side. Reopening so I don't forget (hopefully).

@tim-salabim tim-salabim reopened this Jul 24, 2020
@kptitov
Copy link

kptitov commented Aug 25, 2021

fillColor works when I add a column called fillColor to the .fgb file before passing it to addFgb.
However, I can't seem to pass highlightOptions to leaflet so that the polygons I add with addFgb would behave in the same manner as the ones I can add with the standard leaflet::addPolygons. Would you have any advice on this?
thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants