feat(components/charts): add @skyux/charts library -- DRAFT#4349
Draft
ThomasOrtiz wants to merge 34 commits into13.x.xfrom
Draft
feat(components/charts): add @skyux/charts library -- DRAFT#4349ThomasOrtiz wants to merge 34 commits into13.x.xfrom
ThomasOrtiz wants to merge 34 commits into13.x.xfrom
Conversation
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
View your CI Pipeline Execution ↗ for commit 3e4850f
☁️ Nx Cloud last updated this comment at |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new @skyux/charts package (built on Chart.js) with core chart wrapper UI, chart-type components, supporting plugins/services, and a Playground area for early demos/feedback.
Changes:
- Introduces
@skyux/chartslibrary scaffolding, exports, and build/test configuration (Nx + ng-packagr). - Implements chart wrapper UI (heading/subtitle/actions/legend/data-table modal) plus Bar/Line/Donut chart components and Chart.js integration.
- Adds Chart.js dependency, theme token bump, and Playground routes/demos for the new charts package.
Reviewed changes
Copilot reviewed 106 out of 108 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.base.json | Adds TS path aliases for @skyux/charts and @skyux/charts/testing. |
| package.json | Bumps design tokens and adds chart.js dependency. |
| package-lock.json | Locks updated design tokens and new Chart.js transitive deps. |
| libs/components/theme/package.json | Aligns theme package design token dependency version. |
| libs/components/charts/tsconfig.spec.json | Charts library unit test TS config. |
| libs/components/charts/tsconfig.lib.prod.json | Charts production TS config (partial compilation). |
| libs/components/charts/tsconfig.lib.json | Charts library TS build config. |
| libs/components/charts/tsconfig.json | TS project references for charts library. |
| libs/components/charts/testing/tsconfig.spec.json | Charts testing library unit test TS config. |
| libs/components/charts/testing/src/public-api.ts | Public exports for charts test harnesses. |
| libs/components/charts/testing/src/modules/bar-chart/fixtures/bar-chart-harness-test.component.ts | Harness fixture component shell for bar chart harness tests. |
| libs/components/charts/testing/src/modules/bar-chart/fixtures/bar-chart-harness-test.component.html | Harness fixture template for bar chart harness tests. |
| libs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.ts | Adds SkyBarChartHarness. |
| libs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.spec.ts | Adds initial harness spec coverage for bar chart harness. |
| libs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.filters.ts | Adds harness filters interface for bar chart harness. |
| libs/components/charts/testing/project.json | Nx project config for charts testing library (karma/lint). |
| libs/components/charts/testing/ng-package.json | ng-packagr config for charts testing package. |
| libs/components/charts/testing/karma.conf.js | Karma config for charts testing library. |
| libs/components/charts/testing/eslint.config.js | ESLint config for charts testing library. |
| libs/components/charts/src/lib/modules/shared/types/deep-partial-type.ts | Adds DeepPartial type utility for Chart.js option typing. |
| libs/components/charts/src/lib/modules/shared/types/chart-series.ts | Defines series data model type used across charts. |
| libs/components/charts/src/lib/modules/shared/types/chart-heading-style.ts | Adds heading style type + input transformer. |
| libs/components/charts/src/lib/modules/shared/types/chart-heading-level.ts | Adds heading level type + input transformer. |
| libs/components/charts/src/lib/modules/shared/types/chart-data-point.ts | Base interface for chart datapoints. |
| libs/components/charts/src/lib/modules/shared/types/chart-clicked-data-point.ts | Defines click event payload shape for charts. |
| libs/components/charts/src/lib/modules/shared/types/category.ts | Defines category type (string/number). |
| libs/components/charts/src/lib/modules/shared/types/axis-types.ts | Defines axis config types (category/measure). |
| libs/components/charts/src/lib/modules/shared/sky-charts-resources.module.ts | Adds i18n resources module for charts strings. |
| libs/components/charts/src/lib/modules/shared/services/global-chart-config.service.ts | Centralizes global Chart.js options/plugins merging. |
| libs/components/charts/src/lib/modules/shared/services/chart-style.service.ts | Computes chart styling from theme CSS variables. |
| libs/components/charts/src/lib/modules/shared/services/chart-registry.service.ts | Defines registry interface for series/point coordination. |
| libs/components/charts/src/lib/modules/shared/plugins/tooltip/tooltip-shadow-plugin.ts | Custom tooltip shadow rendering plugin. |
| libs/components/charts/src/lib/modules/shared/plugins/tooltip/tooltip-options.ts | Default tooltip option builder based on theme styles. |
| libs/components/charts/src/lib/modules/shared/plugins/plugin-state/focused-elements-state.ts | Shared WeakMap state for focused elements across plugins. |
| libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/radial-navigation-strategy.ts | Keyboard navigation strategy for radial charts. |
| libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/navigation-strategy.ts | Interfaces for keyboard navigation strategies. |
| libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keys.ts | Key constants + type guards for keyboard navigation. |
| libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keyboard-nav-plugin.ts | Chart.js plugin enabling keyboard navigation and announcements. |
| libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keyboard-nav-plugin-options.ts | Plugin options typing + Chart.js module augmentation. |
| libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/create-navigation-strategy.ts | Strategy factory (cartesian vs radial). |
| libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/cartesian-navigation-strategy.ts | Keyboard navigation strategy for cartesian charts. |
| libs/components/charts/src/lib/modules/shared/plugins/indicator/line-indicator-bounds.ts | Computes indicator bounds for line points. |
| libs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-types.ts | Types for indicator styles and bounds. |
| libs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-plugin.ts | Plugin to draw hover/active/focus indicators. |
| libs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-plugin-options.ts | Indicator plugin options typing + Chart.js augmentation. |
| libs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-draw.ts | Canvas drawing helpers for indicator rendering. |
| libs/components/charts/src/lib/modules/shared/plugins/indicator/donut-indicator-helpers.ts | Donut slice geometry helpers for indicator drawing. |
| libs/components/charts/src/lib/modules/shared/plugins/indicator/bar-indicator-bounds.ts | Computes indicator bounds for bar elements. |
| libs/components/charts/src/lib/modules/shared/plugins/auto-color/auto-color-plugin.ts | Plugin that assigns theme palette colors to datasets/data. |
| libs/components/charts/src/lib/modules/shared/chart-helpers.ts | Shared helpers for chart type detection, categories, legend items, ticks. |
| libs/components/charts/src/lib/modules/line-chart/line-chart.component.ts | Adds sky-line-chart component. |
| libs/components/charts/src/lib/modules/line-chart/line-chart-types.ts | Line chart datum/point types. |
| libs/components/charts/src/lib/modules/line-chart/line-chart-series.component.ts | Line chart series registration component. |
| libs/components/charts/src/lib/modules/line-chart/line-chart-series-datapoint.component.ts | Line chart datapoint registration component. |
| libs/components/charts/src/lib/modules/line-chart/line-chart-registry.service.ts | Registry service for line chart series/axes/points. |
| libs/components/charts/src/lib/modules/line-chart/line-chart-config.service.ts | Builds Chart.js config for line charts. |
| libs/components/charts/src/lib/modules/donut-chart/donut-chart.component.ts | Adds sky-donut-chart component. |
| libs/components/charts/src/lib/modules/donut-chart/donut-chart-types.ts | Donut chart datum/slice types. |
| libs/components/charts/src/lib/modules/donut-chart/donut-chart-series.component.ts | Donut chart series registration component. |
| libs/components/charts/src/lib/modules/donut-chart/donut-chart-series-datapoint.component.ts | Donut chart datapoint registration component. |
| libs/components/charts/src/lib/modules/donut-chart/donut-chart-registry.service.ts | Registry service for donut chart series/points. |
| libs/components/charts/src/lib/modules/donut-chart/donut-chart-config.service.ts | Builds Chart.js config for donut charts. |
| libs/components/charts/src/lib/modules/chartjs.directive.ts | Directive to instantiate/update/destroy Chart.js charts on canvas. |
| libs/components/charts/src/lib/modules/chart/chart.service.ts | Shared chart wrapper service (heading/series/legend state). |
| libs/components/charts/src/lib/modules/chart/chart.component.ts | sky-chart wrapper component (header/legend/actions/data table modal). |
| libs/components/charts/src/lib/modules/chart/chart.component.scss | Layout/styling for chart wrapper. |
| libs/components/charts/src/lib/modules/chart/chart.component.html | Template for chart wrapper, legend, and actions menu. |
| libs/components/charts/src/lib/modules/chart/chart-header-id-token.ts | Injection token/provider for header id wiring. |
| libs/components/charts/src/lib/modules/chart-legend/chart-legend.component.ts | Legend component with roving tabindex + keyboard support. |
| libs/components/charts/src/lib/modules/chart-legend/chart-legend.component.scss | Legend styling (buttons/marker/typography). |
| libs/components/charts/src/lib/modules/chart-legend/chart-legend.component.html | Legend template rendering toggle buttons. |
| libs/components/charts/src/lib/modules/chart-legend/chart-legend-item.ts | Legend item interface. |
| libs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.ts | Modal that renders chart data in ag-grid. |
| libs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.scss | Styling for the chart data grid modal. |
| libs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.html | Modal template with ag-grid wrapper and footer button. |
| libs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal-context.ts | Modal context for passing title/series/categories. |
| libs/components/charts/src/lib/modules/bar-chart/bar-chart.component.ts | Adds sky-bar-chart component. |
| libs/components/charts/src/lib/modules/bar-chart/bar-chart-types.ts | Bar chart datum/point types + orientation type. |
| libs/components/charts/src/lib/modules/bar-chart/bar-chart-series.component.ts | Bar chart series registration component. |
| libs/components/charts/src/lib/modules/bar-chart/bar-chart-series-datapoint.component.ts | Bar chart datapoint registration component. |
| libs/components/charts/src/lib/modules/bar-chart/bar-chart-registry.service.ts | Registry service for bar chart series/axes/points. |
| libs/components/charts/src/lib/modules/bar-chart/bar-chart-config.service.ts | Builds Chart.js config for bar charts. |
| libs/components/charts/src/lib/modules/axis/sky-chart-axis-registry.service.ts | Axis registry token + interface for chart types. |
| libs/components/charts/src/lib/modules/axis/chart-measure-axis.component.ts | Measure-axis config component (log/linear + suggested bounds). |
| libs/components/charts/src/lib/modules/axis/chart-category-axis.component.ts | Category-axis config component (label). |
| libs/components/charts/src/index.ts | Public barrel exports for @skyux/charts. |
| libs/components/charts/src/assets/locales/resources_en_US.json | Adds i18n resource strings for charts. |
| libs/components/charts/README.md | Initial library README scaffold. |
| libs/components/charts/project.json | Nx project config for charts library (build/test/lint). |
| libs/components/charts/package.json | Package manifest for publishing @skyux/charts. |
| libs/components/charts/ng-package.json | ng-packagr configuration for charts build output. |
| libs/components/charts/karma.conf.js | Karma config for charts library unit tests. |
| libs/components/charts/eslint.config.js | ESLint config for charts library. |
| libs/components/charts/documentation.json | Docs manifest entry scaffold for charts group. |
| libs/components/charts/CHANGELOG.md | Changelog scaffold for the new package. |
| apps/playground/src/app/components/components.module.ts | Adds lazy route entry for Playground charts section. |
| apps/playground/src/app/components/charts/shared/chart-demo-utils.ts | Demo data utilities (forked from Chart.js samples). |
| apps/playground/src/app/components/charts/line-chart-demo/line-chart-demo.component.ts | Playground demo component for line charts. |
| apps/playground/src/app/components/charts/line-chart-demo/line-chart-demo.component.html | Playground demo UI for line charts. |
| apps/playground/src/app/components/charts/line-chart-demo/line-chart-demo-routes.ts | Playground routes for line chart demo. |
| apps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.ts | Playground demo component for donut charts. |
| apps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.html | Playground demo UI for donut charts. |
| apps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo-routes.ts | Playground routes for donut chart demo. |
| apps/playground/src/app/components/charts/charts-routes.ts | Top-level Playground charts routes (bar/line/donut). |
| apps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo.component.ts | Playground demo component for bar charts. |
| apps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo-routes.ts | Playground routes for bar chart demo. |
| apps/playground/src/app/app.routes.ts | Adds title to the home route. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (13)
apps/playground/src/app/components/charts/shared/chart-demo-utils.ts-63-74 (1)
63-74:⚠️ Potential issue | 🟡 Minor
points()return type doesn't match actual return values.The method returns
(number | null)[]values (bothxandycome fromthis.numbers()at lines 71–72), but the signature declares{ x: number; y: number }[]. While strict null checks are currently disabled in this project's TypeScript configuration, the type annotation is still technically incorrect.Suggested fix
- }): { x: number; y: number }[] { + }): { x: number | null; y: number | null }[] { const xs = this.numbers(config); const ys = this.numbers(config); return xs.map((x, i) => ({ x, y: ys[i] })); }Aligns with the coding guideline to use strict type checking in TypeScript.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/charts/shared/chart-demo-utils.ts` around lines 63 - 74, The points() signature incorrectly promises {x:number;y:number}[] while it uses this.numbers() which yields (number|null)[]; update the points() return type to { x: number | null; y: number | null }[] to match numbers(), or alternatively change points() to filter or coerce nulls (e.g., filter out indices where xs[i] or ys[i] is null or provide a default) so runtime values match the declared type; reference the points() method and this.numbers() when making the change.libs/components/charts/README.md-7-7 (1)
7-7:⚠️ Potential issue | 🟡 MinorFix incorrect unit-test command in README.
nx test datetimepoints to a different library and will mislead contributors for this package. Update it to the charts target.Suggested patch
-Run `nx test datetime` to execute the unit tests. +Run `nx test charts` to execute the unit tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/README.md` at line 7, The README currently instructs contributors to run the wrong test target ("nx test datetime"); update the command to the correct library target for this package by replacing "nx test datetime" with "nx test charts" in the README (update the line shown in libs/components/charts/README.md), ensuring any surrounding docs or examples that reference the datetime target are updated to use the charts test target as well.libs/components/charts/src/assets/locales/resources_en_US.json-13-20 (1)
13-20:⚠️ Potential issue | 🟡 MinorParameter description keys are misaligned with placeholder indices.
The message uses placeholders
{0}through{3}, but the parameter descriptions use keys_1through_4, skipping_0. This inconsistency could confuse translators or lead to documentation errors.🔧 Proposed fix to align parameter keys
"chart.focus_element.single_series.description": { "_description": "The template for the screen-reader description of the currently focused chart element.", "message": "{0}: {1}. Point {2} of {3}.", - "_1": "Category label", - "_2": "Data point value", - "_3": "Data point index (1-based)", - "_4": "Total data points in the series" + "_0": "Category label", + "_1": "Data point value", + "_2": "Data point index (1-based)", + "_3": "Total data points in the series" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/assets/locales/resources_en_US.json` around lines 13 - 20, The parameter description keys for "chart.focus_element.single_series.description" are off-by-one: the message uses placeholders {0}..{3} but the descriptions are labeled "_1".. "_4"; update the keys to start at "_0" through "_3" so they align with the placeholders (i.e., rename "_1"->"_0", "_2"->"_1", "_3"->"_2", "_4"->"_3") to ensure translators and docs match the message placeholders.libs/components/charts/src/lib/modules/chart-legend/chart-legend.component.scss-74-75 (1)
74-75:⚠️ Potential issue | 🟡 MinorRemove inline SCSS comments.
The comment above
border-radiusviolates the SCSS rule set for this repo.🧹 Suggested cleanup
- // Makes the legend marker circular border-radius: 50%;As per coding guidelines: "
**/*.scss: Do not add any comments in the SCSS code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/chart-legend/chart-legend.component.scss` around lines 74 - 75, Remove the inline SCSS comment above the border-radius rule in chart-legend.component.scss (delete the line "// Makes the legend marker circular") to comply with the repo's SCSS guideline; ensure no other "//" or "/* */" comments remain in the same stylesheet (or move explanatory notes to external docs or the component's TypeScript file) and keep the existing rule (border-radius: 50%) intact in the selector that styles the legend marker.apps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.ts-18-27 (1)
18-27:⚠️ Potential issue | 🟡 MinorRemove duplicate
SkyDonutChartComponentimport.
SkyDonutChartComponentis imported twice in the imports array (lines 20 and 24).🔧 Proposed fix
imports: [ SkyPageModule, - SkyDonutChartComponent, SkyBoxModule, SkyFluidGridModule, SkyChartComponent, SkyDonutChartComponent, SkyDonutChartSeriesComponent, SkyDonutChartSeriesDatapointComponent, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.ts` around lines 18 - 27, Remove the duplicate import entry for SkyDonutChartComponent in the imports array of the DonutChartDemo component; locate the imports array in donut-chart-demo.component.ts and delete the redundant SkyDonutChartComponent entry so it only appears once alongside SkyDonutChartSeriesComponent and SkyDonutChartSeriesDatapointComponent.libs/components/charts/src/lib/modules/chart/chart.component.html-1-5 (1)
1-5:⚠️ Potential issue | 🟡 MinorRedundant ARIA attributes: remove
aria-labelwhenaria-labelledbyis used.The
<section>element has botharia-labelandaria-labelledbyset. Per ARIA specification,aria-labelledbytakes precedence when both are present, makingaria-labelredundant. This can cause confusion for accessibility audits and screen reader behavior.♿ Proposed fix
<section class="sky-chart" - [attr.aria-label]="headingText()" [attr.aria-labelledby]="headerId" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/chart/chart.component.html` around lines 1 - 5, The section has both aria-label and aria-labelledby which is redundant; update the template in chart.component.html (the <section> that binds headingText() and headerId) to remove the static aria-label binding when aria-labelledby is present — either drop [attr.aria-label]="headingText()" entirely or render aria-label only when headerId is falsy (i.e., conditionally include aria-label based on headerId) so that aria-labelledby (headerId) takes precedence and no duplicate ARIA label is emitted.libs/components/charts/src/lib/modules/shared/types/axis-types.ts-39-43 (1)
39-43:⚠️ Potential issue | 🟡 MinorFix documentation typo: "minimum" should be "maximum".
The JSDoc for
suggestedMaxincorrectly states "the chart will automatically determine the minimum" when it should say "maximum".Proposed fix
/** * The suggested maximum value for the axis. - * If not specified, the chart will automatically determine the minimum based on the data. + * If not specified, the chart will automatically determine the maximum based on the data. */ suggestedMax?: number;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/shared/types/axis-types.ts` around lines 39 - 43, Update the JSDoc for the suggestedMax property to correct the typo: change the sentence that currently says "the chart will automatically determine the minimum based on the data" to "the chart will automatically determine the maximum based on the data" so the documentation accurately describes the suggestedMax behavior on the suggestedMax property in axis-types.ts.libs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.spec.ts-14-16 (1)
14-16:⚠️ Potential issue | 🟡 MinorFix naming inconsistency:
boxHarnessshould bebarChartHarness.The return type property
boxHarnessappears to be a copy-paste artifact. It should be renamed tobarChartHarnessfor consistency.Proposed fix
}> { - boxHarness: SkyBarChartHarness; + barChartHarness: SkyBarChartHarness; fixture: ComponentFixture<BarChartHarnessTestComponent>;- return { boxHarness: barChartHarness, fixture, loader }; + return { barChartHarness, fixture, loader };Also applies to: 32-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.spec.ts` around lines 14 - 16, Rename the mistaken return property `boxHarness` to `barChartHarness` in the Promise return type in the bar-chart harness test; update every occurrence (including the other instance around line 32) so the returned object shape reads `{ barChartHarness: SkyBarChartHarness; fixture: ComponentFixture<BarChartHarnessTestComponent>; }` and adjust any test variable references that expect `boxHarness` to use `barChartHarness` instead (look for usages of SkyBarChartHarness and BarChartHarnessTestComponent in this spec).apps/playground/src/app/components/charts/line-chart-demo/line-chart-demo.component.ts-19-21 (1)
19-21:⚠️ Potential issue | 🟡 MinorFix the selector copy/paste.
Line 20 still registers
app-bar-chart-demo. That is misleading in the line-chart demo and can collide with the real bar-chart demo if both are imported into the same template.🐛 Proposed fix
- selector: 'app-bar-chart-demo', + selector: 'app-line-chart-demo',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/charts/line-chart-demo/line-chart-demo.component.ts` around lines 19 - 21, The component decorator in line-chart-demo.component.ts has a wrong selector value; update the `@Component` selector from 'app-bar-chart-demo' to 'app-line-chart-demo' (or another unique line-chart selector) to match the file/class and avoid collisions with the bar-chart demo—ensure the selector string in the `@Component` metadata and any references in templates use the new 'app-line-chart-demo' identifier and keep the existing templateUrl 'line-chart-demo.component.html' and class name as-is.libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keyboard-nav-plugin.ts-189-205 (1)
189-205:⚠️ Potential issue | 🟡 MinorDon't leave
#focusFromMouselatched after clicking an already-focused canvas.When the canvas already has focus,
mousedownsets the flag but no newfocusevent follows to clear it. The next keyboard focus entry is then treated as mouse-originated and skips#startNavigation().🩹 Suggested fix
`#handleMouseDown`(): void { - this.#focusFromMouse = true; + this.#focusFromMouse = document.activeElement !== this.#canvas; if (this.#isNavigating) { this.#endNavigation(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keyboard-nav-plugin.ts` around lines 189 - 205, The mousedown handler currently sets `#focusFromMouse` but doesn't clear it when the canvas already has focus, causing the next keyboard entry to be misclassified; in `#handleMouseDown`() detect if the chart/canvas element is already document.activeElement (or otherwise focused) and if so reset `#focusFromMouse` = false (and if appropriate call `#startNavigation`() when not `#isNavigating`) instead of leaving the flag latched; reference the existing methods/fields `#handleMouseDown`, `#handleFocus`, `#focusFromMouse`, `#isNavigating`, `#endNavigation` and `#startNavigation` when making the change.libs/components/charts/src/lib/modules/line-chart/line-chart-config.service.ts-36-41 (1)
36-41:⚠️ Potential issue | 🟡 MinorJSDoc typo: "bar chart options" should be "line chart options".
The
@paramdescription references bar chart but this is the line chart config service.Proposed fix
/** * Builds a Chart.js Line Chart configuration based on provided options. * `@remarks` This uses the `SkyChartStyleService.styles` signal to support runtime theming recalculations - * `@param` options bar chart options + * `@param` options line chart options */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/line-chart/line-chart-config.service.ts` around lines 36 - 41, The JSDoc for buildConfig incorrectly says "bar chart options"; update the comment for the public buildConfig(options: SkyLineChartOptions): ChartConfiguration<'line'> to reference "line chart options" (and/or more precisely "line chart options: SkyLineChartOptions") so the `@param` description matches the Line Chart context and the SkyChartStyleService.styles runtime theming remark remains accurate.libs/components/charts/src/lib/modules/donut-chart/donut-chart.component.ts-39-39 (1)
39-39:⚠️ Potential issue | 🟡 MinorTypo:
arialLabelshould beariaLabel.This affects accessibility by potentially not setting the correct ARIA attribute on the canvas element.
Proposed fix
- [ariaLabel]="arialLabel()" + [ariaLabel]="ariaLabel()"And update line 74:
- protected readonly arialLabel = this.#chartService.headingText; + protected readonly ariaLabel = this.#chartService.headingText;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/donut-chart/donut-chart.component.ts` at line 39, The template binding uses the misspelled [ariaLabel]="arialLabel()" which prevents the correct ARIA attribute from being set; rename the template binding to [ariaLabel]="ariaLabel()" and rename (or add) the corresponding component method from arialLabel() to ariaLabel() in DonutChartComponent (update any references/imports to the method name so they match).apps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo.component.ts-23-35 (1)
23-35:⚠️ Potential issue | 🟡 MinorDuplicate import:
SkyBarChartComponentis listed twice.Remove the duplicate entry on line 29.
Proposed fix
imports: [ SkyPageModule, SkyBarChartComponent, SkyBoxModule, SkyTabsModule, SkyFluidGridModule, - SkyBarChartComponent, SkyBarChartSeriesComponent, SkyBarChartSeriesDatapointComponent, SkyChartCategoryAxisComponent, SkyChartMeasureAxisComponent, SkyChartComponent, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo.component.ts` around lines 23 - 35, The imports array currently contains a duplicate entry of SkyBarChartComponent; open the imports array in the BarChartDemo component (the imports list that includes SkyPageModule, SkyBarChartComponent, SkyBoxModule, etc.) and remove the second SkyBarChartComponent entry so each component (e.g., SkyBarChartComponent, SkyBarChartSeriesComponent, SkyBarChartSeriesDatapointComponent, SkyChartCategoryAxisComponent, SkyChartMeasureAxisComponent, SkyChartComponent) appears only once.
🧹 Nitpick comments (14)
apps/playground/src/app/components/charts/shared/chart-demo-utils.ts (1)
76-80: Tightenmonths()strict-mode typing and defaults.Line 78 uses
||, which treats0as unset, and Line 80 creates an untyped array. Prefer nullish coalescing and explicit array typing here.Suggested refactor
- public static months(config: { count?: number; section?: number }): string[] { - const cfg = config || {}; - const count = cfg.count || 12; + public static months(config?: { count?: number; section?: number }): string[] { + const cfg = config ?? {}; + const count = cfg.count ?? 12; const section = cfg.section; - const values = []; + const values: string[] = [];As per coding guidelines: "
**/*.{ts,tsx}: Use strict type checking in TypeScript" and "Avoid theanytype in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/charts/shared/chart-demo-utils.ts` around lines 76 - 80, In months(), tighten typing and defaults by avoiding || and untyped arrays: use the passed-in parameter directly (config.count ?? 12) to default count without treating 0 as missing, keep section as config.section (or explicit config.section ?? undefined if you need a defined fallback), and declare values with an explicit type (string[]) instead of an untyped array; update references to cfg to use config or the new local consts and ensure the method signature and returned array remain strongly typed.libs/components/charts/documentation.json (1)
1-17: Populate documentation IDs before finalizing.The documentation manifest structure is correct. As per coding guidelines, remember to populate:
development.docsIdsandprimaryDocsIdwith main component class namestesting.docsIdswith harness class names (e.g., chart harnesses)codeExamples.docsIdswith example component class namesSince this is a draft PR, empty arrays are acceptable for now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/documentation.json` around lines 1 - 17, Fill the placeholder documentation manifest by adding the main component class names to development.docsIds and setting primaryDocsId to the main component's class name, add test harness class names to testing.docsIds (e.g., chart harness classes), and add example component class names to codeExamples.docsIds; update the JSON keys development.docsIds, primaryDocsId, testing.docsIds, and codeExamples.docsIds in the charts group accordingly so the manifest references the real component, test harness, and example class identifiers.apps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.ts (1)
10-12: Consolidate imports from@skyux/layout.Consider merging the two import statements from
@skyux/layoutinto one.✨ Suggested consolidation
-import { SkyBoxModule } from '@skyux/layout'; -import { SkyFluidGridModule } from '@skyux/layout'; +import { SkyBoxModule, SkyFluidGridModule } from '@skyux/layout';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.ts` around lines 10 - 12, The two separate imports from `@skyux/layout` should be consolidated into a single import statement: replace the separate imports of SkyBoxModule and SkyFluidGridModule with one import that lists both symbols from `@skyux/layout` (so update the import lines that mention SkyBoxModule and SkyFluidGridModule in the donut-chart-demo.component.ts to a single combined import).libs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.ts (1)
100-107: Consider sanitizinglabelTextwhen used as AG Grid field name.Using
seriesItem.labelTextdirectly as thefieldproperty may cause issues if the label contains dots (e.g., "Q1.2024") since AG Grid interprets dots as nested object paths. Consider using a sanitized field name or an index-based approach.Proposed fix using index-based field names
// Add a column for each series - series.forEach((seriesItem) => { + series.forEach((seriesItem, index) => { columns.push({ - field: seriesItem.labelText, + field: `series_${index}`, headerName: seriesItem.labelText, minWidth: 120, }); });Then update
#buildRowDatato use the sameseries_${index}pattern for the field key.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.ts` around lines 100 - 107, The current code pushes columns using seriesItem.labelText for the AG Grid field, which breaks when labels contain dots (e.g., "Q1.2024"); change to a sanitized or index-based field name (e.g., `series_${index}`) when pushing into columns and update the corresponding row construction in the `#buildRowData` method to use the same generated field keys so column.field and row[field] match; ensure headerName still uses seriesItem.labelText for display while field uses the safe key.libs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.spec.ts (1)
35-41: Placeholder test needs completion before merging.The test
expect(true).toBeFalse()will always fail and serves as a placeholder. As per coding guidelines, harness spec files should test all public harness methods, filter functionality, and error scenarios. This is acceptable for a draft PR but should be addressed before the PR is ready for review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.spec.ts` around lines 35 - 41, The placeholder assertion expect(true).toBeFalse() in the bar-chart harness spec should be replaced with real tests that exercise the public harness API: use setupTest() to obtain the fixture, detectChanges(), then locate the BarChartHarness (or whichever harness class is exported) and call its public methods (e.g., getSeries(), getLabels(), selectSeries(), applyFilter()) asserting expected results; add tests for filter behavior by providing different input data and asserting the harness returns filtered items, and include at least one error scenario (e.g., calling a method when the chart has no data) to assert it throws or returns the expected error/empty state. Ensure each test references the harness class name and setupTest() to find the component fixture and uses Angular ComponentHarness loader utilities to retrieve the harness instance.libs/components/charts/src/lib/modules/shared/services/global-chart-config.service.ts (2)
40-42: Type assertion may maskundefinedinput.The cast
as Plugin<TType>[]on line 42 doesn't account forconfig.pluginspotentially beingundefined. While this is handled downstream on line 115 withplugins ?? [], the method signature suggests it expectsPlugin<TType>[]rather thanPlugin<TType>[] | undefined.Consider making the parameter type explicit:
♻️ Suggested fix
`#getMergedPlugins`<TType extends ChartType = ChartType>( - plugins: Plugin<TType>[], + plugins: Plugin<TType>[] | undefined, ): Plugin<TType>[] {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/shared/services/global-chart-config.service.ts` around lines 40 - 42, The call passes config.plugins using a forced cast to Plugin<TType>[] which can mask undefined; update the call or signature so undefined is explicit: either change the invocation to this.#getMergedPlugins(config.plugins ?? []) or modify the `#getMergedPlugins` parameter type to accept Plugin<TType>[] | undefined (and handle undefined inside that method). Ensure references to `#getMergedPlugins` and the local variable plugins reflect the new type to avoid unsafe assertions.
108-113: Type assertion on plugin array.The
as Plugin<TType>[]assertion on line 113 is necessary due to Chart.js plugin typing complexity, but consider adding a brief comment explaining why this cast is required for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/shared/services/global-chart-config.service.ts` around lines 108 - 113, Add a short explanatory comment above the globalPlugins declaration that documents why the array is cast with "as Plugin<TType>[]": explain that Chart.js plugin factory return types are complex/heterogeneous and TypeScript inference can't reconcile the specific plugin factory types (created by createAutoColorPlugin, createTooltipShadowPlugin, createKeyboardNavPlugin, createIndicatorPlugin) with Plugin<TType>, so an explicit cast is required to satisfy the Chart.js API; keep the comment concise and reference the involved symbols (globalPlugins, Plugin<TType>, createAutoColorPlugin, createTooltipShadowPlugin, createKeyboardNavPlugin, createIndicatorPlugin).libs/components/charts/src/index.ts (1)
1-33: Consider exporting additional public types for consumer convenience.The barrel exports the main components and data types, but consumers would benefit from also exporting:
SkyChartHeadingStyle- used bySkyChartComponent.headingStyleinputSkyCategory- used by datapoint componentcategoryinputsThese types are already exported from their respective modules and are part of the public component APIs. Re-exporting them from the barrel file would allow consumers to properly type their variables when working with these components, following the same pattern used for
SkyBarDatum,SkyLineDatum, andSkyDonutDatum.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/index.ts` around lines 1 - 33, Add re-exports for the missing public types so consumers can import them from the barrel: export SkyChartHeadingStyle (used by SkyChartComponent.headingStyle) and export SkyCategory (used by datapoint component category inputs) from their existing modules in the same index.ts where SkyChartComponent, SkyBarDatum, SkyLineDatum, and SkyDonutDatum are re-exported; locate the entries around SkyChartComponent and the datapoint/component exports and add corresponding export statements for SkyChartHeadingStyle and SkyCategory so the types are publicly available from the package root.libs/components/charts/src/lib/modules/line-chart/line-chart.component.ts (1)
88-194: Extract the shared chart/legend orchestration.Most of this block matches
SkyBarChartComponentnearly line-for-line. Pulling the common signals/effects/toggle flow into a shared helper or base would keep the concrete chart components focused on type-specific options and reduce drift.As per coding guidelines, "Keep components small and focused on a single responsibility."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/line-chart/line-chart.component.ts` around lines 88 - 194, Summary: Extract the repeated chart/legend orchestration (signals, effects, and toggle flow) into a shared base/helper to avoid duplication between SkyLineChartComponent and SkyBarChartComponent. Create a shared ChartOrchestration class or abstract BaseChartComponent that encapsulates the computed signals (`#chartOptions`, chartConfiguration, `#legendItems`), the three constructor effects that sync series/legend and handle legend toggles (the effects that call `#chartService.setSeries`, `#chartService.setLegendItems`, and `#chartService.legendItemToggleRequested`), the onChartUpdated handler behavior (`#chartUpdated`), and the legend toggle implementation (`#onLegendItemToggled` which uses `#chart`(), chart.isDatasetVisible, chart.setDatasetVisibility, chart.update, and `#refreshLegendItems`). In the concrete components keep only type-specific parsing (`#parseOptions`) and any chart-specific config building, and have them call/extend the shared helper to register their parse/options logic and to expose their data point click callback. Ensure references to `#chartService`, `#chart`(), `#chartUpdated`, `#refreshLegendItems`, and dataPointClicked are preserved or forwarded by the shared implementation so existing interactions continue to work.apps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo.component.ts (1)
190-193: Empty placeholder arrays may cause confusion in the demo.
horizontalSingleSeries,horizontalMultiSeries, andhorizontalLogare empty arrays. Consider adding a TODO comment or populating with sample data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo.component.ts` around lines 190 - 193, The demo exposes three empty arrays (horizontalSingleSeries, horizontalMultiSeries, horizontalLog) which are confusing; either populate them with representative sample data or add clear TODO comments explaining they are placeholders. Locate the declarations of horizontalSingleSeries, horizontalMultiSeries and horizontalLog in the BarChartDemo component and replace the empty arrays with small example datasets matching the chart shape used elsewhere in the component, or add a one-line TODO comment above each declaration indicating it’s intentionally empty and where real demo data should be added.libs/components/charts/src/lib/modules/bar-chart/bar-chart-config.service.ts (1)
107-112: Consider removing or documenting the commented-out code.The commented
barPercentageoption on line 110 should either be removed or have a comment explaining why it's preserved.Proposed fix
datasets: { bar: { categoryPercentage: 0.7, - // barPercentage: 0.7, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/bar-chart/bar-chart-config.service.ts` around lines 107 - 112, The commented-out barPercentage option in the datasets.bar config should be either removed or documented: update the BarChartConfigService (datasets -> bar) by deleting the commented line "// barPercentage: 0.7," if it's no longer needed, or replace it with a short explanatory comment (e.g., why barPercentage was disabled/kept and when to re-enable) so future readers understand the intent.libs/components/charts/src/lib/modules/line-chart/line-chart-registry.service.ts (1)
14-88: Consistent registry implementation.The service mirrors
SkyBarChartRegistrywith appropriate type differences. Consider extracting a shared base class or generic registry implementation in the future to reduce duplication, though this is acceptable for the current scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/line-chart/line-chart-registry.service.ts` around lines 14 - 88, SkyLineChartRegistry largely duplicates logic found in SkyBarChartRegistry; extract the shared behavior into a generic base (e.g., BaseChartRegistry<TPoint> or ChartRegistryBase) that implements common signals and methods (categoryAxis, measureAxis, series signals plus upsert/removeCategoryAxis, upsert/removeMeasureAxis, upsert/removeSeries, upsert/removePoint) and have SkyLineChartRegistry and SkyBarChartRegistry extend it while preserving their specific point types (SkyLineChartPoint) and interfaces (SkyChartRegistry, SkyChartAxisRegistry); update references to the unique symbols SkyLineChartRegistry and SkyBarChartRegistry so each subclass only contains type-specific details.libs/components/charts/src/lib/modules/shared/chart-helpers.ts (2)
82-92: Variable shadowing inparseCategoriesreduces readability.The inner callback parameter
seriesshadows the outerseriesparameter, which can cause confusion.Suggested rename to avoid shadowing
export function parseCategories( series: readonly SkyChartSeries<SkyChartDataPoint>[], ): SkyCategory[] { - const allCategories = series.flatMap((series) => - series.data.map((dp) => dp.category), + const allCategories = series.flatMap((s) => + s.data.map((dp) => dp.category), ); const uniqueCategories = Array.from(new Set(allCategories)); return uniqueCategories; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/shared/chart-helpers.ts` around lines 82 - 92, The parseCategories function currently shadows its outer parameter series by naming the arrow callback parameter the same; update the inner callback parameter name (e.g., change the inner "series" to "s" or "srv") in the series.flatMap call inside parseCategories so the outer parameter series remains unshadowed and the code is clearer; ensure you update both occurrences inside the flatMap (the callback param and its usage series.data) to the new name.
146-158: Floating-point precision in power-of-10 check is unlikely to cause issues in practice for typical chart values.The current implementation
Math.log10(numeric) % 1 === 0works correctly for standard chart axis ranges. Verification shows powers of 10 (including extreme values like 1e23) return exact integer log10 values without precision loss in JavaScript. However, a tolerance-based comparison is still a valid optional improvement if robustness against edge cases is desired.Optional: tolerance-based power-of-10 check
- // Show only powers of 10 - const isPowerOf10 = numeric > 0 && Math.log10(numeric) % 1 === 0; + // Show only powers of 10 (with tolerance for floating-point precision) + const log10 = Math.log10(numeric); + const isPowerOf10 = numeric > 0 && Math.abs(log10 - Math.round(log10)) < 1e-10;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/charts/src/lib/modules/shared/chart-helpers.ts` around lines 146 - 158, The isPowerOf10 check in createLogTickFilter uses strict equality on Math.log10(numeric) % 1 === 0 which can fail for edge-case floating precision; change it to a tolerance-based comparison in the createLogTickFilter function: ensure numeric is finite and > 0, compute const log = Math.log10(numeric), then treat it as a power of ten when Math.abs(log - Math.round(log)) < Number.EPSILON * 100 (or a small tolerance like 1e-12); update the isPowerOf10 logic and keep the same return behavior (empty string for non-powers, numeric.toLocaleString() for powers).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 633c11ba-c723-4349-8637-4bc465d1e9d6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (107)
apps/playground/src/app/app.routes.tsapps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo-routes.tsapps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo.component.htmlapps/playground/src/app/components/charts/bar-chart-demo/bar-chart-demo.component.tsapps/playground/src/app/components/charts/charts-routes.tsapps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo-routes.tsapps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.htmlapps/playground/src/app/components/charts/donut-chart-demo/donut-chart-demo.component.tsapps/playground/src/app/components/charts/line-chart-demo/line-chart-demo-routes.tsapps/playground/src/app/components/charts/line-chart-demo/line-chart-demo.component.htmlapps/playground/src/app/components/charts/line-chart-demo/line-chart-demo.component.tsapps/playground/src/app/components/charts/shared/chart-demo-utils.tsapps/playground/src/app/components/components.module.tslibs/components/charts/CHANGELOG.mdlibs/components/charts/README.mdlibs/components/charts/documentation.jsonlibs/components/charts/eslint.config.jslibs/components/charts/karma.conf.jslibs/components/charts/ng-package.jsonlibs/components/charts/package.jsonlibs/components/charts/project.jsonlibs/components/charts/src/assets/locales/resources_en_US.jsonlibs/components/charts/src/index.tslibs/components/charts/src/lib/modules/axis/chart-category-axis.component.tslibs/components/charts/src/lib/modules/axis/chart-measure-axis.component.tslibs/components/charts/src/lib/modules/axis/sky-chart-axis-registry.service.tslibs/components/charts/src/lib/modules/bar-chart/bar-chart-config.service.tslibs/components/charts/src/lib/modules/bar-chart/bar-chart-registry.service.tslibs/components/charts/src/lib/modules/bar-chart/bar-chart-series-datapoint.component.tslibs/components/charts/src/lib/modules/bar-chart/bar-chart-series.component.tslibs/components/charts/src/lib/modules/bar-chart/bar-chart-types.tslibs/components/charts/src/lib/modules/bar-chart/bar-chart.component.tslibs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal-context.tslibs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.htmllibs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.scsslibs/components/charts/src/lib/modules/chart-data-grid-modal/chart-data-grid-modal.component.tslibs/components/charts/src/lib/modules/chart-legend/chart-legend-item.tslibs/components/charts/src/lib/modules/chart-legend/chart-legend.component.htmllibs/components/charts/src/lib/modules/chart-legend/chart-legend.component.scsslibs/components/charts/src/lib/modules/chart-legend/chart-legend.component.tslibs/components/charts/src/lib/modules/chart/chart-header-id-token.tslibs/components/charts/src/lib/modules/chart/chart.component.htmllibs/components/charts/src/lib/modules/chart/chart.component.scsslibs/components/charts/src/lib/modules/chart/chart.component.tslibs/components/charts/src/lib/modules/chart/chart.service.tslibs/components/charts/src/lib/modules/chartjs.directive.tslibs/components/charts/src/lib/modules/donut-chart/donut-chart-config.service.tslibs/components/charts/src/lib/modules/donut-chart/donut-chart-registry.service.tslibs/components/charts/src/lib/modules/donut-chart/donut-chart-series-datapoint.component.tslibs/components/charts/src/lib/modules/donut-chart/donut-chart-series.component.tslibs/components/charts/src/lib/modules/donut-chart/donut-chart-types.tslibs/components/charts/src/lib/modules/donut-chart/donut-chart.component.tslibs/components/charts/src/lib/modules/line-chart/line-chart-config.service.tslibs/components/charts/src/lib/modules/line-chart/line-chart-registry.service.tslibs/components/charts/src/lib/modules/line-chart/line-chart-series-datapoint.component.tslibs/components/charts/src/lib/modules/line-chart/line-chart-series.component.tslibs/components/charts/src/lib/modules/line-chart/line-chart-types.tslibs/components/charts/src/lib/modules/line-chart/line-chart.component.tslibs/components/charts/src/lib/modules/shared/chart-helpers.tslibs/components/charts/src/lib/modules/shared/plugins/auto-color/auto-color-plugin.tslibs/components/charts/src/lib/modules/shared/plugins/indicator/bar-indicator-bounds.tslibs/components/charts/src/lib/modules/shared/plugins/indicator/donut-indicator-helpers.tslibs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-draw.tslibs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-plugin-options.tslibs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-plugin.tslibs/components/charts/src/lib/modules/shared/plugins/indicator/indicator-types.tslibs/components/charts/src/lib/modules/shared/plugins/indicator/line-indicator-bounds.tslibs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/cartesian-navigation-strategy.tslibs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/create-navigation-strategy.tslibs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keyboard-nav-plugin-options.tslibs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keyboard-nav-plugin.tslibs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/keys.tslibs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/navigation-strategy.tslibs/components/charts/src/lib/modules/shared/plugins/keyboard-nav/radial-navigation-strategy.tslibs/components/charts/src/lib/modules/shared/plugins/plugin-state/focused-elements-state.tslibs/components/charts/src/lib/modules/shared/plugins/tooltip/tooltip-options.tslibs/components/charts/src/lib/modules/shared/plugins/tooltip/tooltip-shadow-plugin.tslibs/components/charts/src/lib/modules/shared/services/chart-registry.service.tslibs/components/charts/src/lib/modules/shared/services/chart-style.service.tslibs/components/charts/src/lib/modules/shared/services/global-chart-config.service.tslibs/components/charts/src/lib/modules/shared/sky-charts-resources.module.tslibs/components/charts/src/lib/modules/shared/types/axis-types.tslibs/components/charts/src/lib/modules/shared/types/category.tslibs/components/charts/src/lib/modules/shared/types/chart-clicked-data-point.tslibs/components/charts/src/lib/modules/shared/types/chart-data-point.tslibs/components/charts/src/lib/modules/shared/types/chart-heading-level.tslibs/components/charts/src/lib/modules/shared/types/chart-heading-style.tslibs/components/charts/src/lib/modules/shared/types/chart-series.tslibs/components/charts/src/lib/modules/shared/types/deep-partial-type.tslibs/components/charts/testing/eslint.config.jslibs/components/charts/testing/karma.conf.jslibs/components/charts/testing/ng-package.jsonlibs/components/charts/testing/project.jsonlibs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.filters.tslibs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.spec.tslibs/components/charts/testing/src/modules/bar-chart/bar-chart-harness.tslibs/components/charts/testing/src/modules/bar-chart/fixtures/bar-chart-harness-test.component.htmllibs/components/charts/testing/src/modules/bar-chart/fixtures/bar-chart-harness-test.component.tslibs/components/charts/testing/src/public-api.tslibs/components/charts/testing/tsconfig.spec.jsonlibs/components/charts/tsconfig.jsonlibs/components/charts/tsconfig.lib.jsonlibs/components/charts/tsconfig.lib.prod.jsonlibs/components/charts/tsconfig.spec.jsonlibs/components/theme/package.jsonpackage.jsontsconfig.base.json
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
changed the title
ThomasOrtiz
changed the title
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
…ns and streamline scale creation
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
commented
Apr 17, 2026
| // #endregion | ||
|
|
||
| // #region Outputs | ||
| public readonly dataPointClicked = |
Contributor
Author
There was a problem hiding this comment.
Unfortunately Host Directives don't support generics and since the dataPointClick type is dependent on the datum type SkyChartDataPointClickArgs<SkyChart[Type]Datum>.
We can use a Host Directive here but the consumer loses the output type's specificity around the generic: output<SkyChartDataPointClickArgs<unknown>>().
…en readers do not eat keyboard input
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
…rt config to use as figure's aria label
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
ThomasOrtiz
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a draft PR to collect early feedback on the new Charts package before merge readiness.
Not intended to merge in current state.
Feature progress
UX Review
TODO
Summary by CodeRabbit