Fix task runner tools not appearing in Tools toggle menu - #169
Conversation
The HeaderNavigation component always used chartData.chartData.data[variation] to derive the available package managers for the Tools filter. However, for the task-runners route with the 'average' variation, the actual data comes from chartData.taskRunnerAverageData, and for registries it comes from chartData.registryAverageData. Make variationData route-aware so it uses the correct data source, matching the logic already used in VariationPage. Fixes vltpkg/vlt.io#1942 Co-authored-by: Jules Pichette <julespichette@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughHeader navigation now detects average variations and selects route-specific aggregate data for task runner and registry routes. ChangesAverage variation routing
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The header now selects the appropriate aggregate data for average task-runner and registry views, restoring the expected Tools filter options. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Problem
On the benchmarks page at
benchmarks.vlt.sh/#/task-runners/average, the Tools filter menu only shows package manager tools — it doesn't include the 3 task-runner-specific tools (nx, turbo, vp). These tools appear in the charts but users cannot toggle them on/off via the Tools dropdown.Root Cause
In
app/src/components/header.tsx, theHeaderNavigationcomponent computesvariationDatausingchartData.chartData.data[currentVariation]unconditionally. However, when on thetask-runnersroute with the "average" variation, the actual data comes fromchartData.taskRunnerAverageData(notchartData.chartData.data["average"]). Similarly for registries, the data comes fromchartData.registryAverageData.The
VariationPagecomponent already handles this correctly with route-aware conditionals, but the header was missing this logic.Fix
Made the
variationDatacomputation inHeaderNavigationroute-aware:baseRoute === "task-runners"and variation is "average", usechartData.taskRunnerAverageDatabaseRoute === "registries"and variation is "average", usechartData.registryAverageDatachartData.chartData.data[currentVariation](existing behavior)This matches the logic already used in
VariationPage(app/src/components/variation/index.tsx).Fixes vltpkg/vlt.io#1942
Summary by CodeRabbit