spatial index widgets examples #31
Conversation
juandjara
force-pushed
the
feature/spatial-index-widgets-examples
branch
from
f09eb8f to
addb30a
Compare
h3-widgets/index.html
Outdated
| <p class="overline">Variable</p> | ||
| <select name="variable" id="variable" class="select"></select> | ||
| <p class="overline">Agreggation method applied</p> | ||
| <p><pre id="agg-method"></pre></p> |
There was a problem hiding this comment.
I think Prettier should flag this one too but just a heads up that the <pre/> cannot be inside a <p/>, maybe <code /> instead.
There was a problem hiding this comment.
Well this was copied from the previous examples, but we can change it to code or just remove this section
There was a problem hiding this comment.
Ah ok! I'll defer to @srtena if he prefers something else, but your changes look good to me.
There was a problem hiding this comment.
Agree it should be code... Those two old examples have a few other issues 🙃 I'll try to find some time to improve them but low priority
h3-widgets/index.ts
Outdated
| debouncedRenderWidgets() | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
After zooming in to the southern tip part of Melilla, such that only one h3 bin is visible, I noticed that the total population in the tooltip doesn't match the formula widget:
This might happen if the resolution of the tiles doesn't match what the Widgets API computes? Sorry this one is tricky to reproduce... just want to be sure we have the request parameters correct.
There was a problem hiding this comment.
This was also caused by the bug above. I think it might be resolved now.
There was a problem hiding this comment.
This still occurs for me, but I can see now that it's zoom-level dependent. If I zoom in enough, both the widget and the tile will agree on the "3666" result. If I start zooming out, the widget result increases (as if it's fetching a larger H3 cell?) first, and the size of the H3 cell displayed on the map does not change until I've zoomed much farther out.
There was a problem hiding this comment.
Hi Don! I'm trying to replicate this and cannot make it. Maybe it is fetching some h3 cells that are behind the white panel ?
There was a problem hiding this comment.
If you have some time today maybe we could do a quick meet with some live debugging
There was a problem hiding this comment.
I think it might be strictly a problem of the 'spatialFiltersResolution' changing at a time when the rendered tile H3 resolution has not changed. Here are two screenshots at slightly different zoom states, one matching and the other not:
Matching
Non-matching
Maybe a next step would be just to see if the same thing currently happens in C4R? If so then at least we know it isn't a new problem.
There was a problem hiding this comment.
After re-installing npm dependencies and getting latest @carto/api-client, unfortunately still seeing the same thing here.
There was a problem hiding this comment.
As discussed earlier today – the same mismatch happens in Builder/C4R as well:
h3_mismatch.webm
So whatever else is going on here, you've implemented the logic consistently with C4R! Assuming the Builder/C4R behavior is fine for now, I think we should just add a comment to getSpatialFiltersResolution in the @carto/api-client PR to keep track of this info, but it doesn't need to block the changes for this initiative.
/cc @srtena @padawannn just FYI.
There was a problem hiding this comment.
For future reference the demo is using carto-demo-data.demo_tables.derived_spatialfeatures_esp_h3res8_v1_yearly_v2, just south of Melilla.
There was a problem hiding this comment.
Agreed to keep that as a known issue
There was a problem hiding this comment.
LGTM for code review, after local testing, nice work!
/cc @srtena any other feedback on this one? I haven't reviewed text and styles closely, defer to you on those!
srtena
force-pushed
the
feature/spatial-index-widgets-examples
branch
from
2045d60 to
a8f4bcc
Compare
related story: https://app.shortcut.com/cartoteam/story/454270/example-for-deck-gl-examples