mapbox / mapbox/mapbox-gl-js

Simplify popup handling with a semantic API covering most use-cases

Open
#13,326 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs discussion :speech_balloon:
Dominant language
TypeScript
Stars
12.4k
Forks
2.4k
PR merge metrics
No merged PRs in 30d

Description

Popup code I find is a constant pain point with this library (and MapLibre GL JS). It is too low-level.

Example: Creating popups on hover for tessellating polygons is a surely common requirement, yet one of a number of things that I find is unduly over-complex. Having to mess around with low-level stuff like having to track last selected features on mouse moves is not great. (In general, the library as a whole needs some focus on creating higher-level abstractions to cover common use-cases by moving common stuff in new native options/flags.)

Below is code we use that involves show-on-hover popups on polygons, with simple styling to highlight the current polygon. It's frankly a huge amount of code for such a standard UI concept. It is difficult to get right. There are still issues like flickering popups if the mouse is moved into a popup. I discuss the problems afterwards below.

// Create popup on hover
// See: https://github.com/mapbox/mapbox-gl-js/issues/5539#issuecomment-340311798 and https://gis.stackexchange.com/questions/451766/popup-on-hover-with-mapbox
const popup = new mapboxgl.Popup ({
	closeButton: false,
	closeOnClick: false
});
let lastFeatureId = null;
map.on ('mousemove', polygonLayerId, function (e) {
	
	// Check for feature change (for efficiency, to avoid repeated lookups as the mouse is slightly moved)
	const feature = e.features[0];
	if (feature.id !== lastFeatureId) {		// Relies on generateId being used, or --generate-ids in Tippecanoe
		
		// Reset feature-state of an existing feature if already set, to avoid creating multiple highlighted features; see: https://docs.mapbox.com/mapbox-gl-js/example/hover-styles/
		if (lastFeatureId !== null) {
			map.setFeatureState ({id: lastFeatureId, source: sourceId, sourceLayer: sourceLayer}, {hover: false});
		}
		
		// Update the feature state
		lastFeatureId = feature.id;
		map.setFeatureState ({id: lastFeatureId, source: sourceId, sourceLayer: sourceLayer}, {hover: true});
		
		// Get the centroid
		//const coordinates = onlineatlas.getCentre (feature.geometry);		// #!# Does not work, because the popup ends up in the way so the top half of a feature doesn't get mousemove - seems buggy
		const coordinates = e.lngLat;
		
		// Ensure that if the map is zoomed out such that multiple copies of the feature are visible, the popup appears over the copy being pointed to.
		while (Math.abs (e.lngLat.lng - coordinates[0]) > 180) {
			coordinates[0] += (e.lngLat.lng > coordinates[0] ? 360 : -360);
		}
		
		// Assemble popup HTML
		const popupHtml = popupHtml (feature);
		
		// Populate the popup and set its coordinates based on the feature found.
		popup.setLngLat (coordinates).setHTML (popupHtml).addTo (map);
	}
});
map.on ('mouseleave', polygonLayerId, function (e) {
	if (lastFeatureId) {
		map.setFeatureState ({id: lastFeatureId, source: sourceId, sourceLayer: sourceLayer}, {hover: false});
		lastFeatureId = null;
		popup.remove();
	}
});

Problems here are:

  • Using mousemove and manually tracking a cached featureId is very low-level and does not clearly express the semantic intent;
  • generateId has to be enabled, polluting the data with a public ID property just in order to track state;
  • We have to state explicitly that we want the first feature, which is almost always the case - a library should normally just use the top feature as that is what has technically been pointed at;
  • The world wrap-around problem is just low-level stuff that surely should be abstracted away at library level: the library should already know which popup is being hovered on;
  • Setting a feature state in order to have hover styling is again pretty low-level stuff; it would be better for the library to generate a hovered property state natively, that styles will assume is always there;
  • Popups placed centrally in a polygon obviously overlap with the feature, so mouseing over them while still within the polygon loses that state; in general this feels buggy;
  • Manually adding/removing the popup surely could be done at a more abstract level;
  • Cursor hover state has to be handled manually.

I would like to see a more semantic approach that abstracts away all this stuff at library-level. Something like the following, with simple defaults facilitating the 99% of UI variations people tend to use:

map.addLayer (
  ....
  popups: {
    enabled: true,          // Implied if popups object defined
    contentHtml: callback,  // A callback that receives the hovered feature, so that properties can be read and assembled to HTML
    singleton: true,        // By default, only one ever shown on the whole map
    hoverMode: false,       // Requires click; true to show on hover
    hoverModeSticky: true,  // A hovered popup becomes sticky when the icon clicked on
    centroid: true,         // Or false to use mouse position and track that as it moves, maybe also 'above' for top
    closeButton: true,      // Whether to show a close button
    hoverState: true,       // On by default; turn off to optimise if not needed
    cursorStates: true,     // Use the appropriate cursor in all states
    on: {mouseleave: ...}   // Callbacks to enable tweaking of behaviour per phase perhaps
  }
);

I release this proposal into the public domain without attribution.

For transparency, I first posted the same thing on the MapLibre GL JS site, as the same issues apply.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The issue does not identify specific files or tests. Start by tracing the existing Popup API and addLayer configuration, then determine how the proposed semantic popup options would fit the library’s current event and feature-state handling. Done would require an agreed API design covering the listed common hover and click use cases, with corresponding implementation and tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
frontend, web-dev
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.