Skip to content

Conversation

bhaskarvk
Copy link
Collaborator

Auto show/hide legend based on a overlay group.

A lot of people have asked for this feature, where the legend should auto show/hide based on toggling an overlay group. I've modified the example to show how it works.

@bhaskarvk bhaskarvk added the type: enhancement Adds a new, backwards-compatible feature label Mar 5, 2017
@bhaskarvk bhaskarvk requested a review from jcheng5 March 5, 2017 16:10
@bhaskarvk bhaskarvk changed the title Feature/attach legend Auto show/hide legend using layer control Mar 5, 2017
@jakeander
Copy link

Is this supposed to hide a legend when I toggle the layer control on the right? I ran the function and the example and the legend remains on the map.

@bhaskarvk
Copy link
Collaborator Author

Could you upload it on round for me to look at?

@jakeander
Copy link

All I did was run the R/legend.R script from the files changed tab then I ran the inst/examples/legend.R file. The maps with the circle markers pops up but when I toggle the 'circles' group, the legend on the bottom left remains.

@tim-salabim
Copy link
Contributor

It does work for me, both layer and legend are toggled with the layer control. Sweet!! Looking forward to have this in master

@bhaskarvk
Copy link
Collaborator Author

@jakeeander you need to properly install the package, not just source the R files. Before running the example do devtools;:install('rstudio/leaflet')

@bhaskarvk
Copy link
Collaborator Author

Oops sorry you need to install this PR and not the master as this hasn't been merged yet.

@bhaskarvk
Copy link
Collaborator Author

@jcheng5 Could you review and merge this if OK ?

@jakeander
Copy link

I used this PR and it worked. Much appreciated! Great work. This is just supposed to work with the overlayGroups parameter and not the baseGroups parameter correct?

@bhaskarvk
Copy link
Collaborator Author

bhaskarvk commented Mar 25, 2017

For now overlay groups only, I figured base groups are for base maps, but it's not too hard to add the functionality for base map changes too if required.

@jakeander
Copy link

That would be a great addition for me. I've made some maps where the baseGroups parameter switches based on different addPolygons set of data. That works well and then an overlayGroups parameter would be set to locations using addCircles or addMarkers.

@bhaskarvk
Copy link
Collaborator Author

OK I will add that feature

@lbeisteiner
Copy link

any update on merging this? I would really appreciate this feature!

@jcheng5
Copy link
Member

jcheng5 commented Apr 6, 2017

@bhaskarvk Seems like it should respect showGroup and hideGroup as well. Maybe ControlStore should have the concept of groups added to it, and controls that are added with a group can be linked through LayerManager's group layer's events.

@bhaskarvk
Copy link
Collaborator Author

Good point. Unfortunately I have no free time for the foreseeable future to work on this. I think I can get back to this my mid may, but probably not before that.

@bhaskarvk
Copy link
Collaborator Author

Actually on second thought, my PR will work correctly with (show|hide)Group because it will trigger a overlayGroup add/remove event, and the eventlistener in this PR will kick in and add/remove the legend. I just don't have time right now to verify that.

@jcheng5
Copy link
Member

jcheng5 commented Apr 7, 2017

It doesn't, I think because the event only fires when the control is used. But we could fire a similar event from the show/hide methods, that'd be a cheap but still sorta general fix.

@osh-chicago
Copy link

Sorry for the basic question, but would someone please let me know how I can add this update to my version of leaflet?

Thanks!

@bhaskarvk
Copy link
Collaborator Author

I should be able to work on this next week.

@osh-chicago
Copy link

@bhaskarvk Thanks so much!

@bhaskarvk
Copy link
Collaborator Author

Working on a new PR based of master. Closing this.

@bhaskarvk bhaskarvk closed this Jul 9, 2017
@bhaskarvk bhaskarvk deleted the feature/attachLegend branch July 9, 2017 06:43
@osh-chicago
Copy link

Glad to hear!

When do you think this will be ready, please? @bhaskarvk

@bhaskarvk
Copy link
Collaborator Author

Hoping to work on this over the weekend.

@osh-chicago
Copy link

Hi @bhaskarvk - did you get the chance to work on this? Thanks!

@bhaskarvk
Copy link
Collaborator Author

New PR #442.
@jakeander Unfortunately supporting baselayer change is not easy. It's not impossible but I am not fully convinced of the use case to add an ugly hack for the use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Adds a new, backwards-compatible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants