feat: add downloads estimations on large charts#1177
feat: add downloads estimations on large charts#1177danielroe merged 4 commits intonpmx-dev:mainfrom
Conversation
|
Deployment failed with the following error: |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
📝 WalkthroughWalkthroughThe pull request enhances the DownloadAnalytics component with estimation overlay functionality for incomplete data buckets. It introduces zoom state tracking, end-date period-end detection, and utility functions for UTC bucket calculations and completion ratios. The component now renders SVG overlays for estimations and last datapoints, includes a pending state overlay during loading, and updates chart configuration for dynamic Y-axis scaling. Legend logic is extended to display an estimation item for monthly and yearly granularities. Translation keys for "legend_estimation" are added across locale files (English and French variants), and the vue-data-ui dependency is updated from 3.14.8 to 3.14.9. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/Package/DownloadAnalytics.vue (1)
1139-1264: Consider extracting per-series SVG building to keepdrawEstimationLinemanageable.This function is well over the 50‑line guideline; splitting the per‑series SVG generation into a helper would reduce complexity and make unit testing easier.
As per coding guidelines: "Keep functions focused and manageable (generally under 50 lines)".
| function drawLastDatapointLabel(svg: Record<string, any>) { | ||
| const data = Array.isArray(svg?.data) ? svg.data : [] | ||
| if (!data.length) return '' | ||
|
|
||
| const dataLabels: string[] = [] | ||
|
|
||
| for (const serie of data) { | ||
| const lastPlot = serie.plots.at(-1) | ||
|
|
||
| dataLabels.push(` | ||
| <text | ||
| text-anchor="start" | ||
| dominant-baseline="middle" | ||
| x="${lastPlot.x + 12}" | ||
| y="${lastPlot.y}" | ||
| font-size="24" | ||
| fill="${colors.value.fg}" | ||
| stroke="${colors.value.bg}" | ||
| stroke-width="1" | ||
| paint-order="stroke fill" | ||
| > | ||
| ${compactNumberFormatter.value.format(Number.isFinite(lastPlot.value) ? lastPlot.value : 0)} | ||
| </text> |
There was a problem hiding this comment.
Guard serie.plots before dereferencing the last plot.
Without an array check, serie.plots.at(-1) and lastPlot.x will throw if a series has no plots. Add a safeguard to keep this type-safe.
Possible fix
- for (const serie of data) {
- const lastPlot = serie.plots.at(-1)
-
- dataLabels.push(`
+ for (const serie of data) {
+ const plots = Array.isArray(serie?.plots) ? serie.plots : []
+ const lastPlot = plots.at(-1)
+ if (!lastPlot) continue
+
+ dataLabels.push(`
<text
text-anchor="start"
dominant-baseline="middle"
x="${lastPlot.x + 12}"
y="${lastPlot.y}"As per coding guidelines: "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
2 participants
Uh oh!
There was an error while loading. Please reload this page.