Conversation
…ckages - Replace git tag --list strategy with package.json-driven tag validation in all 16 publish workflows; use git rev-parse to verify the exact tag exists rather than guessing the latest repo-wide tag - Update error guidance to reflect feat/** → develop → master flow - Standardize dependabot to npm-only, grouped, monthly cadence across all 16 packages; remove github-actions ecosystem updates - Add missing dependabot.yml to AuthKit-UI, ChartKit-UI, HealthKit, HooksKit, paymentkit, StorageKit
* feat: add typed chart contracts and buildChartConfig utility with bar/line/area support and theme-based color fallback * feat: add BarChart component with typed props, stacking/horizontal support, and internal config builder * feat: add LineChart and AreaChart with smooth option, area fill, stacking, and shared config builder * test: add full test suite with mocked react-chartjs-2, config validation, and 80%+ coverage * chore: add README and changeset * fix: extract shared test fixtures to resolve SonarCloud duplication
|
There was a problem hiding this comment.
Pull request overview
This PR converts the existing React/TS library template into a typed Chart.js-based chart kit, adding reusable Bar/Line/Area chart components, a shared config builder, public type exports, and updated CI/publishing automation.
Changes:
- Added
buildChartConfigutility + chart type contracts (ChartDataset,ChartTheme, etc.). - Introduced
BarChart,LineChart, andAreaChartcomponents with Vitest + RTL coverage and shared chart test helpers. - Updated package metadata/docs and GitHub workflows for validation/release/publish.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/index.ts | Exposes the new chart config builder from the utils barrel. |
| src/utils/buildChartConfig.ts | Adds typed transformation from datasets/theme into Chart.js config. |
| src/utils/buildChartConfig.test.ts | Unit tests for config building across variants and theme behaviors. |
| src/types/index.ts | Adds a public types barrel export. |
| src/types/chart.types.ts | Introduces shared chart data/theme/config TypeScript types. |
| src/index.ts | Exports new public types entrypoint from the package root. |
| src/components/LineChart/LineChart.types.ts | Defines typed props contract for LineChart. |
| src/components/LineChart/LineChart.tsx | Implements LineChart using react-chartjs-2 + buildChartConfig. |
| src/components/LineChart/LineChart.test.tsx | Component tests for LineChart behavior (mocked canvas). |
| src/components/LineChart/index.ts | Barrel export for LineChart and its props type. |
| src/components/index.ts | Exposes new chart components from the components barrel. |
| src/components/BarChart/index.ts | Barrel export for BarChart and its props type. |
| src/components/BarChart/BarChart.types.ts | Defines typed props contract for BarChart. |
| src/components/BarChart/BarChart.tsx | Implements BarChart (stacked/horizontal options). |
| src/components/BarChart/BarChart.test.tsx | Component tests for BarChart behavior (mocked canvas). |
| src/components/AreaChart/index.ts | Barrel export for AreaChart and its props type. |
| src/components/AreaChart/AreaChart.types.ts | Defines typed props contract for AreaChart. |
| src/components/AreaChart/AreaChart.tsx | Implements AreaChart using area variant config + options. |
| src/components/AreaChart/AreaChart.test.tsx | Component tests for AreaChart behavior (mocked canvas). |
| src/tests/chart-test-utils.ts | Shared RTL test helpers and fixtures for chart components. |
| README.md | Replaces template README with package docs, types, and usage examples. |
| package.json | Renames package, adds Chart.js deps, updates metadata and scripts. |
| package-lock.json | Lockfile updates for new dependencies and package metadata. |
| ciscode-ui-chart-kit-0.1.0.tgz | Adds an npm pack artifact to the repo (should be removed). |
| ciscode-reactts-developerkit-1.0.0.tgz | Adds an npm pack artifact to the repo (should be removed). |
| .vscode/mcp.json | Adds VS Code MCP server config for GitHub integration. |
| .github/workflows/release-check.yml | Updates CI checks and SonarCloud scanning on PRs to master. |
| .github/workflows/publish.yml | Updates publish pipeline to validate version/tag and use npm provenance. |
| .github/workflows/pr-validation.yml | Adds PR validation workflow for the develop branch. |
| .github/dependabot.yml | Enables monthly npm dependency updates via Dependabot. |
| .github/CODEOWNERS | Adds code ownership rules for review routing. |
| .changeset/initial-release.md | Adds changeset describing the initial chart kit release. |
| function resolveColor(dataset: ChartDataset, index: number, theme: ChartTheme): string { | ||
| return dataset.color ?? theme.colors[index % theme.colors.length]; | ||
| } |
There was a problem hiding this comment.
theme.colors can legally be an empty array per the ChartTheme type, but resolveColor uses index % theme.colors.length. When colors.length is 0 this produces NaN and returns undefined, which then flows into backgroundColor/borderColor (e.g., 'undefined33' for area) at runtime. Consider enforcing a non-empty palette at the type level (e.g., tuple type) and/or throwing a clear error when theme.colors.length === 0.
| function applyAreaOpacity(hex: string): string { | ||
| return `${hex}33`; | ||
| } |
There was a problem hiding this comment.
applyAreaOpacity blindly appends "33" to whatever string is passed. If callers provide a non-#RRGGBB hex (e.g., #RGB, #RRGGBBAA) or a non-hex CSS color, this will create an invalid color value. Since the API is typed as string, consider validating/normalizing to 6-digit hex before appending alpha, or switching to an rgba(...) conversion approach.
| @@ -0,0 +1,93 @@ | |||
| import { describe, it, expect, vi } from 'vitest'; | |||
| import { render, screen } from '@testing-library/react'; | |||
There was a problem hiding this comment.
screen is imported but never used in this test file. With @typescript-eslint/no-unused-vars: 'warn' and --max-warnings=0, this will fail CI lint. Remove the unused screen import.
| import { render, screen } from '@testing-library/react'; | |
| import { render } from '@testing-library/react'; |
| @@ -0,0 +1,59 @@ | |||
| import { describe, it, expect, vi } from 'vitest'; | |||
| import { render, screen } from '@testing-library/react'; | |||
There was a problem hiding this comment.
screen is imported but never used in this test file. With @typescript-eslint/no-unused-vars: 'warn' and --max-warnings=0, this will fail CI lint. Remove the unused screen import.
| import { render, screen } from '@testing-library/react'; | |
| import { render } from '@testing-library/react'; |
| @@ -0,0 +1,95 @@ | |||
| import { describe, it, expect, vi } from 'vitest'; | |||
| import { render, screen } from '@testing-library/react'; | |||
There was a problem hiding this comment.
screen is imported but never used in this test file. With @typescript-eslint/no-unused-vars: 'warn' and --max-warnings=0, this will fail CI lint. Remove the unused screen import.
| import { render, screen } from '@testing-library/react'; | |
| import { render } from '@testing-library/react'; |
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes