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

Add removeMapJunk() options as arguments to mapview() #441

Open
thomaszwagerman opened this issue Sep 30, 2022 · 3 comments
Open

Add removeMapJunk() options as arguments to mapview() #441

thomaszwagerman opened this issue Sep 30, 2022 · 3 comments

Comments

@thomaszwagerman
Copy link

Hi there,

Thank you for this package!

This is a just a suggestion, but I think it would make mapview more intuitive.

removeMapJunk() is a function that allows the removal of elements such as the zoomControl and the homebutton, however it took me a long time to find it as it doesn't appear in any articles or vignettes.

In addition, the mapview() function itself does contain an argument homebutton = FALSE to remove the homebutton. This gives the impression that the other elements cannot be removed. Being able to remove the homebutton in both removeMapJunk() and mapview(), but not being able to do the same with zoomControl seems inconsistent.

My suggestion is to add all removeMapJunk() functionality as arguments to mapview(). I'd be happy to do a PR at some point, but would like to get your thoughts first, as there may be something I have missed!

@tim-salabim
Copy link
Member

Hi @thomaszwagerman thanks for bringing this up. I agree that having homebutton as an argument to mapview() but not the others is inconsistent. However, don't you think having removeMapJunk() is enough? I'm hesitant to add more arguments to mapview() (it has too many as it is)...

library(mapview)

mapview(trails) |> 
  removeMapJunk(
    c("homeButton", "zoomControl")
  )

Created on 2022-10-04 by the reprex package (v2.0.1)

@thomaszwagerman
Copy link
Author

Hi @tim-salabim, thank you for your response (and apologies for the delay in mine, I've been away).

That makes sense - I agree having just removeMapJunk() is enough, especially if the goal is to reduce the number of arguments in mapview(), rather than adding more!

In that case for a PR, how about removing the homebutton argument in mapview(), and adding a brief section in the 5. extra functionality vignette on removing map junk?

@tim-salabim
Copy link
Member

Yeah, that sounds great!

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

2 participants