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

fix relative protocol clientside for tile urls #526

Merged
merged 5 commits into from
Apr 18, 2018

Conversation

schloerke
Copy link
Contributor

Even with the fix, this error still exists on linux in RStudio with

#now works
leaflet() %>% addTiles()

#still does not work
leaflet() %>% addProviderTiles("CartoDB.Positron")

In the inspector...

Protocol "https" is unknown 

@schloerke schloerke requested a review from jcheng5 April 16, 2018 20:42
@schloerke schloerke self-assigned this Apr 16, 2018
@schloerke
Copy link
Contributor Author

Testing Notes.

All examples should work within RStudio 1.1.x on linux except for the last one. (fails loading https)

devtools::install_github("rstudio/leaflet#526")                                                    
#> Using GitHub PAT from envvar GITHUB_PAT
#> Using GitHub PAT from envvar GITHUB_PAT
#> Downloading GitHub repo schloerke/leaflet@urlProtocol
#> from URL https://api.github.com/repos/schloerke/leaflet/zipball/urlProtocol
#> Installing leaflet
#> '/Library/Frameworks/R.framework/Resources/bin/R' --no-site-file  \
#>   --no-environ --no-save --no-restore --quiet CMD INSTALL  \
#>   '/private/var/folders/0k/bxg5lhr92sq74mb1d446ql540000gp/T/RtmprL0PE8/devtools987352a32ffb/schloerke-leaflet-1536cca'  \
#>   --library='/Users/barret/z_R_library' --install-tests
#> 
library(leaflet)                                                                                   
                                                                                                   
# should be a normal world                                                                         
leaflet() %>% addTiles()                                                                           

                                                                                                   
# should contain a map of Ames, IA with red dots / fire icons                                      
leaflet() %>% addTiles() %>% addProviderTiles("OpenFireMap") %>% setView(-93.6, 42.0285, zoom = 15)

                                                                                                   
# should contain a map of the USA with blue heatmap-like lines/dots over highway areas             
leaflet() %>% addTiles() %>% addProviderTiles("SafeCast") %>% setView(-97, 38, zoom = 5)           

                                                                                                   
# should contain a greyscale map of the world. Fails on linux & RStudio combo                      
leaflet() %>% addProviderTiles("CartoDB.Positron")                                                 

@schloerke schloerke merged commit de3ad5a into rstudio:master Apr 18, 2018
@schloerke schloerke deleted the urlProtocol branch April 18, 2018 18:27
@tim-salabim
Copy link
Contributor

On my linux machine in RStudio viewer:

  1. works fine
  2. works fine
  3. crashes RStudio completely
  4. doesn't show any map

@jcheng5
Copy link
Member

jcheng5 commented Apr 19, 2018

@tim-salabim The crash for 3 is a little scary. 4 is expected.

The situation is not ideal, but I'm guessing this is the best we'll be able to do. For what it's worth, all of these should work perfectly in RStudio 1.2 on all platforms.

@tim-salabim
Copy link
Contributor

Is it worth testing with a daily RStudio build? If so, I will do so tonite

@tim-salabim
Copy link
Contributor

tim-salabim commented Apr 19, 2018

With RStudio version 1.2.567 all 4 examples work as expected, no crashing, no blank viewer panes

@rstudio rstudio deleted a comment from tim-salabim Apr 19, 2018
schloerke added a commit to schloerke/leaflet that referenced this pull request Apr 23, 2018
* master:
  v2.0.0 release tweaks (rstudio#531)
  v2.0.0 init submission to cran (rstudio#530)
  fix relative protocol clientside for tile urls (rstudio#526)
  Add comment about webshot being required for reprex::reprex to work (rstudio#529)
schloerke added a commit to schloerke/leaflet that referenced this pull request Apr 24, 2018
* master:
  v2.0.0 release tweaks (rstudio#531)
  v2.0.0 init submission to cran (rstudio#530)
  fix relative protocol clientside for tile urls (rstudio#526)
  Add comment about webshot being required for reprex::reprex to work (rstudio#529)
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

Successfully merging this pull request may close these issues.

3 participants