feat: persist timeline chart metric in URL#3051
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughThe timeline chart now stores and validates its active metric in permalink state, preserves scroll position across query changes, coordinates loading visuals and animation pausing with chart state, and updates the ChangesTimeline URL state and rendering
Sequence Diagram(s)sequenceDiagram
participant PackageTimelinePage
participant TimelineChart
participant URLQuery
PackageTimelinePage->>PackageTimelinePage: preserve scroll on query change
TimelineChart->>URLQuery: read metric query
URLQuery-->>TimelineChart: active metric
TimelineChart->>TimelineChart: reset unsupported metrics to totalSize
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/Package/TimelineChart.vue`:
- Line 190: Validate the value returned by usePermalink in activeTab against the
supported metrics totalSize, dependencyCount, and dependencySize, and normalize
unsupported URL values to totalSize. Preserve the existing permalink behavior
for valid metric values.
- Around line 49-53: Update the TimelineChart setup flow to use the declared
props.permalink value when initializing usePermalink, passing it as the
permanent option so the metric tab respects the requested permalink behavior;
remove the unused prop only if this component should always use
route-query-backed state.
- Around line 193-198: Update the chart animation pause flow around
startChartAnimationPauseTimer so props.loading also prevents the dependency-size
stacked bar from becoming visible while size data is still loading. Re-arm or
defer the 300 ms timer during loading, or include the loading state in the bar’s
visibility/opacity condition, while preserving the existing behavior once
loading completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b0d44c6-c3ab-4f87-b34e-315b091a81d4
📒 Files selected for processing (2)
app/components/Package/TimelineChart.vueapp/pages/package-timeline/[[org]]/[packageName].vue
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
e18e dependency analysisNo dependency warnings found. |
🔗 Linked issue
Resolves #3049
🧭 Context
Timeline page: the selected timeline metric should be persisted in the URL.
📚 Description
Set the active tab from the permalink
Some issues related to the fact that the stackbar chart (dependency size chart) was previously never loaded as the first chart, are fixed:
Both of these issues are fixed by setting the opacity of the chart to 0 while waiting for its internals to stabilize before bumping its opacity back to 1. It's not perfect, a proper solution might be required in the chart library itself, to signal when a chart is stable, but it seems to do the job for now.
The aspect ratio of the ClientOnly's fallback is now also adapted based on the metric, since both chart components have different aspect ratios.
Other: