ucd-library / intertwine

California's modern wine network
MIT License
1 stars 0 forks source link

Fine-Tune Connection Arrow Functionality #61

Closed ldragoon closed 4 years ago

ldragoon commented 4 years ago

Arrows do not behave correctly on zoom. Example - arrow too far from location.

image image

Code => app-leaflet-map.js

Solution: Adjust this ‘20’ value based on whether or not the arrow is pointing at a cluster (large circle) or a point (small circle): https://github.com/ucd-library/intertwine/blob/dev/client/public/elements/views/map/app-leaflet-map.js#L261

Detecting a cluster, an example: https://github.com/ucd-library/intertwine/blob/dev/client/public/elements/views/map/app-leaflet-map.js#L413

Make sure the cluster layer has rendered. Then you ask the cluster layer to return you the maker layer: https://github.com/ucd-library/intertwine/blob/dev/client/public/elements/views/map/app-leaflet-map.js#L376 ..

Then sniff test for if the layer is a point https://github.com/ucd-library/intertwine/blob/dev/client/public/elements/views/map/app-leaflet-map.js#L413

In that example, points have the inertWineId property … so if that property exists, its a point. Otherwise it’s a cluster (edited) .

Reference Function

function getRenderedPointType(id) {
  let layer = this.clusters
      .getLayers()
      .find(layer => layer.inertWineId === id);
  layer = this.clusters.getVisibleParent(layer) || layer;
  return layer.inertWineId ? 'point' : 'cluster';
}

It’s not tested… and if the map hasn’t rendered after a zoom event, it could be wrong. That’s why this function checks after a requestAnimationFrame

https://github.com/ucd-library/intertwine/blob/dev/client/public/elements/views/map/app-leaflet-map.js#L389

ldragoon commented 4 years ago

@jrmerz - I almost have the functionality on the arrows right, there is just some unpredictability when you zoom in. Do you mind taking a look?

jrmerz commented 4 years ago

I would revert the function back to returning a value so the name makes sense. You can have it accept a layer if you wish. As stated above, you don't know the type until the map has rendered (unfortunately). The fix to make the function 'zoom safe' would be to add a promise wrapper:

function getRenderedPointType(layer) {
  return new Promise((resolve, reject) => {
    requestAnimationFrame(() => {
      layer = this.clusters.getVisibleParent(layer) || layer;
      resolve(layer.inertWineId ? 'point' : 'cluster');
    });
  });
}

then

  let layerType = await this.getRenderedLayerType(layer);
  if ( layerType === 'point' ) this.distance = 0;
  else this.distance = 10; // cluster
wrenaria commented 4 years ago

This is definitely looking better, but it's still off sometimes, especially when zooming in and out with a connection selected.

Screenshot:

Screen Shot 2020-09-29 at 3 59 34 PM

wrenaria commented 4 years ago

@jrmerz Sometimes when zooming in and out, the arrow moves along the line too far (and the connection word is missing). Example of this:

Base:

Screen Shot 2020-11-24 at 11 41 03 AM

Zoom in:

Screen Shot 2020-11-24 at 11 40 57 AM

Zoom out:

Screen Shot 2020-11-24 at 11 41 10 AM

Example from https://intert-wine-leigh-akwemh35fa-uc.a.run.app/map/chardonnay/connection/V3F2veqZ

On Chrome: It looks like if I load a connection directly this doesn't happen (like clicking above link), but opening a connection within the map does cause it every time. The connection word also vanishes.

On Firefox and Safari: Arrow heads and connection word only show in mouseover and disappear once a connection is clicked. (If I load a connection directly from URL arrow/connection label persists and works as expected.)