refactor(dashboards): update foundation health to carousel layout #148
Conversation
LFXV2-719 - Convert foundation health component from table to carousel layout with filtering - Add aggregate metrics across all foundations (8 metric cards) - Implement category-based filters (All, Contributors, Projects, Events) - Follow organization involvement patterns for data initialization - Pre-compute all display values in computed signals - Add various visualization types (sparklines, bar charts, bus factor, health scores) - Use justify-between for proper card vertical spacing - Remove public formatting methods from HTML templates Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]>
|
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. WalkthroughReplaces the table-based Foundation Health view with a carousel of metric cards driven by Angular signals, adds filter pills and carousel navigation, and introduces new shared interfaces and aggregate mock constants for sparkline/bar charts, top projects, bus-factor, and health-score distributions. Changes
Sequence DiagramsequenceDiagram
participant User
participant FHComponent as FoundationHealthComponent
participant Signal as selectedFilter (signal)
participant Computed as metricCards (computed)
participant Template
User->>FHComponent: Click filter pill
FHComponent->>Signal: handleFilterChange(filter)
Signal->>Computed: trigger recompute
Computed-->>Template: provide filtered metricCards
Template->>User: render carousel of metric cards
User->>FHComponent: Click prev/next
FHComponent->>FHComponent: scrollLeft()/scrollRight()
FHComponent->>Template: adjust carousel scroll (carouselScroll)
Template-->>User: animated scroll
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
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 refactors the foundation health component from a table-based layout to a modern carousel layout with category-based filtering, displaying 8 aggregate metric cards across all foundations with various visualization types.
Key changes:
- Replaced table layout with a horizontally scrollable carousel of metric cards
- Added filter pills for categorizing metrics (All, Contributors, Projects, Events)
- Integrated 8 metric cards with different visualizations: sparklines, bar charts, top projects list, bus factor, and health score distribution
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/index.ts |
Exports new foundation metrics interfaces |
packages/shared/src/interfaces/foundation-metrics.interface.ts |
Defines TypeScript interfaces for metric cards, categories, and aggregate data |
packages/shared/src/constants/foundation-health.constants.ts |
Adds chart configurations and aggregate metrics constants with mock data |
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts |
Refactors component to use carousel with filtering, computed signals, and direct chart integration |
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.scss |
Adds CSS class to hide scrollbars on carousel |
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html |
Updates template from table to carousel with filter pills and navigation controls |
Comments suppressed due to low confidence (1)
packages/shared/src/constants/foundation-health.constants.ts:29
- The comment states 'mock data for 5 major foundations' but the
FOUNDATION_HEALTH_DATAarray only contains 1 foundation (CNCF). Either add the remaining 4 foundations or update the comment to reflect the current implementation.
* Matches the React implementation with mock data for 5 major foundations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html
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
📜 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 (6)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.scss(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts(3 hunks)packages/shared/src/constants/foundation-health.constants.ts(2 hunks)packages/shared/src/interfaces/foundation-metrics.interface.ts(1 hunks)packages/shared/src/interfaces/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (4)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
Component(17-416)apps/lfx-one/src/app/shared/components/filter-pills/filter-pills.component.ts (1)
FilterOption(7-10)packages/shared/src/constants/foundation-health.constants.ts (3)
FOUNDATION_SPARKLINE_CHART_OPTIONS(67-75)FOUNDATION_BAR_CHART_OPTIONS(81-97)AGGREGATE_FOUNDATION_METRICS(150-166)packages/shared/src/interfaces/foundation-metrics.interface.ts (3)
FoundationMetricCard(14-48)MetricCategory(8-8)TopProjectDisplay(97-102)
packages/shared/src/constants/foundation-health.constants.ts (1)
packages/shared/src/interfaces/foundation-metrics.interface.ts (4)
ProjectHealthDistribution(54-65)CompanyBusFactor(71-80)TopProjectByValue(86-91)AggregateFoundationMetrics(108-139)
⏰ 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: Upload results
...x-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
Show resolved
Hide resolved
LFXV2-719 - Change FOUNDATION_HEALTH_DATA from array to single object - Remove array indexing since only aggregate data is used - Simplify Tailwind utility classes for better maintainability Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/shared/src/constants/foundation-health.constants.ts (1)
31-59: Consider renaming for clarity.The export name
FOUNDATION_HEALTH_DATA(plural) with singular typeFoundationmay cause confusion. Since this now represents a single aggregate foundation rather than an array, consider renaming toAGGREGATE_FOUNDATION_DATAor changing the type annotation to reflect its aggregate nature.Example:
-export const FOUNDATION_HEALTH_DATA: Foundation = { +export const AGGREGATE_FOUNDATION_DATA: Foundation = { id: 'aggregate',apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html (1)
73-85: Simplify padding utilities.The padding classes can be streamlined by removing the redundant
m-0which is immediately overridden bymb-[5px].- <div class="space-y-0.5 p-0 pr-5 pl-[60px] m-0 mb-[5px]"> + <div class="space-y-0.5 p-0 pr-5 pl-[60px] mb-[5px]">
📜 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 (2)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html(1 hunks)packages/shared/src/constants/foundation-health.constants.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/src/constants/foundation-health.constants.ts (2)
packages/shared/src/interfaces/dashboard.interface.ts (1)
Foundation(193-222)packages/shared/src/interfaces/foundation-metrics.interface.ts (4)
ProjectHealthDistribution(54-65)CompanyBusFactor(71-80)TopProjectByValue(86-91)AggregateFoundationMetrics(108-139)
🔇 Additional comments (15)
packages/shared/src/constants/foundation-health.constants.ts (6)
4-4: LGTM!The imports are clean and all imported interfaces are appropriately used throughout the file.
13-25: LGTM!The smooth data generator correctly implements a controlled random walk to create realistic trend data for sparklines.
100-109: LGTM!The helper functions correctly implement average and sum calculations for metric aggregation.
65-95: LGTM!The chart configurations are appropriate for embedded sparkline and bar chart visualizations. Disabling legends, tooltips, and axes is correct for compact metric displays.
115-142: LGTM!The mock data constants are well-structured and the values are internally consistent (e.g., health distribution sums to 1139, bus factor percentages sum to 100%).
148-164: LGTM!The aggregate metrics are correctly computed:
totalProjectsproperly sums the health distribution (1139)totalProjectsDatauses the matching base value- All metric calculations align with the new single aggregate foundation approach
The previous review concerns about aggregating from index [0] are now resolved by the structural change from an array to a single aggregate object.
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html (9)
5-15: LGTM!The header layout correctly integrates the dynamic title signal and filter pills component with appropriate data attributes for testing.
18-35: LGTM!The carousel navigation buttons are properly implemented with:
- Explicit
type="button"to prevent form submissionaria-labelattributes for accessibility- Test IDs for E2E testing
39-40: LGTM!The carousel container is well-configured with smooth scrolling, appropriate spacing, and the
hide-scrollbarutility class as specified in the PR objectives.
41-48: LGTM!The card structure correctly implements:
- Responsive widths (mobile-first with desktop override)
justify-betweenfor vertical spacing as specified in objectives- Dynamic test IDs and icons from card configuration
51-59: LGTM!The sparkline chart correctly integrates ChartComponent directly without a wrapper, as specified in the PR objectives.
62-70: LGTM!The bar chart implementation correctly uses direct ChartComponent integration with appropriate fixed dimensions for consistent metric visualization.
88-108: LGTM!The bus factor visualization correctly implements a proportional inline bar chart using dynamic width bindings from pre-computed percentages.
111-130: LGTM!The health score distribution correctly renders a bar chart using pre-computed display values from the
healthScoreDistribution()signal, following the PR objective to remove formatting logic from templates.
134-143: LGTM!The card footer conditionally renders pre-computed display values with appropriate styling and spacing.
Summary
Refactors the foundation health component from a table-based layout to a carousel layout with filtering capabilities, displaying aggregate metrics across all foundations.
Changes
Technical Implementation
foundation-metrics.interface.tswith type definitionsJIRA
LFXV2-719
Generated with Claude Code