falling-fruit / falling-fruit-web

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

More dynamic UX for cluster click #423

Closed wbazant closed 1 month ago

wbazant commented 3 months ago

Zooming by 1 after a click results in slow, frustrating UX.

After playing around, zooming in by 3 (so into 1/8 of the original screen) seems to be just on the edge, sometimes it's slightly too far into the cluster (so we don't see all the dots corresponding to the cluster, just most of them) but mostly results in e.g. zooming into the right country or city.

Also, if there's just one location somewhere remote, we might as well zoom into it directly.

The formula for delta zoom given cluster count, how many other clusters there are near it, etc. could probably get very complicated if we wanted it to, but for now I'm proposing this:

-    mapRef.current?.setZoom(view.zoom + 1)
+    mapRef.current?.setZoom(cluster.count === 1 ? VISIBLE_CLUSTER_ZOOM_LIMIT + 1 : Math.min(VISIBLE_CLUSTER_ZOOM_LIMIT + 1, view.zoom + 3))
wbazant commented 3 months ago

I just noticed, I probably also need to change this in fetchClusters:

src/redux/mapSlice.js
export const fetchMapClusters = createAsyncThunk(
  'map/fetchMapClusters',
  async (_, { getState }) => {
    const state = getState()
    return await getClusters(               
      selectParams(state, { 
        zoom: state.map.view?.zoom ? state.map.view.zoom + 1 : null,
      }),                                             
    )                                
  },                      
)
ezwelty commented 2 months ago

@wbazant Your suggested setZoom is a good start, but it would be possible to do better by using setBounds rather than setZoom. The clusters are computed on a nested quadtree which, for a given zoom level, divides the Earth into a 2^zoom x 2^zoom grid of equal-sized cells in the Web Mercator projection (EPSG:3857). Since we currently use zoom+1 clusters for a given map zoom, this means each Google Map tile (which use the same quadtree) is split into 4 cluster cells.

What this means is that the latitude, longitude of the cluster (what is returned by the API) is sufficient to determine the map bounds of the corresponding grid cell. The logic is worked out in https://github.com/falling-fruit/falling-fruit-api/blob/main/helpers.js, see wgs84_to_mercator, mercator_to_gridcell, and the commented out gridcell_to_mercator and mercator_to_wgs84 – I could write up a function for this if desired. But this would be the maximum bounds. Alternatively, I could look into adjusting the API to return the actual bounds of the locations within the cluster.

wbazant commented 2 months ago

That would be better, since the zoom heuristic also depends on the viewport - I was thinking zooming in by a factor of 3 will work when the cluster grid is at least 8x8 on the screen, as you point out that needs two tiles.

I had a go at having your suggestion implemented by an AI assistant this morning, would you check https://github.com/falling-fruit/falling-fruit-web/pull/419/commits/42f77939fae56696ccd106aaad5569a0c4a6e840 ?

The math seems legit - at least as a series of transformations, I didn't try to understand them, but I gave Claude your file and the requirements, and it came up with:

 _.cluster_to_bounds = function({lat, lng, zoom}) {                                                                                                                                           
   const mercator = _.wgs84_to_mercator({x: lng, y: lat})                                                                                                                                     
   const cell = _.mercator_to_gridcell(mercator, zoom)                                                                                                                                        
   const sw = _.gridcell_to_mercator(cell)                                                                                                                                                    
   const ne = _.gridcell_to_mercator({x: cell.x + 1, y: cell.y + 1, zoom: cell.zoom})                                                                                                         
   return {                                                                                                                                                                                   
     sw: _.mercator_to_wgs84(sw),                                                                                                                                                             
     ne: _.mercator_to_wgs84(ne)                                                                                                                                                              
   }                                                                                                                                                                                          
 } 

and then we unpacked the implementations of your functions. I also tested it a bit and it seems to zoom in quite sensibly.

Returning the cluster bounds from the API would tie up with our proposed resolution for #378 and be much better for sparsely located locations!

ezwelty commented 2 months ago

Neat, the AI did a good job refactoring my code! I simplified and added a comment, but the math was right. (https://github.com/falling-fruit/falling-fruit-web/pull/419/commits/a8da6cba1f2ea9cf1f7a7cdb57c0efc1276d9850)

wbazant commented 1 month ago

I think the live site works as proposed now, and when a cluster ends up splitting into several clusters it's very nice and dynamic - maybe the during the zoom-in the clusters could be made opaque that shows they're being discarded, instead of being just replaced.

If I'm going after a small cluster I end up having to 'chase' it around the map. I'm not sure what to do about it but I'll keep the issue open for a bit more

ezwelty commented 1 month ago

@wbazant Until I look into adding bounding boxes to the API's GET /clusters (if performance allows, since this would have to be computed on the fly for each type filter), does it make sense to bring back the special deep zoom for count-1 clusters?