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

Allow adding multiple layers to map.on() event handler (h/t @omerbn) #11114

Merged
merged 10 commits into from Oct 12, 2021

Conversation

arindam1993
Copy link
Contributor

@arindam1993 arindam1993 commented Oct 12, 2021

Closes #9995
and adding a small tweak to what @omerbn already did over in #10954.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • manually test the debug page
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog:
  • <changelog>Add support for attaching events to multiple layers with map.on(), allowing users to get retrieve features under the mouse or touch event based on the order in which they are rendered. </changelog>

* If a single `layerId`(as a single string) or multiple `layerIds` (as an array of strings) was specified when adding the event listener with {@link Map#on},
* `features` will be an array of [GeoJSON](http://geojson.org/) [Feature objects](https://tools.ietf.org/html/rfc7946#section-3.2).
* The array will contain all features from that layer that are rendered at the event's point,
* in the order that they are rendered with the topmost feature being at the start of the array.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the order that they are rendered with the topmost feature being at the start of the array.

Is this new statement well tested?

I'm curious if the following unit test still returns the right order if we have ['A1', 'A2'] as layer order and request:

map.on(event, ['A2', 'A1'], spy);

instead of

map.on(event, ['A1', 'A2'], spy);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new unit tests add tests which correctly test if the array is being correctly sent over to the layers property in map.queryRenderedFeatures(pt, {layers: []) and thats what handles the actual ordering of the features. We have a bunch of tests for that in the query test suite.

src/ui/events.js Outdated Show resolved Hide resolved
Arindam Bose and others added 2 commits October 12, 2021 15:07
Co-authored-by: Karim Naaji <karim.naaji@gmail.com>
@arindam1993 arindam1993 merged commit a1e764e into main Oct 12, 2021
@arindam1993 arindam1993 deleted the omerbn-add-multiple-layers-to-map-events branch October 12, 2021 22:27
katydecorah pushed a commit that referenced this pull request Oct 20, 2021
* main:
  Add touch pan blocker to gesture handling for touch devices (#11116)
  Address accessibility issues (#11064)
  add support for non-mercator projections (#11124)
  Image fallback expressions within paint properties (#11049)
  Replaces EPSG:4326 with OGC:CRS84 in GL JS `LngLat` doc (#11072)
  Add globe view support to heatmap shaders (#11120)
  Exclude flaky test (#11118)
  consistify YOUR_MAPBOX_ACCESS_TOKEN as placeholder string (#11113)
  Allow adding multiple layers to `map.on()` event handler (h/t @omerbn) (#11114)
  render-test-flakiness:clear worker storage (#11111)
  upgrade to supercluster v7.1.4, earcut v2.2.3, vt-pbf v3.1.3, geojson-rewind v0.5.1 (#11110)
  Added v1.13.2 changelog entry (#11108)
  One weird JSON.parse() trick (#11098)
  Fixed doc usage of map.getCenter (#11093)
  s̶y̶m̶b̶o̶l̶-̶c̶l̶i̶p̶ dynamic-filtering with `pitch` and `distance-from-camera` expressions (#10795)
  Update link to transpiling guide (#11096)
  Cherry pick 2.5.1 changelog (#11099)
  Fix an iOS15 issue where Safari tab bar interrupts panning (#11084)
  Fix conditional check for isFullscreen to accommodate Safari (#11086)
  Render tests for #11041 (#11070)
@fwextensions
Copy link

I take it this was only done on the v2 branch? I wanted to share an event listener across multiple layers, so I tried adding it using an array of layer IDs on a project using the v1 API. But the listener never fires. No errors, either. It seems to only work when a single layer ID string is passed to map.on().

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

Successfully merging this pull request may close these issues.

Making click event to accept multiple layers
4 participants