fix(chart): bar brush selection stats#261
Conversation
📝 WalkthroughWalkthrough
ChangesChart builder selection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
hfl0506
added
bug
hfl0506
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/src/lib/brushSelection.test.ts (1)
4-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the remaining selection edge cases.
This only covers primitive grouped bars. Add cases for tuple/object values,
null/non-finite values, and duplicate(seriesIndex, dataIndex)entries across overlapping regions so the required filtering and deduplication behavior cannot regress.🤖 Prompt for 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. In `@ui/src/lib/brushSelection.test.ts` around lines 4 - 28, Extend the brushSelectionStats tests with cases covering tuple and object-valued bar data, null and non-finite values, and duplicate (seriesIndex, dataIndex) selections across overlapping regions. Assert that only valid selectable values contribute and duplicate data points are counted once, while preserving the existing grouped-bar aggregation expectations.
🤖 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.
Nitpick comments:
In `@ui/src/lib/brushSelection.test.ts`:
- Around line 4-28: Extend the brushSelectionStats tests with cases covering
tuple and object-valued bar data, null and non-finite values, and duplicate
(seriesIndex, dataIndex) selections across overlapping regions. Assert that only
valid selectable values contribute and duplicate data points are counted once,
while preserving the existing grouped-bar aggregation expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b7e3b1c-c800-4b1d-9eea-57a2581890b0
📒 Files selected for processing (6)
ui/src/components/ChartBar.vueui/src/components/ChartCard.vueui/src/composables/charts/useBarChartOptions.test.tsui/src/composables/charts/useBarChartOptions.tsui/src/lib/brushSelection.test.tsui/src/lib/brushSelection.ts
|
@hfl0506 lets only fix the series x count problem; don't introduce any brush selection. I was thinking of closing this issue, but you started working on it. my bad |
will do |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/src/lib/builders/index.ts (1)
13-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for all new builder branches.
The added tests cover
seriesandmixedTuples, but notvalueTuples/valuePoints3Dor eitherrender3D.modebranch. Add focused cases so these paths cannot silently fall back tostatType.🤖 Prompt for 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. In `@ui/src/lib/builders/index.ts` around lines 13 - 19, Add focused regression tests for builderForChart covering valueTuples and valuePoints3D selecting value, render3D.mode set to continuous selecting value, and render3D.mode set to mixed selecting mixed. Assert each case returns the expected builder and cannot fall through to statType.
🤖 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.
Nitpick comments:
In `@ui/src/lib/builders/index.ts`:
- Around line 13-19: Add focused regression tests for builderForChart covering
valueTuples and valuePoints3D selecting value, render3D.mode set to continuous
selecting value, and render3D.mode set to mixed selecting mixed. Assert each
case returns the expected builder and cannot fall through to statType.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: adbddd3a-e7c1-459f-b2e7-ff178da249ab
📒 Files selected for processing (2)
ui/src/lib/builders/index.tsui/src/lib/utils.test.ts
Hi fahim, please take a look when you have time. Thanks |
2 participants
Related Issue
Closes #252
Changes
Added rectangular, keep, and clear brush controls to supported 2D bar charts.
Added multi-region selection metrics: Regions, Total, Avg, and Count.
Aggregates directly from rendered series indices, correctly handling grouped multi-
series and tuple-mode bars.
Ignores null/non-finite values and deduplicates overlapping selected points.
Clears brush regions and summary state when chart options change.
Positioned the non-interactive stats overlay below the ECharts toolbox.
Preserved optimized large-bar rendering at 2,000+ points; brushing is hidden where
ECharts large mode cannot select reliably.
Added regression coverage for grouped bars, mixed/value tuples, multi-region
aggregation, and the large-data threshold.
Test Result
# commands task build:ui task build:cli ./bin/vizb bar flights.csv -g year,month -p x,y -o flights.html open flights.htmlSummary by CodeRabbit
Bug Fixes
Tests