Page MenuHomePhabricator

Create clustering MVP with Leaflet.markercluster
Closed, ResolvedPublic

Description

Continue the work started in the T318702: Proof-of-concept: Integrate leaflet clustering with the Nearby layer patch[1] to integrate Leaflet.markercluster as library for the nearby clustering. This includes:

  • Integrating the popups into the clustered points.
  • Add a feature flag for the clustering.
  • Tweak how points are added or removed when the map moves or zooms maybe Its fine to keep points "forever".
  • Popups should work as expected
  • Focus frame should work as expected
  • Allow to load more points at once by tweaking the limit.

Bonus points

  • Tweak settings so they mock the behavior of the iOS App Nearby

Optional

  • Think of ways to load images on demand when hitting the limit

[1] https://gerrit.wikimedia.org/r/c/841462

Event Timeline

Change 845514 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Kartographer@master] Slightly move popup handler creation

https://gerrit.wikimedia.org/r/845514

While working on this one bigger question arose that we should talk about:

  • The current implementation uses a mechanism to cleanup points that have the same location. For performance reasons and to avoid duplicates.
  • By this we're currently hiding the fact, that there might be multiple articles of interest at the same spot. ( They could not have been looked at anyways without some clustering/spreading mechanism )
  • With clustering we now could show all points, even with the same location, but:
  • A first experiment showed that it can happen, that we have >50 points at the exact same spot. ( In one example it's because it's some yearly event and each year's article got the location where it was held at. )

Question: What should we do about that? :-D

Change 845530 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Kartographer@master] Add clustering for nearby feature

https://gerrit.wikimedia.org/r/845530

Change 845514 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Slightly move popup handler creation

https://gerrit.wikimedia.org/r/845514

Change 845530 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Add clustering for the nearby feature

https://gerrit.wikimedia.org/r/845530

Related notes from discussion:

  • Our change affects wikis where nearby will be enabled without clustering: now the overlapping points can be navigated using the <tab> key, rather than silently suppressed. We are kind-of okay with this.
  • An idea came up to replace the multiple, zoom-bracketed nearby point queues with a single FIFO like a circular buffer. No matter how the user is panning and zooming, the nearby points will accumulate in this buffer and the earliest-added points will be deleted.

Change 845639 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Kartographer@master] Simplify nearbyLayer to cluster transparently

https://gerrit.wikimedia.org/r/845639

An idea came up to replace the multiple, zoom-bracketed nearby point queues with a single FIFO like a circular buffer. No matter how the user is panning and zooming, the nearby points will accumulate in this buffer and the earliest-added points will be deleted.

After a longer conversation among devs and some experiments in the code we came to the conclusion that the FIFO approach is not very feasible atm. - Especially with clustering in place it also feels not so urgent to keep the map "constantly" clean by removing points. If the users moves/scrolls a lot it's always transparent how the screen got crowded with data. And the user can always reset manually by toggling the feature.

To still keep the users memory sane we can still reset the points when we hit a limit.

Change 849110 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Kartographer@master] Reset points when hitting a limit

https://gerrit.wikimedia.org/r/849110

This is ready to play with. The only open "issue" is reloading thumbnails when we retrieve more points than we can retrieve thumbnails with one request. It's fine to leave that unsolved until we decide on a general limit of points to retrieve. We can still test how point are spread, loaded and added. They just will not all have thumbnails for now.

Moving to UX review so we can plan a meeting to play with it together.

Change 850118 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[operations/mediawiki-config@master] [beta] Enable Kartographer show nearby clustering

https://gerrit.wikimedia.org/r/850118

Change 850118 merged by jenkins-bot:

[operations/mediawiki-config@master] [beta] Enable Kartographer show nearby clustering

https://gerrit.wikimedia.org/r/850118

Change 849110 abandoned by WMDE-Fisch:

[mediawiki/extensions/Kartographer@master] Reset points when hitting a limit

Reason:

We will move away from auto loading more and more points.

https://gerrit.wikimedia.org/r/849110

Change 845639 abandoned by WMDE-Fisch:

[mediawiki/extensions/Kartographer@master] Simplify nearbyLayer to cluster transparently

Reason:

Not relevant anymore.

https://gerrit.wikimedia.org/r/845639

Clustering is implemented.