Uh oh!
There was an error while loading. Please reload this page.
Add pivot/funnel/grouped-bar widget type tests and update ROADMAP - #720
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add dedicated tests for pivot, funnel, and grouped-bar widget types - Add integration test for dashboard combining all three types - Include pivot, grouped-bar in ChartTypeSchema acceptance test - Mark protocol support complete in ROADMAP, separate from renderer impl Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Adds missing test coverage for pivot, funnel, and grouped-bar dashboard widget types (already present in the protocol), and updates the dashboard roadmap to distinguish protocol completion from renderer implementation work.
Changes:
- Extend
DashboardWidgetSchematests to validatepivot,funnel, andgrouped-barwidgets (standalone +chartConfig+ full dashboard composition). - Update the
ChartTypeSchemaacceptance list indashboard.test.tsto includepivotandgrouped-bar. - Update
ROADMAP.mdto mark protocol support as complete and track chart renderer work separately.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/spec/src/ui/dashboard.test.ts | Adds test coverage for pivot/funnel/grouped-bar widget types and chartConfig integration. |
| ROADMAP.md | Marks protocol support items complete and splits out pending renderer work for plugin-charts. |
| const types = ['metric', 'bar', 'line', 'pie', 'funnel', 'table', 'bubble', 'gauge', 'heatmap', 'pivot', 'grouped-bar']; | ||
| types.forEach(type => { |
There was a problem hiding this comment.
The test name says it "should accept all chart types", but the types array is only a small subset of ChartTypeSchema (e.g., missing horizontal-bar, area, donut, scatter, etc.). This makes the test misleading and easier to accidentally desync from the enum. Consider either enumerating all ChartTypeSchema.options (or deriving from ChartTypeSchema.Enum) or renaming the test to indicate it’s only checking a representative subset (or removing this test and relying on chart.test.ts, which already covers categories).
| consttypes=['metric','bar','line','pie','funnel','table','bubble','gauge','heatmap','pivot','grouped-bar']; | |
| types.forEach(type=>{ | |
| consttypes=ChartTypeSchema.options; | |
| types.forEach((type)=>{ |
Protocol already defines
pivot,funnel, andgrouped-barinChartTypeSchema(used byDashboardWidgetSchema.type), but test coverage was missing and ROADMAP didn't reflect protocol completion.Tests (
dashboard.test.ts)pivotwith measures,funnelwith categoryField,grouped-barwith valueField)chartConfigintegration for all three typespivot,grouped-bartoChartTypeSchemaacceptance test listROADMAP
funnelandgrouped-baras completeplugin-charts) as separate pending items for objectui coordination5,587 tests pass, 0 CodeQL alerts.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.