[G2M] POIMap clusters #91

geoerika · 2021-09-22T23:03:08Z

Cluster implementation:

Users have the choice to select between cluster or icon mode
When cluster mode is used, cluster layer will be replaced with icon layer when, at any given zoom, cluster calculations don't return any cluster node for the POIs that are displayed in the current viewport.

Storybook: https://60f06c1c5a1772003b824d76-dbmgbrqtav.chromatic.com/
Logic for state use: https://www.figma.com/file/g2ww5nK6pazEWRTlmPKXXV/POIMap---cluster-logic

Recording:

Screen.Recording.2021-09-28.at.9.27.59.AM.mov

…setLayer in it from shared/utils

…lusterLayerData

… we see or not clusters in the current viewport

…luster to state and use it to change map layers and view

…erclusterRadius from constants and shared/utils

…tate; refactor useEffect for showClusters state; add onViewStateChange to get zoom; refactor onInteractionStateChange

…add switch for Show Clusters; adjust mapLayers to the new state; add conditions for switch elements

… to calculate clusters

…onViewStateChange; move setLayerVisibleData to onAfterRender; fix useEffect fro clusterZoom

woozyking

Looks good, minor suggestions

woozyking · 2021-09-28T13:48:56Z

src/components/poi-map/index.js

+    if (mapLayers.includes('POICluster')) {
+      setClusterClick(true)
+    } else {
+      setClusterClick(false)
+    }


perhaps

Suggested change

if (mapLayers.includes('POICluster')) {

setClusterClick(true)

} else {

setClusterClick(false)

}

setClusterClick(mapLayers.includes('POICluster'))

woozyking · 2021-09-28T13:51:21Z

src/components/layers/poi-cluster.js

      const index = new Supercluster({
        maxZoom: props.superclusterZoom,
-        radius: props.getSuperclusterRadius(this.context.viewport.zoom, props.sizeScale),
+        radius: props.getSuperclusterRadius({ zoom: this.context.viewport.zoom }),


```suggestion

radius: props.getSuperclusterRadius(this.context.viewport),

kc-leung

After I get zoom in by clicking a POI and I toggle the show cluster or show radius it zooms me back to its map original view. Is this how it supposed to behave?

Screen.Recording.2021-09-28.at.10.18.27.AM.mov

geoerika · 2021-09-28T16:21:01Z

After I get zoom in by clicking a POI and I toggle the show cluster or show radius it zooms me back to its map original view. Is this how it supposed to behave?

Yes, the integration with POIManage will use the onClick event that will select an activePOI with the result of selecting that activePOI as the only POI on the map, so this is not seen in POIManage. Here is how it works:

Screen.Recording.2021-09-28.at.12.19.24.PM.mov

…state in useLayoutEffect; simplify conditions for switch rendering

… useResizeObserver

…k; reset clusterClick at the end of useLayoutEffect"

woozyking

approved with some minor observations

woozyking · 2021-10-01T19:35:18Z

src/components/poi-map/utils.js

+ */
+const setLayer = ({ layer, props, visible }) =>
+  layer === 'POICluster' ?
+    new eqMapLayers[layer]({ ...props, visible }) :


why does the POICluster type layer require new to instantiate?

Cluster layer is a class component - I am pretty sure you asked that before, like a year ago LOL. When I have time will try change it into function.
Source: https://github.com/visgl/deck.gl/blob/8.5-release/examples/website/icon/icon-cluster-layer.js

woozyking · 2021-10-01T19:37:19Z

src/components/poi-map/utils.js

+ */
+export const isClusterZoomLevel = ({ layerVisibleData, viewportBBOX, zoom }) => {
+  const visiblePOIs = layerVisibleData.reduce((agg, elem) => {
+    return elem.objects ? [...agg, ...elem.objects] : [...agg, elem.object]


would you mean

return Array.isArray(elem.objects) ? [...agg, ...elem.objects] : [...agg, elem.object]

No need, as 'objects' key doesn't exist when we have just one element in Cluster layer. It only exists at all when we have a cluster of locations.
So, this is how an icon with number one looks like in layerVisibleData:

This is how a cluster with several locations looks like:

woozyking · 2021-10-01T19:38:03Z

src/components/poi-map/utils.js

+    return elem.objects ? [...agg, ...elem.objects] : [...agg, elem.object]
+  }, [])
+
+  if (visiblePOIs?.length) {


with a reduce operation like ^ visiblePOIs should be guaranteed as an Array, thus the optional chain should be unnecessary

… there was a delay in the POICluster of calc zoom and we had cluster icons changing quick between 2 zooms; set transitionDuration to 2 sec; remove optional chain for visiblePOI in isClusterZoomLevel

geoerika marked this pull request as draft September 22, 2021 23:03

geoerika changed the title ~~[WIP] Poi map clusters~~ [WIP] POIMap clusters Sep 22, 2021

geoerika force-pushed the POIMap-clusters branch from d2e29f1 to 7066db0 Compare September 23, 2021 19:01

geoerika added 15 commits September 27, 2021 08:34

layers - refactor pickable in poi layers

135a745

POIMap/utils - create utils file for POImap and move processLayers & …

b26c2bd

…setLayer in it from shared/utils

POIMap/utils - edit comments for processLayers

2f29047

POIMap - retrieve current cluster layer elements in the viewport as c…

cc01647

…lusterLayerData

POIMap/utils - add clusterZoomLevel and use in POIMap to determine if…

26a3fb3

… we see or not clusters in the current viewport

POIMap/utils - rename clusterLayerData as layerVisibleData; add showC…

57c13ee

…luster to state and use it to change map layers and view

constants - add CLUSTER_SIZE_SCALE, SUPERCLUSTER_ZOOM

40f3ad1

shared/utils - add getSuperclusterRadius

7533f2b

layers/POICluster - use CLUSTER_SIZE_SCALE, SUPERCLUSTER_ZOOM, getSup…

d6eb0ce

…erclusterRadius from constants and shared/utils

POIMap - rename clusterZoomLevel as isClusterZoomLevel; add zoom to s…

a1a2a96

…tate; refactor useEffect for showClusters state; add onViewStateChange to get zoom; refactor onInteractionStateChange

POIMap/utils - fix function descriptions

c13ba45

POIMap - add clusterZoom to state; set inital showClusters to false; …

d2237fb

…add switch for Show Clusters; adjust mapLayers to the new state; add conditions for switch elements

Stories/poi - rephrase story titles

9663e6a

POImap/utils - add viewportBBOX prop to isClusterZoomLevel and use it…

33efdf4

… to calculate clusters

POIMap - add viewportBBOX to state; calculate viewportBBOX coords in …

c67a587

…onViewStateChange; move setLayerVisibleData to onAfterRender; fix useEffect fro clusterZoom

geoerika force-pushed the POIMap-clusters branch 4 times, most recently from c40f0bb to 20238f9 Compare September 28, 2021 13:31

geoerika requested review from kc-leung and woozyking September 28, 2021 13:40

geoerika changed the title ~~[WIP] POIMap clusters~~ [G2M] POIMap clusters Sep 28, 2021

geoerika marked this pull request as ready for review September 28, 2021 13:40

woozyking reviewed Sep 28, 2021

View reviewed changes

kc-leung reviewed Sep 28, 2021

View reviewed changes

geoerika force-pushed the POIMap-clusters branch from 24029ed to 3ca2e2a Compare September 28, 2021 16:12

geoerika force-pushed the POIMap-clusters branch 5 times, most recently from eef8978 to 55eecfb Compare September 29, 2021 23:47

geoerika changed the title ~~[G2M] POIMap clusters~~ [WIP] POIMap clusters Sep 29, 2021

geoerika marked this pull request as draft September 29, 2021 23:48

geoerika force-pushed the POIMap-clusters branch 3 times, most recently from 1351464 to fe32f81 Compare September 30, 2021 22:58

POIMap - add clusterClick to state and use it to block reset of view …

73e16ff

…state in useLayoutEffect; simplify conditions for switch rendering

geoerika force-pushed the POIMap-clusters branch from fe32f81 to 9236d34 Compare September 30, 2021 23:40

geoerika added 6 commits October 1, 2021 11:07

package - change version to 0.7.2-alpha.6

cdb6718

POIMap - use deckref to calculate viewport with and height instead of…

6d35ba4

… useResizeObserver

hooks - delete useResizeObserver

866ed81

layers/POICluster - refactor radius in new Soperclaster

59d7b2b

POIMap - refactor mapLayers for showRadius; setClusterClick in onClic…

1a57ed2

…k; reset clusterClick at the end of useLayoutEffect"

POIMap - delete clusterClick and simplify useLayoutEffect conditions

e754742

geoerika force-pushed the POIMap-clusters branch 2 times, most recently from a50f011 to 4a600a1 Compare October 1, 2021 15:54

geoerika changed the title ~~[WIP] POIMap clusters~~ [G2M] POIMap clusters Oct 1, 2021

geoerika marked this pull request as ready for review October 1, 2021 17:21

geoerika requested review from kc-leung and woozyking October 1, 2021 17:21

woozyking approved these changes Oct 1, 2021

View reviewed changes

geoerika added 2 commits October 1, 2021 16:06

POIMap/poi-cluster - send props directly from map to cluster layer as…

ef91053

… there was a delay in the POICluster of calc zoom and we had cluster icons changing quick between 2 zooms; set transitionDuration to 2 sec; remove optional chain for visiblePOI in isClusterZoomLevel

package - change version to v0.7.2

73a2aab

geoerika force-pushed the POIMap-clusters branch from 73c24c5 to 73a2aab Compare October 1, 2021 20:07

geoerika merged commit dcdc955 into master Oct 1, 2021

geoerika deleted the POIMap-clusters branch October 1, 2021 20:12

[G2M] POIMap clusters #91

[G2M] POIMap clusters #91

Uh oh!

Conversation

geoerika commented Sep 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

woozyking left a comment

Choose a reason for hiding this comment

Uh oh!

woozyking Sep 28, 2021 • edited by geoerika Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

woozyking Sep 28, 2021 • edited by geoerika Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kc-leung left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoerika commented Sep 28, 2021

Uh oh!

woozyking left a comment

Choose a reason for hiding this comment

Uh oh!

woozyking Oct 1, 2021 • edited by geoerika Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoerika Oct 1, 2021

Choose a reason for hiding this comment

Uh oh!

woozyking Oct 1, 2021 • edited by geoerika Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoerika Oct 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

woozyking Oct 1, 2021 • edited by geoerika Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoerika Oct 1, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

geoerika commented Sep 22, 2021 •

edited

Loading

woozyking Sep 28, 2021 •

edited by geoerika

Loading

woozyking Sep 28, 2021 •

edited by geoerika

Loading

kc-leung left a comment •

edited

Loading

woozyking Oct 1, 2021 •

edited by geoerika

Loading

woozyking Oct 1, 2021 •

edited by geoerika

Loading

geoerika Oct 1, 2021 •

edited

Loading

woozyking Oct 1, 2021 •

edited by geoerika

Loading