build(deps): update dependencies and sync color palette#195
Conversation
LFXV2-866 - Update @linuxfoundation/lfx-ui-core to 0.0.21 - Update @commitlint/cli to 20.1.0 and config-angular to 20.0.0 - Update prettier to 3.7.4, lint-staged to 16.2.7, chart.js to 4.5.1 - Sync color palette (gray, emerald, red, amber, violet) with lfx-ui-core - Fix chart.js TooltipItem types to use generic ChartType - Add null-safety operators for optional chart values - Simplify shadow classes to use Tailwind shadow-md - Update RSVP button styling from cyan to blue Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughDependency and devDependency versions bumped; Tailwind styling classes consolidated (custom shadows → Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull request overview
This PR updates project dependencies and synchronizes the color palette with the lfx-ui-core design system. The changes improve TypeScript type safety for Chart.js tooltips and modernize styling by replacing verbose inline shadow definitions with Tailwind utilities.
- Updated multiple development dependencies to their latest versions (@commitlint/*, prettier, lint-staged, chart.js)
- Synchronized color palette (gray, emerald, red, amber, violet) with lfx-ui-core v0.0.21 design system
- Improved Chart.js TypeScript types by using generic
ChartTypeinstead of specific chart types and simplifying tooltip callback type annotations - Modernized UI styling by replacing verbose inline shadow classes with Tailwind's
shadow-mdutility and updating RSVP button color scheme from cyan to blue
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated dev dependencies: @commitlint/* to v20.x, prettier to v3.7.4, lint-staged to v16.2.7, lfx-ui-core to v0.0.21, chart.js to v4.5.1 |
| apps/lfx-one/package.json | Updated lfx-ui-core and prettier versions to match root package |
| yarn.lock | Reflects all dependency version updates with checksums |
| packages/shared/src/constants/colors.constants.ts | Synchronized all color palettes (gray, emerald, red, amber, violet) with lfx-ui-core design system |
| packages/shared/src/constants/chart-options.constants.ts | Changed chart option types from specific ('line', 'bar') to generic ChartType for better flexibility |
| apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts | Removed explicit TooltipItem imports and type annotations, added nullish coalescing for parsed values, fixed pointStyle type |
| apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts | Updated import to include ChartType, simplified chart option method signatures, improved tooltip title handling |
| apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts | Added ChartType import, updated tooltip callbacks to use generic types, added nullish coalescing for safety |
| apps/lfx-one/src/app/modules/meetings/components/rsvp-button-group/rsvp-button-group.component.html | Changed RSVP button color scheme from cyan to blue for better design consistency |
| apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html | Simplified shadow styling from verbose inline to Tailwind's shadow-md utility |
| apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html | Simplified shadow styling from verbose inline to Tailwind's shadow-md utility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...x-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
Outdated
Show resolved
Hide resolved
...x-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
Outdated
Show resolved
Hide resolved
...modules/dashboards/components/organization-involvement/organization-involvement.component.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/shared/src/constants/chart-options.constants.ts (1)
1-3: Add Claude Code attribution per coding guidelines.As per coding guidelines, when code is assisted by AI tools, a comment should be prepended:
Generated with Claude Code (https://claude.ai/code). Since the PR description indicates this code was generated with Claude Code, please add this attribution after the license header.Apply this diff:
// Copyright The Linux Foundation and each contributor to LFX. // SPDX-License-Identifier: MIT +// Generated with Claude Code (https://claude.ai/code) import type { ChartData, ChartOptions, ChartType } from 'chart.js';apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (2)
654-668: Add null safety forcontext.parsed.y.Same issue as in
createBarChartOptions: the tooltip callback lacks null safety forcontext.parsed.y, which could result in displayingundefinedor causing runtime errors.Apply this diff:
tooltip: { ...(BASE_LINE_CHART_OPTIONS.plugins?.tooltip ?? {}), callbacks: { title: (context) => context[0]?.label ?? '', - label: (context) => `${label}: ${context.parsed.y}`, + label: (context) => `${label}: ${context.parsed.y ?? 0}`, }, },
1-3: Add Claude Code attribution per coding guidelines.As per coding guidelines, when code is assisted by AI tools, a comment should be prepended:
Generated with Claude Code (https://claude.ai/code). Since the PR description indicates this code was generated with Claude Code, please add this attribution after the license header.Apply this diff:
// Copyright The Linux Foundation and each contributor to LFX. // SPDX-License-Identifier: MIT +// Generated with Claude Code (https://claude.ai/code) import { CommonModule } from '@angular/common';
🧹 Nitpick comments (7)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (5)
1-3: Missing “Generated with Claude Code” header markerRepo guidelines say to prepend
Generated with Claude Code (https://claude.ai/code)on TS/JS files assisted by Claude Code. Since the PR description notes that this PR was generated with Claude Code, consider adding that marker above the existing copyright/SPDX header here (and in other touched TS files) to stay consistent with repo conventions.
24-25: chart.js type imports are correct; consider tightening local typing to avoid castsUsing
import type { ChartOptions, ChartType, TooltipItem } from 'chart.js';is the right direction for the new generics. To avoid needingas ChartOptions<ChartType>later, you could explicitly type the local options:- public readonly sparklineOptions = BASE_LINE_CHART_OPTIONS; - public readonly barChartOptions = BASE_BAR_CHART_OPTIONS; + public readonly sparklineOptions: ChartOptions<ChartType> = BASE_LINE_CHART_OPTIONS; + public readonly barChartOptions: ChartOptions<ChartType> = BASE_BAR_CHART_OPTIONS;That lets tooltip callbacks get proper contextual typing while keeping assignments type‑safe.
Also applies to: 73-75
221-237: Add null-safety forparsed.yin Total Projects tooltip for consistencyIn the Total Projects tooltip label you still use
context.parsed.ydirectly, whereas other cards (e.g., Active Contributors) now guard with?? 0. To avoid potentialundefinedleaking into the label and to align with the rest of the file:- label: (context: TooltipItem<ChartType>) => { - const count = context.parsed.y; + label: (context: TooltipItem<ChartType>) => { + const count = context.parsed.y ?? 0; return `Total projects: ${count}`; },
263-277: Mirror the sameparsed.yguard in Total Members tooltipSame pattern as above applies to the Total Members tooltip; using
?? 0would keep behavior consistent with other metrics:- label: (context: TooltipItem<ChartType>) => { - const count = context.parsed.y; + label: (context: TooltipItem<ChartType>) => { + const count = context.parsed.y ?? 0; return `Total members: ${count}`; },
343-358: Consider avoiding inlineas ChartOptions<ChartType>cast on Active ContributorsThe Active Contributors card’s
chartOptionsis forced toChartOptions<ChartType>via a cast. If you typesparklineOptionsasChartOptions<ChartType>(see earlier suggestion), this cast should become unnecessary and TypeScript can validate the callbacks structurally instead of trusting the assertion.apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (2)
200-213: Tooltip callbacks: good null‑safety; consider consistent guarding around labels/date parsingAcross these charts you’ve:
- Standardized title formatting via
parseLocalDateStringand localetoLocaleDateString(...).- Added
?? 0guards forcontext.parsed.ybefore numeric formatting in several places.- Wrapped some callbacks in
try/catchto avoid tooltip crashes when data is malformed (nice touch in the PR velocity and unique contributors tooltips).Two optional cleanups to consider for long‑term robustness:
Guard
context[0]access everywhere
Some callbacks still assumecontext[0]is always present (e.g.,context[0].label), while others already usecontext[0]?.label || ''. For consistency and to harden against edge cases, you could uniformly switch to the guarded pattern.Align
parseLocalDateStringusage with existing try/catch style
Where you callparseLocalDateStringwithout a try/catch, it assumes labels are alwaysYYYY-MM-DD. If there’s any chance those labels change format, you might want to mirror the defensive pattern you’re already using in the PR velocity and unique contributors tooltips (returning the raw label as a fallback).These are not blockers but would make the tooltip layer more resilient.
Also applies to: 243-256, 318-336, 384-417, 430-432, 459-468, 517-557, 595-608, 643-657
1-3: Add Claude Code header marker here as wellSame note as for
FoundationHealthComponent: since the PR is marked as generated with Claude Code, consider adding theGenerated with Claude Code (https://claude.ai/code)comment above the existing copyright/SPDX header to comply with the repo’s TS/JS header guideline.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
apps/lfx-one/package.json(2 hunks)apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html(1 hunks)apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts(6 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts(13 hunks)apps/lfx-one/src/app/modules/meetings/components/rsvp-button-group/rsvp-button-group.component.html(1 hunks)package.json(2 hunks)packages/shared/src/constants/chart-options.constants.ts(5 hunks)packages/shared/src/constants/colors.constants.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.htmlapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/meetings/components/rsvp-button-group/rsvp-button-group.component.htmlpackages/shared/src/constants/chart-options.constants.tspackages/shared/src/constants/colors.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Use data-testid naming convention - [section]-[component]-[element] for hierarchical structure
Files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.htmlapps/lfx-one/src/app/modules/meetings/components/rsvp-button-group/rsvp-button-group.component.html
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Always use direct imports for standalone components - no barrel exports
Use TypeScript interfaces instead of union types for better maintainability
Do not nest ternary expressions
Prefer interface for defining object shapes in TypeScript
Use path mappings and import aliases as configured in tsconfig.json for imports
Files:
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tspackages/shared/src/constants/chart-options.constants.tspackages/shared/src/constants/colors.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: License headers are required on all source files
Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code
Files:
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tspackages/shared/src/constants/chart-options.constants.tspackages/shared/src/constants/colors.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Angular 19 zoneless change detection with signals for component state management
Files:
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants are centralized in @lfx-one/shared package
Files:
packages/shared/src/constants/chart-options.constants.tspackages/shared/src/constants/colors.constants.ts
🧬 Code graph analysis (2)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)
packages/shared/src/utils/date-time.utils.ts (1)
parseLocalDateString(52-65)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
packages/shared/src/constants/chart-options.constants.ts (2)
BASE_BAR_CHART_OPTIONS(112-133)BASE_LINE_CHART_OPTIONS(87-106)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (9)
apps/lfx-one/src/app/modules/committees/committee-manage/committee-manage.component.html (1)
18-27: Form card shadow tweak is fineSwitching to
shadow-mdstandardizes shadows and doesn’t affect behavior or testability.apps/lfx-one/package.json (1)
42-42: Dependency bumps look consistent; verify formatting & UIUpdating
@linuxfoundation/lfx-ui-coreto^0.0.21andprettierto^3.7.4is aligned with the PR goals. Please re-run formatting and a quick UI/regression pass onlfx-oneafter install to catch any breaking config or style shifts from these versions.Also applies to: 86-86
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html (1)
4-5: Standardized card shadow on committee table looks goodUsing
shadow-mdon thelfx-cardkeeps styling consistent across committee screens without impacting bindings or existingdata-testidhooks.apps/lfx-one/src/app/modules/meetings/components/rsvp-button-group/rsvp-button-group.component.html (1)
6-17: RSVP container/header blue theming is consistent and safeThe conditional classes still gate correctly on
selectedResponse()anddisabled(), and the new blue shades align with the updated primary palette without affecting behavior.package.json (1)
31-47: Tooling and chart.js version bumps are reasonable; run checks across the monorepoBumping commitlint, lint-staged, prettier,
@linuxfoundation/lfx-ui-core, andchart.jsto the new versions fits the PR intent. Please make sure:
yarn lint,yarn format:check, andyarn commitlintstill pass under the new toolchain.- Charts render correctly with
chart.js@^4.5.1given the updated tooltip/ChartOptions typings elsewhere in the repo.Also applies to: 65-67
packages/shared/src/constants/colors.constants.ts (1)
32-105: Color palette sync preserves API and aligns with design systemThe gray/emerald/red/amber/violet scales keep the same shade keys (50–950) while updating hex values, so existing consumers of
lfxColorsremain source-compatible. This looks like a clean sync with the design tokens.apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (1)
384-399: New tooltip callbacks for Maintainers and Events look robustFor Maintainers and Events, the callbacks now:
- Use
TooltipItem<ChartType>[]withcontext[0]?.label ?? ''for titles.- Guard
parsed.ywith?? 0where numeric values are rendered.This is consistent with the shared ChartOptions typing and should be safe across line/bar variants.
Also applies to: 429-445
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)
161-169: Active Weeks Streak tooltip logic is straightforwardMapping
context.parsed.y === 1to'Active'/'Inactive'matches the 0/1 data encoding and keeps the tooltip easy to read. No issues here.apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
27-27: LGTM!The import change from
TooltipItemtoChartTypealigns with the broader refactoring to use generic chart types throughout the codebase.
...modules/dashboards/components/organization-involvement/organization-involvement.component.ts
Show resolved
Hide resolved
LFXV2-866 Add consistent ?? 0 fallback for context.parsed.y in chart tooltip callbacks across dashboard components to handle potential undefined/null values safely. Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (4)
161-165: Make tooltip titles consistently null‑safe oncontext[0]Several tooltip
titlecallbacks still assumecontext[0]is always present (context[0].label), while others use optional chaining and empty‑string fallbacks. For consistency and a bit more robustness against unexpected empty tooltip contexts, consider patterns like:title: (context) => context[0]?.label ?? '' // or, when parsing dates: const dateStr = context[0]?.label || ''; if (!dateStr) return '';This applies to the title callbacks around these blocks as well as the ones returning raw labels.
Also applies to: 200-205, 243-247, 318-322, 459-462, 595-599
200-204: Factor out repeated date formatting for tooltip titlesThe
parseLocalDateString(...).toLocaleDateString('en-US', { ... })pattern is repeated across multiple tooltiptitleimplementations. Extracting a small helper would reduce duplication and keep formatting changes localized, e.g.:private formatTooltipDateLabel(dateStr: string, prefix?: string): string { if (!dateStr) return ''; const date = parseLocalDateString(dateStr); const formatted = date.toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric', }); return prefix ? `${prefix} ${formatted}` : formatted; }Then each callback can call this helper (with or without a
"Week of"prefix), keeping tooltip code leaner.Also applies to: 243-247, 318-322, 384-390, 517-523, 595-599, 643-647
384-417: Prefer guards over try/catch +console.errorin tooltip callbacksThe weekly PR and unique‑contributors tooltip callbacks now wrap their logic in
try/catchand log errors on every failure. Given these run on hover, repeated exceptions could spam the console.You can avoid most of this by adding simple guards before indexing:
const first = context[0]; if (!first) return ''; // or [] const dataIndex = first.dataIndex; if (dataIndex == null || !chartData[dataIndex]) return ''; // or []and by null‑coalescing numeric fields (e.g.
weekData.AVG_MERGED_IN_DAYS ?? 0) instead of relying on exceptions. That keeps tooltips safe without noisy logging.Also applies to: 529-552, 643-651
1-3: Add “Generated with Claude Code” header per guidelinesPer the project guidelines for
*.tsfiles assisted by Claude Code, prepend a comment like:// Generated with Claude Code (https://claude.ai/code)near the top of the file (typically above or just below the license header).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts(5 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts(13 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Always use direct imports for standalone components - no barrel exports
Use TypeScript interfaces instead of union types for better maintainability
Do not nest ternary expressions
Prefer interface for defining object shapes in TypeScript
Use path mappings and import aliases as configured in tsconfig.json for imports
Files:
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: License headers are required on all source files
Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code
Files:
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Angular 19 zoneless change detection with signals for component state management
Files:
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
🧬 Code graph analysis (2)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)
packages/shared/src/utils/date-time.utils.ts (1)
parseLocalDateString(52-65)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
packages/shared/src/constants/chart-options.constants.ts (2)
BASE_BAR_CHART_OPTIONS(112-133)BASE_LINE_CHART_OPTIONS(87-106)
🔇 Additional comments (7)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (2)
27-27: LGTM: Type generalization is consistent with shared constants.The change from specific chart types (
'bar','line') to the genericChartTypealigns with the typing used inBASE_BAR_CHART_OPTIONSandBASE_LINE_CHART_OPTIONS(as shown in relevant snippets fromchart-options.constants.ts). This provides flexibility while maintaining type safety through inference.Also applies to: 638-638, 654-654
646-647: LGTM: Null safety additions address previous review feedback.The tooltip callbacks now include appropriate null safety:
context[0]?.label ?? ''handles missing context elementscontext.parsed.y ?? 0prevents displayingundefinedfor invalid/missing data pointsThis directly resolves the concerns raised in past review comments about potential runtime errors when Chart.js returns null/undefined for parsed values.
Also applies to: 662-663
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (5)
227-230: LGTM! Improved null-safety for tooltip callbacks.The addition of optional chaining (
context[0]?.label) and nullish coalescing (?? 0) properly guards against undefined values in the tooltip context. This makes the callback more robust and consistent with the other charts in this component.
268-271: Consistent null-safety applied.Good addition of the null-safety operators here as well. This addresses the previous review feedback about inconsistent use of
?? 0across tooltip callbacks.
348-351: Consistent implementation.The null-safety pattern is correctly applied here, maintaining consistency across all chart tooltip callbacks in this component.
389-392: LGTM!Consistent application of the null-safety pattern.
435-438: Excellent consistency across all charts.The same defensive pattern works well for both line and bar charts. All five tooltip callbacks in this component now consistently use optional chaining and nullish coalescing for safe data access. This aligns with the PR objectives to improve Chart.js TypeScript safety with null-safety operators.
Summary
Fixes: LFXV2-866
🤖 Generated with Claude Code