Refactor by Omaima33 · Pull Request #2 · CISCODE-MA/ChartKit-UI

Omaima33 · 2026-04-06T15:09:22Z

Summary

What does this PR change?

Why

Why is this change needed?

Checklist

Added/updated tests (if behavior changed)
npm run lint passes
npm run typecheck passes
npm test passes
npm run build passes
Added a changeset (npx changeset) if this affects consumers

Notes

Anything reviewers should pay attention to?

…/line/area support and theme-based color fallback

…pport, and internal config builder

…king, and shared config builder

…ion, and 80%+ coverage

sonarqubecloud · 2026-04-06T15:10:34Z

Quality Gate failed

Failed conditions
4.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Copilot

Pull request overview

This PR repurposes the template package into @ciscode/ui-chart-kit, introducing typed Chart.js-based React chart components plus shared chart types and a config builder utility.

Changes:

Added BarChart, LineChart, and AreaChart components backed by a shared buildChartConfig utility.
Introduced public chart type contracts (ChartDataPoint, ChartDataset, ChartTheme, etc.) and exported them from the package entrypoint.
Updated package identity/docs and added release metadata (changeset).

Reviewed changes

Copilot reviewed 24 out of 27 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/utils/index.ts	Re-exports the new `buildChartConfig` utility.
src/utils/buildChartConfig.ts	Implements typed Chart.js config/dataset construction.
src/utils/buildChartConfig.test.ts	Unit tests for config building across variants and theme options.
src/types/index.ts	Public re-export barrel for chart types.
src/types/chart.types.ts	Defines public chart data/theme/config types.
src/index.ts	Exposes `types` from the package root.
src/components/index.ts	Exports the new chart components from the components barrel.
src/components/BarChart/index.ts	Component barrel export for `BarChart`.
src/components/BarChart/BarChart.types.ts	Typed props for `BarChart`.
src/components/BarChart/BarChart.tsx	Bar chart component implementation.
src/components/BarChart/BarChart.test.tsx	Component tests for bar chart behavior (stacked/horizontal/etc.).
src/components/LineChart/index.ts	Component barrel export for `LineChart`.
src/components/LineChart/LineChart.types.ts	Typed props for `LineChart`.
src/components/LineChart/LineChart.tsx	Line chart component implementation.
src/components/LineChart/LineChart.test.tsx	Component tests for line chart behavior (smooth/tension).
src/components/AreaChart/index.ts	Component barrel export for `AreaChart`.
src/components/AreaChart/AreaChart.types.ts	Typed props for `AreaChart`.
src/components/AreaChart/AreaChart.tsx	Area chart component implementation (fill/stacking/smooth).
src/components/AreaChart/AreaChart.test.tsx	Component tests for area chart behavior (fill/stacked/smooth).
src/tests/chart-test-utils.ts	Shared test helpers and fixtures for chart components.
README.md	Rewritten documentation for the chart kit API and usage examples.
package.json	Renames/versions the package and adds Chart.js dependencies.
package-lock.json	Lockfile updated for new dependencies (but root metadata still references old name/version).
.changeset/initial-release.md	Adds changeset for the initial release.
.vscode/mcp.json	Adds workspace MCP server configuration.
ciscode-ui-chart-kit-0.1.0.tgz	Adds a packed tarball artifact to the repo.
ciscode-reactts-developerkit-1.0.0.tgz	Adds a packed tarball artifact to the repo.

Copilot · 2026-04-06T15:14:27Z

src/utils/buildChartConfig.ts

+} from '../types/chart.types';
+
+function resolveColor(dataset: ChartDataset, index: number, theme: ChartTheme): string {
+  return dataset.color ?? theme.colors[index % theme.colors.length];


resolveColor can return undefined when theme.colors is an empty array (because index % theme.colors.length becomes NaN), which then propagates into backgroundColor/borderColor and can produce invalid Chart.js config (e.g., 'undefined33' for area). Consider enforcing theme.colors to be non-empty (type-level and/or runtime check) and throwing a clear error or providing a safe fallback when it is empty.

Suggested change

return dataset.color ?? theme.colors[index % theme.colors.length];

if (dataset.color) {

return dataset.color;

}

if (theme.colors.length === 0) {

throw new Error('Chart theme.colors must contain at least one color when dataset.color is not provided.');

}

return theme.colors[index % theme.colors.length];

Copilot · 2026-04-06T15:14:28Z

src/utils/buildChartConfig.ts

+function applyAreaOpacity(hex: string): string {
+  return `${hex}33`;
+}
+


applyAreaOpacity blindly appends 33 to the input string. If a consumer passes a non-6-digit hex (e.g., #abc), an already-alpha hex (#RRGGBBAA), or an rgb(...)/CSS var, the resulting color becomes invalid. Either validate/normalize to #RRGGBB before appending, or switch to generating rgba(...) / using a color utility that can handle different formats.

Suggested change

function applyAreaOpacity(hex: string): string {

return `${hex}33`;

}

function normalizeHexColor(color: string): string | null {

const shortHexMatch = /^#([\da-fA-F]{3})$/.exec(color);

if (shortHexMatch) {

const [r, g, b] = shortHexMatch[1].split('');

return `#${r}${r}${g}${g}${b}${b}`;

}

if (/^#([\da-fA-F]{6})$/.test(color)) {

return color;

}

return null;

}

function applyAreaOpacity(color: string): string {

const normalizedHex = normalizeHexColor(color);

return normalizedHex ? `${normalizedHex}33` : color;

}

Copilot · 2026-04-06T15:14:28Z

src/components/LineChart/LineChart.test.tsx

@@ -0,0 +1,59 @@
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen } from '@testing-library/react';


screen is imported but never used in this test file. With the current ESLint config (@typescript-eslint/no-unused-vars: 'warn' and --max-warnings=0), this will fail npm run lint. Remove the unused import or use it in an assertion.

Suggested change

import { render, screen } from '@testing-library/react';

import { render } from '@testing-library/react';

Copilot · 2026-04-06T15:14:28Z

src/components/BarChart/BarChart.test.tsx

@@ -0,0 +1,93 @@
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen } from '@testing-library/react';


screen is imported but never used in this test file. With the current ESLint config (@typescript-eslint/no-unused-vars: 'warn' and --max-warnings=0), this will fail npm run lint. Remove the unused import or use it in an assertion.

Suggested change

import { render, screen } from '@testing-library/react';

import { render } from '@testing-library/react';

Copilot · 2026-04-06T15:14:29Z

src/components/AreaChart/AreaChart.test.tsx

@@ -0,0 +1,95 @@
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen } from '@testing-library/react';


screen is imported but never used in this test file. With the current ESLint config (@typescript-eslint/no-unused-vars: 'warn' and --max-warnings=0), this will fail npm run lint. Remove the unused import or use it in an assertion.

Suggested change

import { render, screen } from '@testing-library/react';

import { render } from '@testing-library/react';

Copilot · 2026-04-06T15:14:29Z

.vscode/mcp.json

+  "servers": {
+    "github": {
+      "command": "npx",
+      "args": ["-y", "@modelcontextprotocol/server-github"]
+    }
+  }


Adding .vscode/mcp.json hardcodes a local editor/MCP setup that runs npx -y @modelcontextprotocol/server-github. Committing this can create an unexpected supply-chain execution path for anyone opening the workspace. Prefer documenting optional MCP setup in CONTRIBUTING/README and keeping editor-specific config out of the repo (or at least require explicit opt-in).

Suggested change

"servers": {

"github": {

"command": "npx",

"args": ["-y", "@modelcontextprotocol/server-github"]

}

}

"servers": {}

Copilot · 2026-04-06T15:14:29Z

package.json

  "types": "./dist/index.d.ts",
  "publishConfig": {
    "access": "public"
  },
  "peerDependencies": {
    "react": ">=18",
    "react-dom": ">=18"
  },
  "devDependencies": {
    "@changesets/cli": "^2.27.8",
    "@eslint/js": "^9.39.2",
    "@testing-library/jest-dom": "^6.9.1",
    "@testing-library/react": "^16.3.2",
    "@testing-library/user-event": "^14.6.1",
    "@types/react": "^18.3.12",
    "@types/react-dom": "^18.3.1",
    "@vitest/coverage-v8": "^2.1.8",
    "@vitest/ui": "^2.1.8",
    "eslint": "^9.39.2",
    "eslint-config-prettier": "^10.1.8",
    "eslint-plugin-react": "^7.37.5",
    "eslint-plugin-react-hooks": "^7.0.1",
    "globals": "^17.4.0",
    "husky": "^9.1.7",
    "jsdom": "^28.0.0",
    "lint-staged": "^15.2.10",
    "prettier": "^3.4.2",
    "rimraf": "^6.0.1",
    "tsup": "^8.3.5",
    "typescript": "^5.7.2",
    "typescript-eslint": "^8.55.0",
    "vitest": "^2.1.8"
  },
  "scripts": {
    "clean": "rimraf dist *.tsbuildinfo && rm -rf coverage",
    "build": "tsup",
    "dev": "tsup --watch",
    "lint": "eslint . --max-warnings=0",
    "lint:fix": "eslint . --fix",
    "typecheck": "tsc -p tsconfig.json --noEmit",
    "test": "vitest run",
    "test:watch": "vitest",
    "test:cov": "vitest run --coverage",
    "format": "prettier . --check",
    "format:write": "prettier . --write",
    "verify": "npm run lint && npm run typecheck && npm run test:cov",
    "changeset": "changeset",
    "version-packages": "changeset version",
    "release": "changeset publish",
    "prepublishOnly": "npm run verify && npm run build",
    "prepare": "husky"
  },
  "lint-staged": {
    "*.{ts,tsx,js,jsx,json,md,yml,yaml}": [
      "prettier --write"
    ],
    "*.{ts,tsx,js,jsx}": [
      "eslint . --fix"
    ]
  },
  "engines": {
    "node": ">=20"
+  },


chart.js and react-chartjs-2 are added as runtime dependencies. Given tsup.config.ts only externalizes react/react-dom, these will likely be bundled into your distributed build, which can significantly increase bundle size and can lead to duplicate Chart.js copies if consumers also depend on it. Consider moving them to peerDependencies (and adding them to external) if you want consumers to share a single instance, or explicitly documenting that they are intentionally bundled.

Copilot · 2026-04-06T15:14:30Z

package.json

  "name": "@ciscode/ui-chart-kit",
-  "version": "0.0.0",
-  "description": "React 19 chart library on Chart.js.",
+  "version": "0.1.0",
+  "description": "Typed React chart components (Bar, Line, Area) built on Chart.js — no raw Chart.js config required.",
  "license": "MIT",
  "private": false,


The PR description is still the default template and doesn’t describe the substantial changes here (new chart components, new public types, package rename). Please update the PR description (Summary/Why/Notes) so reviewers and release tooling have the correct context.

Copilot · 2026-04-06T15:14:30Z

README.md

+| `react-dom` | ≥ 18    |

-Anything not exported from `src/index.ts` is considered private.
+`chart.js` and `react-chartjs-2` are bundled — you do **not** need to install them separately.


README says "chart.js and react-chartjs-2 are bundled". In package.json they are declared as dependencies, which means they are installed automatically but not necessarily bundled into the output (that depends on the build config). Consider rephrasing to avoid ambiguity (e.g., "installed automatically as dependencies" or explicitly state whether they are bundled into dist).

Suggested change

`chart.js` and `react-chartjs-2` are bundled — you do **not** need to install them separately.

`chart.js` and `react-chartjs-2` are installed automatically as dependencies — you do **not** need to install them separately.

Copilot · 2026-04-06T15:14:30Z

src/types/chart.types.ts

+}
+
+export interface ChartTheme {
+  colors: string[];


ChartTheme.colors is typed as string[], which allows an empty array, but buildChartConfig assumes at least one color (it indexes into theme.colors). Consider tightening this type to a non-empty tuple (e.g. [string, ...string[]]) and/or documenting/enforcing the non-empty requirement to prevent runtime invalid colors/config.

Suggested change

colors: string[];

colors: [string, ...string[]];

Omaima33 added 6 commits March 31, 2026 16:22

feat: add typed chart contracts and buildChartConfig utility with bar…

65a0013

…/line/area support and theme-based color fallback

feat: add BarChart component with typed props, stacking/horizontal su…

d56e280

…pport, and internal config builder

feat: add LineChart and AreaChart with smooth option, area fill, stac…

6fc8277

…king, and shared config builder

test: add full test suite with mocked react-chartjs-2, config validat…

17d076a

…ion, and 80%+ coverage

chore: add README and changeset

e3111b0

fix: extract shared test fixtures to resolve SonarCloud duplication

cc7340d

Omaima33 requested a review from a team as a code owner April 6, 2026 15:09

Copilot AI review requested due to automatic review settings April 6, 2026 15:09

Merge branch 'develop' into refactor

63f8859

Copilot started reviewing on behalf of Omaima33 April 6, 2026 15:09 View session

Omaima33 merged commit 92f099e into develop Apr 6, 2026
1 check passed

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor#2

Refactor#2
Omaima33 merged 7 commits intodevelopfrom
refactor

Omaima33 commented Apr 6, 2026

Uh oh!

Uh oh!

sonarqubecloud bot commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-  return dataset.color ?? theme.colors[index % theme.colors.length];
+  if (dataset.color) {
+    return dataset.color;
+  }
+  if (theme.colors.length === 0) {
+    throw new Error('Chart theme.colors must contain at least one color when dataset.color is not provided.');
+  }
+  return theme.colors[index % theme.colors.length];

-function applyAreaOpacity(hex: string): string {
-  return `${hex}33`;
-}
+function normalizeHexColor(color: string): string | null {
+  const shortHexMatch = /^#([\da-fA-F]{3})$/.exec(color);
+  if (shortHexMatch) {
+    const [r, g, b] = shortHexMatch[1].split('');
+    return `#${r}${r}${g}${g}${b}${b}`;
+  }
+  if (/^#([\da-fA-F]{6})$/.test(color)) {
+    return color;
+  }
+  return null;
+}
+function applyAreaOpacity(color: string): string {
+  const normalizedHex = normalizeHexColor(color);
+  return normalizedHex ? `${normalizedHex}33` : color;
+}

		@@ -0,0 +1,59 @@
		import { describe, it, expect, vi } from 'vitest';
		import { render, screen } from '@testing-library/react';

	import { render, screen } from '@testing-library/react';
	import { render } from '@testing-library/react';

		@@ -0,0 +1,93 @@
		import { describe, it, expect, vi } from 'vitest';
		import { render, screen } from '@testing-library/react';

		@@ -0,0 +1,95 @@
		import { describe, it, expect, vi } from 'vitest';
		import { render, screen } from '@testing-library/react';

	`chart.js` and `react-chartjs-2` are bundled — you do not need to install them separately.
	`chart.js` and `react-chartjs-2` are installed automatically as dependencies — you do not need to install them separately.

Conversation

Omaima33 commented Apr 6, 2026

Summary

Why

Checklist

Notes

Uh oh!

Uh oh!

sonarqubecloud bot commented Apr 6, 2026

Quality Gate failed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants