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

Leaflet-popup-container does not respect graph width when the map is embedded in an RMarkdown document #15

Open
martinschmelzer opened this issue May 10, 2020 · 8 comments

Comments

@martinschmelzer
Copy link

A user on SO reported the following problem:

https://stackoverflow.com/questions/61686111/how-to-fix-distorted-interactive-popup-of-ggplot-figure-in-leaflet-map/61696804#61696804

I tried to locate the problem. One solution might be to set the minimum width equal to the graph width inside popup.js (line 42):

layer.bindPopup(pop, { maxWidth: 2000, minWidth: wdth });

@JMLuther
Copy link

Within RStudio viewer and within RMarkdown preview, this is not an issue.
this issue occurs during rendering to html output.
The CSS fix in the SO response by Martin works well and fixes the issue (thanks for the workaround, btw).

@tim-salabim
Copy link
Member

I'll have a look at this over the weekend (hopefully). I also want to move popupTable creation to js, but my understanding of js and css is limited so bare with me. Or feel free to pitch in with a PR if you feel comfortable in that realm. I want to give the user the option to pass the css instructions somehow so that everyone can style their popups as they want. I think all the necessary infrastructure is there, I just struggle to see through the forest...

@martinschmelzer
Copy link
Author

I am also not a JS wizard but I'll take a look when I find the time :)

@martinschmelzer
Copy link
Author

Locally I added the styles attribute to addPopGraphs which takes raw CSS code like
.leaflet-pop-content-wrapper { background-color: blue; }. In addPopupImages, this code is written to a temporary file which then is added as an htmldependency. This works for both stand-alone leaflet widgets and rmarkdown documents containing them.
For users who are not very comfortable with CSS, the styles attribute could also be a named list like list(wrapper = "wrapper styles...", content = "content styles...".
What do you think about this approach?

@tim-salabim
Copy link
Member

If you can wait another week, I should have sorted out most of it. I now know how to add a popupTable on the js side, so just need to figure out how to best set css options from the r side. I am planning on tackling this from Thursday onwards as we have a long weekend in Germany.

One thing to consider here is that we need to be careful as to what happens when you call addpopupgraph more than once with different css settings...

@martinschmelzer
Copy link
Author

I am in no rush. Concerning multiple calls to any popup generating method, the className .attribute of layer.bindPopup could come in handy. So leaflet-popup-content-wrapper { ... } would become classForCallXYZ leaflet-popup-content-wrapper { ... }

@tim-salabim
Copy link
Member

So leaflet-popup-content-wrapper { ... } would become classForCallXYZ leaflet-popup-content-wrapper { ... }

Nice, I had no idea that CSS works this way. I will try this end of the week and get back to you. If this works, I think we could come up with a very general framework of passing css from R to any htmlwidget... Might even be worth thinking about a stand-alone package for this?

Input a named list -> write to .css -> attach to the widget call

This should be enough, shouldn't it? Are you aware of any exisiting tool for such functionality?

tim-salabim added a commit that referenced this issue May 1, 2021
@tim-salabim
Copy link
Member

tim-salabim commented May 1, 2021

Sorry for the long silence!
This should now work as expected in both interactive and markdown mode. We can pass minWidth via the ... notation in addPopupImages (used under the hood in addPopupGraphs). If not supplied, we append the current width of an image in each bindPopup call for each layer (marker, polygon, line etc).

image

image

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