PB-729 : fix recent earthquake tooltip - #patch #988
Conversation
Passing run #2931 ↗︎
Details:
Review all test suite changes for PR #988 ↗︎ |
|||||||||||||||
| @@ -49,6 +49,9 @@ onUnmounted(() => { | |||
| }) | |||
|
|
|||
| function getPixelForCoordinateFromMap() { | |||
| if (!coordinates.value) { | |||
| return | |||
There was a problem hiding this comment.
I kinda prefer when we are explicit with our returns (I know this works, and I know this returns undefined... but here we know what is inside and we should return null
There was a problem hiding this comment.
I renamed the function to better explain what it does, having a get... was suggesting it was supposed to return something, but that's not the case
| const layer = store.getters.getLayerConfigById(parsedLayer.id) | ||
| if (layer.isHighlightable) { | ||
| // only highlightable feature will output something, for the others a click coordinate is required | ||
| // (and we don't have it if we are here, as we are dealing with pre-selected feature in the URL at app startup) |
There was a problem hiding this comment.
So, I know layer is already used outside of the scope but
I would move this out of the forEach, and have at line 124 somthing like :
124 : const layerConfig = store.getters.getLayerConfigById(parsedLayer.id)
125: if (features !== undefined && layerConfig.isHighlightable) { ...
It doesn't change that much, I must concede that, but I do find it a bit cleaner
There was a problem hiding this comment.
layer was already computed above, so I removed the .getters. call in the for loop and used the already present variable instead
We were missing a `coord` param for the HTML popup endpoint (undocumented) so that the HTML endpoint can select features for us instead of giving feature IDs (not optimal, but that's the way it is for now) Also not requesting geometries (in identify requests) when the layer isn't highligtable, it was returning the extent of the whole layer as geom. Using instead the click coordinate as a geometry (adapting the code to be able to handle no geom)
fa96ee2
to
a2a3331
Compare
We were missing a
coordparam for the HTML popup endpoint (undocumented) so that the HTML endpoint can select features for us instead of giving feature IDs (not optimal, but that's the way it is for now)Also not requesting geometries (in identify requests) when the layer isn't highligtable, it was returning the extent of the whole layer as geom. Using instead the click coordinate as a geometry (adapting the code to be able to handle no geom)
Test link with recent earthquakes
Test link