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

Error in latest main branch on windrose layer ajax call #220

Closed
jywarren opened this issue Jul 1, 2019 · 16 comments · Fixed by #227
Closed

Error in latest main branch on windrose layer ajax call #220

jywarren opened this issue Jul 1, 2019 · 16 comments · Fixed by #227
Labels
bug Something isn't working priority

Comments

@jywarren
Copy link
Member

jywarren commented Jul 1, 2019

Hi, all -- i'm trying to release some additional tweaks to the MapKnitter layer in #219 -- but seeing this error:


(index):89 GET http://localhost:3001/lib/leaflet-environmental-layers/dist/windRoseLayer.js net::ERR_ABORTED 404 (Not Found)
LeafletEnvironmentalLayers.js:12112 Uncaught TypeError: Cannot read property '_leaflet_id' of undefined
    at stamp (LeafletEnvironmentalLayers.js:12112)
    at NewClass.getLayerId (LeafletEnvironmentalLayers.js:18842)
    at NewClass.addLayer (LeafletEnvironmentalLayers.js:18735)
    at NewClass.addMarker (LeafletEnvironmentalLayers.js:26644)
    at NewClass.parseData (LeafletEnvironmentalLayers.js:26652)
    at Object.success (LeafletEnvironmentalLayers.js:26600)
    at fire (jquery.self-e200ee796ef24add7054280d843a80de75392557bb4248241e870fac0914c0c1.js?body=1:3292)
    at Object.fireWith [as resolveWith] (jquery.self-e200ee796ef24add7054280d843a80de75392557bb4248241e870fac0914c0c1.js?body=1:3422)
    at done (jquery.self-e200ee796ef24add7054280d843a80de75392557bb4248241e870fac0914c0c1.js?body=1:9534)
    at XMLHttpRequest.<anonymous> (jquery.self-e200ee796ef24add7054280d843a80de75392557bb4248241e870fac0914c0c1.js?body=1:9786)

Note that in the MK code (in publiclab/mapknitter#775) we are only showing the MK layer, so I wonder if we're making an assumption that the windrose layer will be there?

@ananyaarun @sagarpreet-chadha I'm hoping this will get featured in tomorrow's release of MapKnitter 3.0, so if you have a moment to take a look that would be great! Thank you!

@jywarren jywarren added bug Something isn't working priority labels Jul 1, 2019
@jywarren jywarren changed the title Error in latest main branch Error in latest main branch on windrose layer ajax call Jul 1, 2019
@ananyaarun
Copy link
Member

Ok looking at it @jywarren

@jywarren
Copy link
Member Author

jywarren commented Jul 2, 2019

Thanks!!! This is being seen downstream as well: publiclab/plots2#5971 (comment)

This is a great rehearsal for publishing new versions later! It's nice to see that plots2 will also help us catch such errors 😄

@ananyaarun
Copy link
Member

Hey, so as you said in the call today, basically there are no tests for this layer yet in LEL.
But I guess this broke some test during the integration process.

I'm figuring out writing tests. I'll start with the indigenous layers and once I'm done with that perhaps other layers will be easy.
cc @jywarren

@jywarren
Copy link
Member Author

jywarren commented Jul 2, 2019 via email

@sagarpreet-chadha
Copy link
Collaborator

Hey!
I think it is now solved in the last PR by me . As the windrose.js file was moved from /dist folder to /src folder via some FTO so we needed to change the URL of script file in plots2 . Thanks :)

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

Hi, ok, thanks! Does this mean we no longer need to individually include it?

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

OK, i think i got the windrose one resolved by changing it's path from 'dist' to 'src'.

But I'm still seeing this error in the stamp() method, which seems to be part of Leaflet?

LeafletEnvironmentalLayers.js:12112 Uncaught TypeError: Cannot read property '_leaflet_id' of undefined
    at stamp (LeafletEnvironmentalLayers.js:12112)
    at NewClass.getLayerId (LeafletEnvironmentalLayers.js:18842)

It all begins at this line:

var MapKnitter = L.layerGroup.mapKnitterLayer().addTo(Mapknitterunique);

here:

https://github.com/publiclab/mapknitter/blob/40e21153fba5ed98bca2cc423e00885420158784/app/views/front_ui/index.html.erb#L267

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

Hmm, i'm really struggling with this @sagarpreet-chadha @ananyaarun can you see what's going wrong?

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

In local testing, the mapknitter layer loads. But i see the error and so the spinner never stops:

image

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

It looks like we're getting a null passed out of this function:

              var mapknitter ;
              if (!isNaN(lat) && !isNaN(lng) ){
                if (image_url !== undefined){
                  mapknitter = L.marker([lat , lng] , {icon: redDotIcon}).bindPopup(
                    "<div class='mapknitter-info'><h4>"+ "<a href=" + map_page + ">" + title + "</a></h4>" + 
                    "<p>by " + "<a href=" + url + ">" + author + "</a>" + 
                    " near " + location  + 
                    "</p><a href=" + map_page + "><img src="+image_url+" style='width: 245px;'></a></div>"
                  ) ;
                }
              }

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

I see. I introduced code to check if there's an image. But now if there is no image, it passes back a null layer. Let's be sure it checks ahead of time and stops trying to make a layer. This was my fault, sorry!

jywarren added a commit to jywarren/leaflet-environmental-layers that referenced this issue Jul 5, 2019
jywarren added a commit that referenced this issue Jul 5, 2019
@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

Testing v1.3.3 now!

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

Gah! @sagarpreet-chadha @ananyaarun my apologies, i forgot the dist files need to be built before publishing on NPM now. Doing that as v1.3.4!

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

Wait, something more complex is going on. Moving to #218

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

Resolved!

@ananyaarun
Copy link
Member

Great !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants