feat(components/charts): add pie and donut chart components - #4584
Blackbaud-SteveBrush wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds ChangesPie chart feature
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Consumer
participant SkyChartPie
participant SkyChartPieSlice
participant Chart.js
participant AccessibilityData
Consumer->>SkyChartPie: Provide display mode and projected slices
SkyChartPie->>SkyChartPieSlice: Read label and value inputs
SkyChartPie->>Chart.js: Build pie or donut configuration
Chart.js-->>SkyChartPie: Render chart canvas
SkyChartPie->>AccessibilityData: Publish formatted summary and table data
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 3fce954
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libs/components/charts/src/lib/chart-pie/chart-pie.spec.ts (1)
269-276: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
percentvalue format.
chart-pie.tslines 87-92 document thatpercentexpects fractional values, so0.25displays as25%. The spec coversnumberandcurrencyonly. Add apercentcase so the documented conversion stays verified.💚 Proposed test addition
+ it('should format the tooltip value as a percent', () => { + component.valueFormat = 'percent'; + fixture.detectChanges(); + + const label = getTooltipLabel(requireChart()); + expect(label(tooltipContext('North', 0.25))).toBe('North: 25%'); + });🤖 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 `@libs/components/charts/src/lib/chart-pie/chart-pie.spec.ts` around lines 269 - 276, Add a test case alongside the existing value-format specs in the chart-pie test suite that sets valueFormat to percent, passes the fractional value 0.25 through the tooltip label, and asserts it renders as 25%.
🤖 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 `@libs/components/charts/src/assets/locales/resources_en_US.json`:
- Around line 17-21: Update the pie chart accessibility resources and the
display-mode selection in chart-pie.ts: add a donut-specific accessible summary
key whose message announces “Donut chart” while preserving the slice count and
data-table information, then make displayMode select that key for donut charts
and retain the existing pie key for pie charts.
In `@libs/components/charts/src/lib/chart-pie/chart-pie.spec.ts`:
- Around line 316-329: Update both a11y tests in the “a11y” describe block to
call fixture.detectChanges() before fixture.whenStable(), ensuring the pie and
donut configurations are rendered before accessibility assertions.
---
Nitpick comments:
In `@libs/components/charts/src/lib/chart-pie/chart-pie.spec.ts`:
- Around line 269-276: Add a test case alongside the existing value-format specs
in the chart-pie test suite that sets valueFormat to percent, passes the
fractional value 0.25 through the tooltip label, and asserts it renders as 25%.
🪄 Autofix
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: Enterprise
Run ID: 16c622ff-6cae-49fd-a2b7-6c1478d6dd2f
📒 Files selected for processing (41)
.github/skills/add-skyux-component/SKILL.mdapps/e2e/charts-storybook/src/app/chart-pie/chart-pie.component.htmlapps/e2e/charts-storybook/src/app/chart-pie/chart-pie.component.scssapps/e2e/charts-storybook/src/app/chart-pie/chart-pie.component.stories.tsapps/e2e/charts-storybook/src/app/chart-pie/chart-pie.component.tsapps/playground/src/app/components/charts/chart-pie/chart-pie-playground.htmlapps/playground/src/app/components/charts/chart-pie/chart-pie-playground.tsapps/playground/src/app/components/charts/chart-pie/variants/chart-pie-async-playground.htmlapps/playground/src/app/components/charts/chart-pie/variants/chart-pie-async-playground.tsapps/playground/src/app/components/charts/chart-pie/variants/chart-pie-basic-playground.htmlapps/playground/src/app/components/charts/chart-pie/variants/chart-pie-basic-playground.tsapps/playground/src/app/components/charts/chart-pie/variants/chart-pie-value-format-playground.htmlapps/playground/src/app/components/charts/chart-pie/variants/chart-pie-value-format-playground.tsapps/playground/src/app/components/charts/charts.routes.tslibs/components/charts/documentation.jsonlibs/components/charts/src/assets/locales/resources_en_US.jsonlibs/components/charts/src/index.tslibs/components/charts/src/lib/chart-js/chart-js.tslibs/components/charts/src/lib/chart-pie/chart-pie-display-mode.tslibs/components/charts/src/lib/chart-pie/chart-pie-slice.tslibs/components/charts/src/lib/chart-pie/chart-pie.htmllibs/components/charts/src/lib/chart-pie/chart-pie.spec.tslibs/components/charts/src/lib/chart-pie/chart-pie.tslibs/components/charts/src/lib/shared/chart-theme-styles.tslibs/components/charts/src/lib/shared/fixtures/theme-styles-fixture.tslibs/components/charts/src/lib/shared/sky-charts-resources.module.tslibs/components/charts/testing/src/modules/chart-pie/chart-pie-harness-filters.tslibs/components/charts/testing/src/modules/chart-pie/chart-pie-harness.spec.tslibs/components/charts/testing/src/modules/chart-pie/chart-pie-harness.tslibs/components/charts/testing/src/public-api.tslibs/components/code-examples/routes/src/index.tslibs/components/code-examples/src/index.tslibs/components/code-examples/src/lib/modules/charts/chart-pie/async/example.htmllibs/components/code-examples/src/lib/modules/charts/chart-pie/async/example.spec.tslibs/components/code-examples/src/lib/modules/charts/chart-pie/async/example.tslibs/components/code-examples/src/lib/modules/charts/chart-pie/basic/example.htmllibs/components/code-examples/src/lib/modules/charts/chart-pie/basic/example.spec.tslibs/components/code-examples/src/lib/modules/charts/chart-pie/basic/example.tslibs/components/code-examples/src/lib/modules/charts/chart-pie/donut/example.htmllibs/components/code-examples/src/lib/modules/charts/chart-pie/donut/example.spec.tslibs/components/code-examples/src/lib/modules/charts/chart-pie/donut/example.ts
|
Component Storybooks: Apps: |
Summary by CodeRabbit
New Features
Testing