ref(dashboards): Simplify shared heat map bucket helpers#117957
Conversation
Reshape the shared heat map bucket helpers from the original Explore-shaped signatures to a simpler form: the X-axis interval is derived from the page filter + container width, and the Y-axis bucket count from the container height via PIXELS_PER_BUCKET (renamed from PIXELS_PER_X_BUCKET). Explore's metric panel is updated to the new signatures. This changes Explore's heat map bucket sizing (the X axis no longer snaps against useChartInterval's option list, and the Y axis is height-based rather than width/aspect-ratio based). Split out of the heat map dashboard widget work, which needs these simpler signatures. Refs DAIN-1654 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With the simplified Y bucket count derived from a constant height, yBuckets is always positive, so the heat map query no longer waited for layout — it fired at width 0 with a coarse fallback interval, then refetched once measured. Gate the query on a measured container width. Refs DAIN-1654
📊 Type Coverage Diff
🔍 4 new type safety issues introducedNon-null assertions (
This is informational only and does not block the PR. |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 225b034. Configure here.
nsdeschenes
left a comment
There was a problem hiding this comment.
Looks all good, just some small non-implementation related questions 😄
| * assertions. Unfortunately heavily limited to only work with `[0]` and not | ||
| * `.at(0)` or any other index. |
There was a problem hiding this comment.
Would it be worth looking into introducing this package, and utilizing this function: https://remedajs.com/docs/#hasAtLeast ?
I know @TkDodo is a maintainer of it :o
There was a problem hiding this comment.
🤷🏻♂️ I don't have a strong opinion! It seems like overkill for this case, but maybe it's worth it?
| '/organizations/$organizationIdOrSlug/events-heatmap/', | ||
| { | ||
| path: enabled ? {organizationIdOrSlug: organization.slug} : skipToken, | ||
| path: !enabled || !valid ? skipToken : {organizationIdOrSlug: organization.slug}, |
There was a problem hiding this comment.
Is this how we "skipToken" apiOptions? :o
There was a problem hiding this comment.
Yeah, skipToken goes in the path position with our stuff
…-dashboards-widget Reconcile with the merged (and reworked) heat map bucket helpers PR (#117957): the two getHeatmapXAxisBucketInterval/getHeatmapYAxisBucketCount helpers were consolidated on master into calculateHeatMapBucketDimensions(selection, {width, height}, availableIntervals) -> {interval, yBuckets} | null. Point the dashboards HeatmapMeasuredArea at the new helper (supplying availableIntervals via getIntervalOptionsForPageFilter), and take master's Explore metricPanel. Keep the dashboards-only HEATMAP_RESIZE_DEBOUNCE_MS setting.
Re-splits the helpers used in Explore for Heat Map calculations, in preparation for Dashboards usage. The biggest change is that I combined the utility that calculates the X and Y buckets into one, since they're always used together. I feel this is less confusing, and it prevents having to re-calculate some intermediate values. I also messed around a bit with the logic of some of the functions, namely:
Refs DAIN-1654