Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mvl22 opened this issue Nov 13, 2024 · 2 comments
Open

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

mvl22 opened this issue Nov 13, 2024 · 2 comments

Comments

@mvl22
Copy link
Contributor

mvl22 commented Nov 13, 2024

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.

@stevage
Copy link
Contributor

stevage commented Nov 15, 2024

You may find map-gl-utils a useful library for "higher-level" functions.

It has a hoverPopup method (https://stevage.github.io/map-gl-utils/#hoverpopup).

@mvl22
Copy link
Contributor Author

mvl22 commented Nov 15, 2024

You may find map-gl-utils a useful library for "higher-level" functions.

Indeed; I'm aware of that library. I think it exposes the point that common-use cases like this are too low-level and that the library itself should abstract this stuff away. I'm certainly not proposing the low-level abilities should be removed obviously, but the core developer experience should be made much more straightforward for this commonly-needed use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants