search

falling-fruit / falling-fruit-web

Mobile-friendly website for Falling Fruit
https://beta.fallingfruit.org
GNU General Public License v3.0
29 stars 7 forks source link

Refactor: state of current location/review should be in Redux #406

Open wbazant opened 2 weeks ago

wbazant commented 2 weeks ago

Many FallingFruit activities are associated with a single location: showing details of it, editing the form, moving the marker around the map, etc. We have a good URL schema for them - '/locations/:locationId' identifies those pages - but the data about current location is not globally available. I'm coming to a conclusion that it creates weird places in the code / gaps / bugs.

  1. updateEntryLocation in src/redux/mapSlice.js, whose job it is to resolve URLs without the @ like /locations/1989068 to /locations/1989068/@55.82696200000001,-4.260660999999999,16z , is called from src/components/entry/EntryWrapper.js, whose job it is to display the location. A smell, plus I realised now it comes with a small bug: /locations/1989068/edit/ is not resolved with the same @.

  2. a "new location" marker is in the middle of the map and the map is made to move in the background, even on desktop where moving the marker would arguably be more intuitive

  3. there was no good way to add going back and forth between position and location screens for the PR to edit location position, see https://github.com/falling-fruit/falling-fruit-web/pull/396#issuecomment-2174495405

  4. having the location open, and then clicking "edit", results in doing the API call again for no particular reason

  5. no place to store the state of location drawer, #359

There's also a similar smell associated with the lack of the current review being globally available, but AFAIK this is only a thing in the context of a user editing their own review, and the map needing to know location ID of that review, which I worked around by making a look-up object in src/redux/miscSlice.js.

ezwelty commented 2 weeks ago

Thank you for really digging into the implicit logic buried in the code. This really resonates with me. #359 has many flavors, all related to navigating back from the mobile view location to other pages: Street View (see #360), /imports/:id, etc. Just as editing a location calls the API again unnecessarily, so does editing a review.

Thinking back to https://github.com/falling-fruit/falling-fruit-web/issues/398#issuecomment-2180392371, I wonder whether this would also facilitate accessing settings and returning to the currently selected location, although that may be best addressed by adopting the single-page-app style solution used on the current mobile app: putting the settings in a drawer that opens from the left over the rest of the app, which would restore the ability to switch basemaps, labels, etc without reloading the map.