RagtagOpen / nomad

Apache License 2.0
10 stars 22 forks source link

[WIP] first pass of destination page #721

Closed tkell closed 6 years ago

tkell commented 6 years ago

A shot at #674 – comments & corrections welcome!

jillh510 commented 6 years ago

When there are no carpools for the destination, the page stays on "Loading carpools for this destination...." indefinitely.

tkell commented 6 years ago

Ty @jillh510, will take a look at the JavaScript to see what is happening.

tkell commented 6 years ago

Hmm, @jillh510, I get taken to No carpools nearby. when there are no carpools? I assume you get that consistently?

jillh510 commented 6 years ago

I get stuck on "Loading carpools for this destination...." even if there are carpools. Huh.

TheUnlocked commented 6 years ago

When it gets stuck on "Loading carpools for this destination....", the console emits the errors:

Uncaught SyntaxError: Identifier 'markers' has already been declared
    at script.js:1

and

Uncaught ReferenceError: map is not defined
    at doSearch (find.js:105)
    at setLatLng (find.js:36)
    at geoSuccess (find.js:97)

in that order. When it successfully shows the carpools, it still emits the first error, but it otherwise works.

TheUnlocked commented 6 years ago

It looks like a race condition. I'm not confident enough with git to PR onto a PR, so here are my edits to make it work:

Top of find.js:

let map;
// Other declarations

Replace the geoSuccess(position) definition in find.js with

function geoSuccess(position) {
    if (!map){
        // wait for the map to load before proceeding
        setTimeout(() => geoSuccess(position), 100);
        return;
    }
    // zoom to user position if/when they allow location access
    const coords = position.coords;
    setLatLng(coords.latitude, coords.longitude);
    zoomMap();
}
jillh510 commented 6 years ago

In my version setLatLng is never getting called, which means doSearch never gets called. I've blocked the app from figuring out my location, in case that makes a difference.

jillh510 commented 6 years ago

Wondering if the problem is here: if (!userQuery && !userLatLon.lat && navigator.geolocation) { navigator.geolocation.getCurrentPosition(geoSuccess); }

navigator.geolocation.getCurrentPosition seems to get called, but geoSuccess never does. Maybe we need an error handler for the case where the user has denied access to their location.

jillh510 commented 6 years ago

If I change it to ' navigator.geolocation.getCurrentPosition(geoSuccess, function () {alert('foo');});', I do see the foo alert

tkell commented 6 years ago

Ah, so it turns out I was doing things totally incorrectly. I've now got the JS to a better place, but I am not sure how to generate the geoJSONURL that the doSearch function needs. Will keep starting at it, but hopefully someone knows how to get that data.

tkell commented 6 years ago

OK @jillh510, I think I have the page working properly! Do we want anymore links, or a marker to indicate the destination itself?

jillh510 commented 6 years ago

I'm curious why we're using the code in find.js at all, given that the template already has the list of current carpools headed to that destination.

dryan commented 6 years ago

Hey @tkell, since we've got folks starting to use this this weekend, can we punt on the map for now. If we can get the list of starting points to look like the list on /carpools/find we'll be in good shape for now.

screenshot 2018-10-12 18 44 55
tkell commented 6 years ago

@jillh510, this is an excellent question! I will see if I can simplify things.

@dryan, sorry, I did not see that this was supposed to be done by this weekend - and I am not sure what you mean by punting / not working on the map?

Will work on this more tomorrow evening, EST – feel free to hack on it as needed!

dryan commented 6 years ago

No problem. What I’m saying is if we can just get a list of places that people can leave to get to a destination that’s enough for now.

tkell commented 6 years ago

@jillh510, the JS does a ton of formatting that does not appear to be easily accessible from within the destination template, so I think I'd like to stick with it?

tkell commented 6 years ago

OK, I think this is working!

The JS is not great – going to see if I can split the find.js code out into another file.

tkell commented 6 years ago

k, @jillh510 and @dryan, please let me know what you think!

tkell commented 6 years ago

Closing as we're going to us #730 !