MushroomObserver / mushroom-observer

A website for sharing observations of mushrooms.
https://mushroomobserver.org
MIT License
77 stars 25 forks source link

Observation form phase 2a: matching MO Locations #2211

Closed nimmolo closed 2 weeks ago

nimmolo commented 3 weeks ago

This "phase 2a" PR is a first attempt to smooth out the location section of creating an observation. It tries to guess a Location if we get a lat/lng. It also rearranges the "when" and "where" sections of the form into a tighter configuration, with the "help blocks" now available via "?" icons.

The main new function is it adds a JS listener so that changes to the lat/lng fields, either from user input or image autofill, call up a new autocompleter class, ForLocationContaining. This class queries the db to return all MO locations encompassing the lat/lng given, in order of ascending area. The JS then populates the autocomplete options of the Location input with these locations, and auto-fills the place_name and hidden location_id fields with the first (smallest fitting) location.

Note this does not yet alter the flow for undefined locations, or creating new locations.

A big difference from main is that the create obs form does not try to geolocate a typed place name on Google anymore - these geolocated boxes were not really useful, because they didn’t necessariliy correspond to the bounds of any MO location. Now, the autocompleter returns only defined MO locations. (Tech detail: it returns the bounds of all autocompleted MO locations as hash attributes, so we can draw the location box on the map without an extra DB call).

A possible hitch is the flow of a user trying to orient the map in order to put down a remote point that has no MO locality. Before, the Google reverse geolocation would probably get you close to any intended point; for now people will have to make do with orienting by existing MO locations, and then panning and zooming. The next PR will offer to reverse geocode a new Location in a modal, allowing users to use that as a location within which to drop their pin, with only a few clicks.

Temporarily, it is still true that if you don’t want to use an existing MO locality and you want to define your own, you’d do so by insisting on a new place name, and then optionally creating the location after the obs is created.

Screen Shot 2024-07-04 at 7 27 10 PM


The next PR in this series will make a defined location (or none) the only valid options for creating an observation. "phase 2b" would move the UI for creating a custom location to a pop-up modal, available from the "create_obs" form. It would require any undefined place_name to be created as a db Location prior to saving the obs, rather than offering this optionally, after creating the obs, as is currently the case. (It would still not require a Location however — it would only block creating an obs with an undefined place name. You can create an obs without a place name by clearing the input.)


"phase 2c" is intended to supplement the suggestions in 2a with a call to Google's reverse geolocation service, in the event that MO does not have any matching locations.

coveralls commented 3 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling 8db12b6a5c4df2ed5dc1766f0ed793c5913c9ac9 on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.

mo-nathan commented 3 weeks ago

The "Open Map" widget is nice, but a bit odd in my view. Seems like "Open Map" should become "Close Map" and make it go away if the person doesn't want to see it. I also think the buttons should be disabled if they don't do anything. When I hit "Create Observation" it defaulted to "Wilder Ridge, Humboldt Co., California, USA" and the "Show Point" button doesn't do anything. Seems like all of these buttons should be toggles: "Open/Close" | "Show/Hide Point" | "Show/Hide Locality". There there's no need for "Clear Map". The Point and Locality button should be disabled if you haven't provided that info. All of this is "nice to have" not required.

mo-nathan commented 3 weeks ago

Loaded my first image and this is AWESOME! Thanks for all your work on it. It's huge!

mo-nathan commented 3 weeks ago

Reveals some location weirdnesses, but the cleanup shouldn't be a big deal. For example,

Screenshot 2024-07-05 at 7 35 53 AM

Note "Doane Rock Area" name. This is a much smaller area that in reality doesn't intersect the shown lat/long. See https://www.google.com/maps/place/Doane+Rock+Picnic+Area/@41.8442875,-69.9747906,14z/data=!4m6!3m5!1s0x89fb68ad630b38cb:0x40f4c738eebace7c!8m2!3d41.8442875!4d-69.9568949!16s%2Fg%2F1tz72f6r?entry=ttu.

However, this is a data issue not a functionality issue. It does make we wonder if we should make it easy for users to revise a location when it gets selected like this. Otherwise I expect errors like this will just get ignored.

mo-nathan commented 3 weeks ago

I did find some bugs. If you load an image with more precise GPS and you want to edit the lat/long to be less specific (I was testing it with a matsutake photo from my "spot"), the point of focus jumps around a lot and it's really difficult to get it to behave. I'll send Nimmo the image I'm working with since I don't really want to record it somewhere for posterity with the GPS location. With this image if you try to round off the latitude, it can suddenly jump to some other location off the bottom of Cape Cod.

coveralls commented 3 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling d85c9c28818d0c67f82cf6643e3d6f9e31a6c2bc on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.

coveralls commented 3 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling d85c9c28818d0c67f82cf6643e3d6f9e31a6c2bc on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.

nimmolo commented 3 weeks ago

Branch (and test server) updated.

nimmolo commented 3 weeks ago

Whoops - this PR messes up the locations form. With this currently, you can no longer geolocate a place name from Google. You can only lookup the bounds of an MO location.

Will fix.

coveralls commented 3 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling 98ad2cf7648f1c8453ab0534b969c639f76122e6 on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.

nimmolo commented 3 weeks ago

@mo-nathan @JoeCohen I believe this is robust enough for prime time now. Jason discovered a couple bugs even yesterday, but I believe I've addressed them.

Another bug I found and fixed:

The latest is deployed on test now. Please do test it this weekend if you have time! Map appearance is not testable in system tests, so we must rely on manual testing here.

Note: The retired "show point/show place" buttons are still commented out in the form code. Once we're sure users don't need them, after a week of feedback say, i'll remove in the next PR. I'm pretty sure we don't need them, though. (Thanks to Nathan and Jason for suggesting fewer buttons.)

mo-nathan commented 3 weeks ago

I'll check it out when I get back from a much needed walk.

mo-nathan commented 3 weeks ago

Saw one weird thing that probably isn't a blocker, but it's not what I expected. If you create an observation and drop a photo on it with GPS info and then edit the lat/long, it automatically picked the smallest location name and I wasn't able to get back to the list of locations to select something else. If you haven't edited the lat/long then it works fine. I played around with it some more and couldn't get it to be 100% consistent, but it seemed pretty easy to break especially if I tried to enter very different lat/long info.

mo-nathan commented 3 weeks ago

Another non-blocker are the JS console errors that show up when the page loads: "Incorrect use of

coveralls commented 3 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling 53ce10860f503bfafe6978499b368f1c3fbc9d47 on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.

nimmolo commented 3 weeks ago

Photo with GPS workflow: yep, something's off there. It's not updating the "locality" list when the point moves. Edit: Fixed.

Console errors: that's a Rails-generated non-compliant HTML situation that's been around a long time and is unlikely to ever be fixed, because of the nature of it: The Rails date_select helper generates three inputs with one label, but each input has a different ID — none of which correspond to the label id.

Our label is for the when field, but the fields are named when(1i) when(2i) when(3i).

coveralls commented 3 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling b0c89fec388df798472947bb9679a374440d7137 on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.

coveralls commented 2 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling 8218def2fe8ff18772711dde65c16fe2bebf775b on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.

coveralls commented 2 weeks ago

Coverage Status

coverage: 94.455%. remained the same when pulling 8bba09c832a6a37f4601d5119d2abf21eadc8ef8 on create-obs-location into 9be801bebd94f9562920ff5dffef134f55896942 on main.