Adds "Near Me" Geolocation Mode to the Search Drop Down

[FlipperPA]

This is an initial attempt to request the end user's location on load, and calculate the distance in miles to the meeting. If the user accepts, it will calculate the distance to the first meeting in the array and dump the results to the console.

@joshreisner, what do you think of this approach? My hope was simply to display the distance to each meeting with the address, if the user allows their location to be probed. After talking with our tech advisory group here in Philly, that seems to be the main concern for folks from out of town.

[joshreisner]

Hey @FlipperPA the current WP plugin has three "modes" : Search, Near Location, and Near Me. Only in the "Near Me" mode does it request user geo and show the distance column. You're able to link directly to the "Near Me" mode from the site. Doesn't look like Philadelphia is doing this but most other sites do. I'm open to making a UI change but that'd be separate conversation.

If the goal of this branch is to implement the "Near Me" mode as it works in the plugin that'd be awesome!

Running the branch locally I'm not actually seeing the new column. Do you know what the issue might be?

[FlipperPA]

@joshreisner I was taking a slightly different approach, and requesting the user's location while in the standard search mode. I was hoping to automatically display distance if the user accepts the geocode request, to simplify for the end user. This part of the interface has confused our users in the past, however that may just be a case of the squeaky wheel getting the grease. I'm open to whatever you think is best, as you have the most experience here.

In testing, I'm able to calculate the distance of all of our 1,765 meetings in 134 milliseconds on my machine, so it doesn't seem to be a big hit on performance. I'm not sure how to test the performance hit on a mobile browser (I'm still using console.time / console.timeEnd wrapped around the for loop doing the distance calculations).

It's still definitely a work in progress, but if you pull the latest/greatest, accept the geocode probe, then click the search dropdown and flip to "Near Location" to force a re-render, you'll see it.

I'm still very much a ReactJS noob, and still fumbling my way through the code. Sorry for the slow progress!

[joshreisner]

It's not a question of processing time, it's a question of authorization and user expectations. I don't want the default view to immediately ask the user for their location before rendering the page. I'd prefer they opt in to sharing it, either by clicking a link like "show meetings near me" or selecting an interface element from the top (right now that's "near me" in the dropdown but could be made more prominent)

If the request is immediate, people are going to be more inclined to deny it, and once they've done that, it's pretty much game over. I think only a very slim minority of people are going to go in and re-authorize location sharing.

[FlipperPA]

Fair enough! I'll pivot. Does the code for calculating distance and such look okay? It seems to match up well with the tests I've done at home, but I must admit, my trigonometry is very out of practice. :)

[FlipperPA]

@joshreisner I think this is close to ready to go. Feel free to break out the red pen if you have suggestions on improving anything, as I'm still new to ReactJS. This has been refactored to use the "Near Me" in the dropdown.

[FlipperPA]

@joshreisner PS: you asked earlier about the column not showing up; I've made a modification here to allow "Near Me" mode to show up with local development: https://github.com/code4recovery/react/blob/feature/user-location/src/helpers/data.jsx#L320

[joshreisner]

awesome thanks @FlipperPA ! i promise to review this soon and let you know

[joshreisner]

Hey @FlipperPA so this works really well in testing, but there are a few things I think we want to take care of before we merge this.

1. architecturally, i don't think the filtering belongs in the Controls component, probably it'd be better to move it to the Data helper
2. it's great to check for navigator.geolocation but we should probably do that when we're setting capabilities.geolocation since that's the intent of that var
3. the existing "near me" functionality doesn't simply display distances to the user, it also filters by distance. I think the default is 5 miles, but a new dropdown is exposed in place of the Region / District dropdown with like 1, 2, 5, 10, etc.
4. we should hide the distance column again if they switch modes
5. extremely minor, but we should use const and let instead of var
6. we should try to make the distance more prominent on mobile (the main use-case) like it is in the plugin

Sorry this sounds like a lot of stuff! I'm happy to merge if we can take care of 1 and 2. The rest of the stuff can be deferred for later.

[FlipperPA]

Thanks for the feedback, @joshreisner! I'll be getting back to this in a day or two, as we've just pushed out the new version of our website. You might be interested in seeing this in action: https://www.aasepia.org

:)

[FlipperPA]

@joshreisner I've amended this PR to take care of (1) & (5).

However, I have a questions about (2). The reason for having the navigator.geolocation where I do is that prompts the end user to allow their location to be used. It triggers after they set their mode to "Near Me."

If we were to trigger the request when setting the capabilities.geolocation variable, it would ask them to allow us to use their location before changing their mode to Near Me, which was what I refactored away from, right? Or am I missing something?

I'll add Issues for (3), (4), and (6), so they don't fall through the cracks moving forward.

I hope you're having a lovely holiday season, brother!

[joshreisner]

@FlipperPA navigator.geolocation just checks for the existence of the object; you have to run getCurrentPosition (or better yet probably watchPosition for mobile users) to actually pop the request. https://codepen.io/joshreisner/pen/xxbLPEK

Nice job on the site. I updated your API URL in the app so it's pulling from the new data source now.

[joshreisner]

I see the refactor to move the function, but I'm talking about actually moving the filtering. Let me approach it from a different angle:

See how in your dev environment if you switch the day to Tuesday the URL changes to https://react.test/?day=2 and then if you reload the page then Tuesday is still set? This is important functionality because it lets you permalink to Tuesday.

Similarly, when you set the mode to "Near Me" then the URL changes to https://react.test/?mode=me -- but if you reload it's no longer geolocating. This is because the trigger to geolocate is currently inside the control so you need to click or tap to make it go.

Instead we should remove lines 131-135 from controls.setMode and do all this navigator stuff inside data.filterMeetingData at about line 37, (probably make it an else condition on keyword search).

Hopefully that makes sense? If not lmk, I'm happy to do a zoom. I don't think it will take much more work, it seems very close…

[joshreisner]

Also before I forget, did you want to make 'near me' the default? Because you should be able to do that with this change, and by adding mode: 'me' to your config.defaults

        defaults: {
            title: true,
            mode: 'me', //add me to make the geo happen automatically
        },

[FlipperPA]

AHA, that absolutely makes sense. I think I'll take you up on an hour on video, if that's cool, just so we can get some of this pushed across the finish line without going back and forth. My goal here is to get good enough with ReactJS to help the next person who wants to contribute!

[joshreisner]

Hey @FlipperPA ok lmk what you think of the state of it now. The main thing is: filterMeetingData runs at each render, but we will get a render a) when you change modes, and b) when the getCurrentPosition callback fires. So there's an if statement in filterMeetingData that says do we have a center? If not, ask for current position. If so, then do the filtering (although we're not there yet).

Distances are updated or cleared on each render,

This should actually close #38 -- this is happening now in table.jsx.

Also this should satisfy or get pretty close to #39 as well.

[joshreisner]

Question: should we be storing the center in the query string? It is currently. I could see that as a privacy concern, but also possibly a support win, if someone is able to share what they're seeing in terms of distances.

Related: are we getting too much precision on the geo? Maybe we should limit it to 3 or 4 decimal places? https://www.google.com/search?q=geo+coordinate+precision

[FlipperPA]

@joshreisner I took at look at this, and I think we're getting close. I think we may just need some refinement on passing the URL parameters around depending on the mode.

If I come to the site, and search for bucks county, it gives me what I expect:

If I then switch mode to Near Me, and Allow Location Access, it reverts to the full meeting list without the search term (I believe this is as designed), and adds the distance:

Then, if I switch back to search, it doesn't re-apply the search I had done:

Of course, this might be something we need to add when we put the distance filter in place.

[joshreisner]

Yeah, I don't think we want to preserve the search terms.