feat(ui): implement scroll shadow directive for dashboards #220
Conversation
This commit implements a reusable ScrollShadowDirective and applies it across multiple dashboard and module components for consistent scroll fade effects. Key Changes: - Create ScrollShadowDirective with horizontal and vertical scroll detection - Apply directive to dashboard components (recent-progress, foundation-health, organization-involvement) - Add scroll shadow overlays with Tailwind CSS gradients - Enhance committee, mailing-list, and meetings components - Remove custom scroll logic in favor of centralized directive - Fix accessibility modifiers and type safety issues Related tickets: - LFXV2-956: Implement scroll shadow directive - LFXV2-957: Apply scroll shadow directive to carousel components - LFXV2-958: Enhance committee dashboard and table components - LFXV2-959: Enhance mailing-list components - LFXV2-960: Enhance meetings dashboard and navigation 🤖 Generated with Claude Code (https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <[email protected]> Signed-off-by: Asitha de Silva <[email protected]>
WalkthroughAdds a standalone ScrollShadowDirective for scroll-aware shadow overlays and smooth scrolling; moves search/filter UI from several dashboards into their child table components via new input bindings; integrates meetings time filter into the top-bar; converts mailing-list creation to a stepper flow; removes a sidebar shadow utility. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
![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 (2)
apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.ts (2)
86-95: Critical: Category and voting status filters are broken.The
categoryFilterandvotingStatusFiltersignals are never updated when users change the corresponding dropdown values in the child component. The filtering logic at lines 230-243 relies on these signals, but there's no subscription to the form controlvalueChangesforcategoryandvotingStatus.Currently, only the search filter works because it has a
valueChangessubscription (line 144). The category and voting status filters will not function.🔎 Proposed fix: Subscribe to form control changes
Add subscriptions similar to
initializeSearchTerm:// Initialize search form this.searchForm = this.initializeSearchForm(); - this.categoryFilter = signal<string | null>(null); - this.votingStatusFilter = signal<string | null>(null); this.searchTerm = this.initializeSearchTerm(); + this.categoryFilter = this.initializeCategoryFilter(); + this.votingStatusFilter = this.initializeVotingStatusFilter();Then add these initialization methods after
initializeSearchTerm:private initializeCategoryFilter(): Signal<string | null> { return toSignal( this.searchForm.get('category')!.valueChanges.pipe( startWith(null), distinctUntilChanged() ), { initialValue: null } ); } private initializeVotingStatusFilter(): Signal<string | null> { return toSignal( this.searchForm.get('votingStatus')!.valueChanges.pipe( startWith(null), distinctUntilChanged() ), { initialValue: null } ); }
104-110: Remove orphaned event handler methods.The
onCategoryChangeandonVotingStatusChangemethods are no longer called since the filter UI moved to the child component. These methods should be removed to avoid confusion.🔎 Proposed cleanup
- public onCategoryChange(value: string | null): void { - this.categoryFilter.set(value); - } - - public onVotingStatusChange(value: string | null): void { - this.votingStatusFilter.set(value); - } - public onSearch(): void { // Trigger search through form control value changes }
🤖 Fix all issues with AI Agents
In
@apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts:
- Line 20: The timeFilterValue input is not propagated into the form control
(timeFilter), leaving the form hardcoded to 'upcoming'; add an effect (or
ngOnChanges) in the MeetingsTopBarComponent that watches timeFilterValue and
updates the FormControl (timeFilter.setValue or patchValue) after the form is
initialized in the constructor so the form reflects the parent-provided value,
guard against redundant updates by comparing current value before setting, and
apply the same sync pattern for the other inputs referenced around lines 36-40.
In @apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts:
- Around line 23-27: The directive attaches a scroll listener in ngAfterViewInit
but never removes it, causing a memory leak; implement OnDestroy, store the
listener callback (or use a bound method reference) when adding it to
this.el.nativeElement in ngAfterViewInit, and remove it in ngOnDestroy via
removeEventListener on the same element and same callback (or use
Renderer2.listen and call the returned cleanup function from ngOnDestroy);
ensure references to ngAfterViewInit, ngOnDestroy, updateScrollShadows, and
this.el.nativeElement (or container) are used to locate and fix the code.
🧹 Nitpick comments (11)
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html (2)
80-91: Unused template variable and inconsistent signal invocation.
let-activateCallback="activateCallback"is declared but never used in the template. Remove it to reduce noise.Line 84 passes
formValue(the signal reference) while line 83 passesform()(the signal's value). Iflfx-mailing-list-basic-infoexpects a value rather than a signal forformValue, this should be[formValue]="formValue()".🔎 Proposed fix
<p-step-panel [value]="1" data-testid="mailing-list-manage-panel-1"> - <ng-template #content let-activateCallback="activateCallback"> + <ng-template #content> <lfx-mailing-list-basic-info [form]="form()" - [formValue]="formValue" + [formValue]="formValue()" [service]="selectedService()" [prefix]="servicePrefix()" [maxGroupNameLength]="maxGroupNameLength()" [isEditMode]="isEditMode()"> </lfx-mailing-list-basic-info> </ng-template> </p-step-panel>Please verify whether
lfx-mailing-list-basic-infoexpects theformValueinput as a signal or as a plain value. If it expects a signal, the current code is correct; otherwise, invoke the signal withformValue().
93-107: Remove unused template variables for consistency.The
let-activateCallback="activateCallback"declarations in the remaining step panels (lines 94, 100) are also unused. Clean these up for consistency.🔎 Proposed fix
<p-step-panel [value]="2" data-testid="mailing-list-manage-panel-2"> - <ng-template #content let-activateCallback="activateCallback"> + <ng-template #content> <lfx-mailing-list-settings [form]="form()"></lfx-mailing-list-settings> </ng-template> </p-step-panel> <p-step-panel [value]="3" data-testid="mailing-list-manage-panel-3"> - <ng-template #content let-activateCallback="activateCallback"> + <ng-template #content> <!-- Step 3: People & Groups - Placeholder --> <div class="text-center text-gray-500 py-8"> <p>Step 3: People & Groups Component</p> <p class="text-sm mt-2">Coming next</p> </div> </ng-template> </p-step-panel>apps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.ts (1)
50-52: Consider extracting filter option type to a shared interface.The inline type
{ label: string; value: string | null }[]is used for bothcommitteeFilterOptionsandstatusFilterOptions. Per coding guidelines, prefer TypeScript interfaces for better maintainability and reusability.🔎 Suggested interface
Create a shared interface (e.g., in
@lfx-one/shared/interfaces):export interface FilterOption { label: string; value: string | null; }Then update the inputs:
- public committeeFilterOptions = input.required<{ label: string; value: string | null }[]>(); - public statusFilterOptions = input.required<{ label: string; value: string | null }[]>(); + public committeeFilterOptions = input.required<FilterOption[]>(); + public statusFilterOptions = input.required<FilterOption[]>();apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.html (1)
31-42: Update data-testid to follow naming convention.The data-testid should follow the
[section]-[component]-[element]pattern. Consider renamingtime-filter-dropdowntomeetings-topbar-timefilter-dropdownfor consistency with the hierarchical structure.As per coding guidelines for HTML templates.
🔎 Proposed fix
<lfx-select [form]="searchForm" control="timeFilter" size="small" [options]="timeFilterOptions()" placeholder="Select Time" styleClass="w-full" (onChange)="onTimeFilterChange($event.value)" - data-testid="time-filter-dropdown"></lfx-select> + data-testid="meetings-topbar-timefilter-dropdown"></lfx-select>apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts (2)
29-33: Replace computed signal with readonly property for static data.The
timeFilterOptionsusescomputed()but returns static data that never changes. Usingcomputed()for static data creates unnecessary reactivity overhead. Consider using a readonly property instead.🔎 Proposed fix
- // Initialize time filter options - this.timeFilterOptions = computed(() => [ - { label: 'Upcoming', value: 'upcoming' }, - { label: 'Past', value: 'past' }, - ]); + // Initialize time filter options + this.timeFilterOptions = [ + { label: 'Upcoming', value: 'upcoming' }, + { label: 'Past', value: 'past' }, + ];Then update the property declaration at line 26:
- public timeFilterOptions: Signal<{ label: string; value: 'upcoming' | 'past' }[]>; + public readonly timeFilterOptions: { label: string; value: 'upcoming' | 'past' }[];
50-58: Remove unnecessary truthy check.The
if (value)guard is unnecessary sincevalueis typed as'upcoming' | 'past', and both are truthy strings. This check adds no value and can be simplified.🔎 Proposed fix
// Subscribe to time filter changes this.searchForm .get('timeFilter') ?.valueChanges.pipe(takeUntilDestroyed()) .subscribe((value) => { - if (value) { - this.timeFilterChange.emit(value); - } + this.timeFilterChange.emit(value); });apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.ts (1)
42-42: Consider using a signal forsearchFormto align with Angular 19 best practices.Per the coding guideline to "use zoneless change detection with signals in Angular 19," the
searchFormproperty could be converted to a signal for consistency. However, this is a minor improvement since the current implementation works correctly with Angular's input signal conversion.apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html (1)
98-102: Unnecessary null-check for ViewChild in template.The template checks
scrollShadowDirective &&before accessing its properties, but the ViewChild is declared with the definite assignment assertion (!), indicating it will always be initialized. Since the directive is applied to line 51 and ViewChild queries are resolved after view initialization, the null-check is redundant in the conditional blocks.🔎 Simplify the conditional checks
<!-- Right shadow fade --> -@if (scrollShadowDirective && scrollShadowDirective.showRightShadow()) { +@if (scrollShadowDirective.showRightShadow()) { <div class="absolute top-0 bottom-0 right-0 w-32 z-10 pointer-events-none bg-[linear-gradient(to_right,transparent,#f9fafb)]" data-testid="dashboard-involvement-shadow-right"></div> } <!-- Left shadow fade --> -@if (scrollShadowDirective && scrollShadowDirective.showLeftShadow()) { +@if (scrollShadowDirective.showLeftShadow()) { <div class="absolute top-0 bottom-0 left-0 w-32 z-10 pointer-events-none bg-[linear-gradient(to_left,transparent,#f9fafb)]" data-testid="dashboard-involvement-shadow-left"></div> }Also applies to: 105-109
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html (1)
59-63: Unnecessary null-check for ViewChild in template.Similar to the organization-involvement component, the template includes redundant null-checks on
scrollShadowDirectivebefore accessing its properties. Since the ViewChild is declared with a definite assignment assertion and the directive is applied at line 42, the null-checks are unnecessary.🔎 Simplify the conditional checks
<!-- Right shadow fade --> -@if (scrollShadowDirective && scrollShadowDirective.showRightShadow()) { +@if (scrollShadowDirective.showRightShadow()) { <div class="absolute top-0 bottom-0 right-0 w-32 z-10 pointer-events-none bg-[linear-gradient(to_right,transparent,#f9fafb)]" data-testid="dashboard-recent-progress-shadow-right"></div> } <!-- Left shadow fade --> -@if (scrollShadowDirective && scrollShadowDirective.showLeftShadow()) { +@if (scrollShadowDirective.showLeftShadow()) { <div class="absolute top-0 bottom-0 left-0 w-32 z-10 pointer-events-none bg-[linear-gradient(to_left,transparent,#f9fafb)]" data-testid="dashboard-recent-progress-shadow-left"></div> }Also applies to: 66-70
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html (2)
181-181: Remove unnecessary null checks onscrollShadowDirective.The null checks
scrollShadowDirective &&are unnecessary because the component declares the ViewChild with a non-null assertion operator (!), guaranteeing the directive instance exists when these conditions are evaluated.🔎 Proposed simplification
<!-- Right shadow fade --> - @if (scrollShadowDirective && scrollShadowDirective.showRightShadow()) { + @if (scrollShadowDirective.showRightShadow()) { <div class="absolute top-0 bottom-0 right-0 w-32 z-10 pointer-events-none bg-[linear-gradient(to_right,transparent,#f9fafb)]" data-testid="foundation-health-shadow-right"></div> } <!-- Left shadow fade --> - @if (scrollShadowDirective && scrollShadowDirective.showLeftShadow()) { + @if (scrollShadowDirective.showLeftShadow()) { <div class="absolute top-0 bottom-0 left-0 w-32 z-10 pointer-events-none bg-[linear-gradient(to_left,transparent,#f9fafb)]" data-testid="foundation-health-shadow-left"></div> }Also applies to: 188-188
183-183: Consider using Tailwind color variables instead of hardcoded hex.The hardcoded color
#f9fafb(appears to be gray-50) reduces maintainability and consistency with the design system. Consider using CSS custom properties or Tailwind's color system for better theme consistency.As per coding guidelines, "Use Tailwind CSS with PrimeUI plugin and LFX colors for styling."
You could either:
- Define a CSS custom property in your theme (e.g.,
var(--surface-ground))- Use Tailwind's color in a CSS class if gradients with Tailwind colors are supported in your setup
Also applies to: 190-190
📜 Review details
Configuration used: Organization 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 (22)
apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.htmlapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/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.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts
🧰 Additional context used
📓 Path-based instructions (8)
apps/lfx-one/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.{ts,tsx}: Use zoneless change detection with signals in Angular 19
Always use direct imports for standalone components - no barrel exports
Use ESLint and Prettier for code quality, following Angular-specific ESLint rules
Files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/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/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts
apps/lfx-one/src/**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.ts: All PrimeNG components must be wrapped in LFX components for UI library independence
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.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/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/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/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/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}: License headers are required on all source files
Always prepend 'Generated with Claude Code (https://claude.ai/code)' if Claude Code assisted with the code
Files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.htmlapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/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/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Do not nest ternary expressions
Always use async/await for promises in TypeScript/JavaScript
Use camelCase for variable and function names
Files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/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/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts
apps/lfx-one/**/*.{ts,tsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier configuration with Tailwind integration in lfx-one app
Files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.htmlapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/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/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html
apps/lfx-one/src/**/*.component.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.html: Use data-testid naming convention: [section]-[component]-[element] for hierarchical structure
Use flex + flex-col + gap-* instead of space-y-* for spacing
Files:
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.htmlapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.htmlapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html
apps/lfx-one/src/**/*.{html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS with PrimeUI plugin and LFX colors for styling
Files:
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.htmlapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.htmlapps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.htmlapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.htmlapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.htmlapps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html
🧠 Learnings (1)
📚 Learning: 2025-12-10T23:20:29.281Z
Learnt from: asithade
Repo: linuxfoundation/lfx-v2-ui PR: 206
File: apps/lfx-one/src/app/modules/meetings/components/agenda-template-selector/agenda-template-selector.component.ts:11-15
Timestamp: 2025-12-10T23:20:29.281Z
Learning: For Angular 19+ projects, standalone defaults to true for components, directives, and pipes when not explicitly declared. Therefore, components with an imports array in the Component decorator do not require an explicit standalone: true declaration to be standalone. In reviews, verify that a component without an explicit standalone flag and with an imports array will behave as a standalone component according to Angular 19+ defaults. If you need to enforce explicit standalone behavior for clarity or consistency, you can still add standalone: true, but it is not required by default in Angular 19+. Ensure this guidance is considered when reviewing any component.ts files across the codebase.
Applied to files:
apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.tsapps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/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/mailing-lists/mailing-list-manage/mailing-list-manage.component.tsapps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.ts (1)
packages/shared/src/interfaces/mailing-list.interface.ts (1)
GroupsIOService(37-68)
⏰ 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 (24)
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.ts (2)
24-29: LGTM!The component decorator is correctly structured. The imports array includes all necessary modules for the stepper-based flow. Based on learnings, Angular 19+ defaults
standalone: truewhen an imports array is present, so explicit declaration is not required.
296-343: LGTM!The reactive service initialization chain is well-structured with proper fallback logic to the parent project and comprehensive error handling via
catchError.apps/lfx-one/src/app/modules/mailing-lists/mailing-list-manage/mailing-list-manage.component.html (2)
14-16: LGTM!The page title correctly displays "Add Mailing List" in create mode and "Edit Mailing List" in edit mode. The
data-testidattribute follows the naming convention.
18-76: LGTM!The stepper configuration and navigation controls are well-implemented. The
linearbinding correctly allows free navigation only in edit mode, and the conditional button rendering logic is appropriate for the multi-step flow.apps/lfx-one/src/app/modules/mailing-lists/components/mailing-list-table/mailing-list-table.component.html (1)
5-46: Well-structured filter bar implementation.The filter controls are properly implemented with:
- Responsive layout using
flex-coltomd:flex-row- Proper
gap-*spacing instead ofspace-y-*(per coding guidelines)- Correct
data-testidnaming convention following[section]-[component]-[element]patternapps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.html (2)
12-39: Well-implemented empty state for missing services.The "No Services" empty state follows the established patterns with proper layout using
flex + flex-col + gap-*. The contact support button provides a clear call-to-action.
93-96: No issues found. The HTML at line 95 correctly binds[statusFilterOptions]="statusOptions()"to the signal defined in the component's TypeScript file (line 57), and the signal is properly initialized at line 101.Likely an incorrect or invalid review comment.
apps/lfx-one/src/app/modules/mailing-lists/mailing-list-dashboard/mailing-list-dashboard.component.ts (2)
249-285: Service initialization logic is well-structured with proper fallback.The implementation correctly:
- Resets
servicesLoadedwhen project changes- Falls back to parent project services when none are found
- Sets
servicesLoadedto true on both success and error pathsHowever, the deeply nested
switchMapoperators increase cognitive complexity.
68-72: Clear signal naming for service availability state.The three signals (
servicesLoaded,availableServices,hasNoServices) provide a clean API for the template to handle different loading and empty states.apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts (1)
65-67: LGTM!The handler correctly emits the time filter change event.
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html (1)
23-34: LGTM!The time filter integration is implemented correctly. The bindings properly connect the parent's
timeFiltersignal with the child component's input/output, and the conditional meeting selection is accurate.apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.ts (2)
5-5: LGTM!The import changes correctly remove form-related dependencies that are no longer needed after moving time filter logic to the child component.
Also applies to: 22-22
91-93: LGTM!The
onTimeFilterChangehandler correctly updates the time filter signal, which triggers reactive data loading through the observable conversions at lines 98 and 153.apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.html (1)
5-46: Filter bar implementation looks good.The new filter bar correctly uses:
flex + flex-col + gap-*spacing utilities (per coding guidelines)- Proper
data-testidattributes for test targeting- Correct form binding syntax with
searchForm()calling the input signal- Responsive layout with
md:flex-rowbreakpointThe control names (
search,category,votingStatus) match the parent component's FormGroup definition.apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.ts (1)
23-23: Imports cleanup looks correct.The removal of
ReactiveFormsModule,InputTextComponent, andSelectComponentis appropriate since these are now only used in the childCommitteeTableComponent. The parent component still correctly importsFormControlandFormGrouptypes needed for its form logic.apps/lfx-one/src/app/modules/committees/components/committee-table/committee-table.component.ts (1)
6-6: Form-related inputs and imports are correctly implemented.The component properly:
- Imports
ReactiveFormsModuleandFormGroupfor form binding- Adds
InputTextComponentandSelectComponentfor the filter controls- Declares three required inputs with appropriate types (
FormGroupand option arrays)- Ensures parent components must provide these inputs via
input.required()The input signal pattern aligns with Angular 19 best practices, and the component is correctly standalone by default (per project learnings).
Also applies to: 10-11, 29-29, 35-36, 57-59
apps/lfx-one/src/app/modules/committees/committee-dashboard/committee-dashboard.component.html (1)
65-67: Input bindings to child component are syntactically correct.The template correctly passes:
searchFormas aFormGroupinstance (Angular handles input signal conversion)categories()andvotingStatusOptions()by calling the signal functionsThe bindings satisfy the child component's required inputs.
Note: The functionality of the category and voting status filters depends on the parent component subscribing to form control value changes, which is flagged as a critical issue in the TypeScript file review.
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html (1)
23-36: LGTM! Clean directive integration.The template correctly integrates the ScrollShadowDirective with proper accessibility labels on carousel control buttons, consistent data-testid naming, and correct application of the directive with Tailwind flex/gap spacing patterns.
Also applies to: 50-51
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html (1)
4-9: LGTM! Simplified sidebar styling.Removing the custom shadow utility while retaining the border creates a cleaner visual separation. This aligns with the PR's focus on directive-based scroll shadows for specific carousel components.
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (2)
36-36: LGTM! Type-safe ViewChild declaration.The ViewChild correctly uses the directive type as the selector, providing better type safety compared to string-based selectors. The
publicmodifier appropriately exposes the directive for template access, and the definite assignment assertion is valid since the directive is applied in the template.
4-4: Clean directive integration with proper imports.The imports are correctly structured with the ScrollShadowDirective added to both the component's imports array and ES6 imports. This follows Angular 19+ standalone component patterns appropriately.
Based on learnings, the component defaults to standalone in Angular 19+, so the explicit standalone declaration is not required.
Also applies to: 10-10, 31-31
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html (1)
23-36: Consistent directive implementation across carousel components.The recent-progress component follows the same directive-based scroll pattern as organization-involvement, ensuring consistency across dashboard carousels. The template correctly applies the directive and wires up carousel controls.
Also applies to: 41-42
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (1)
4-4: Consistent directive integration pattern.The foundation-health component follows the same clean refactoring pattern as the other dashboard components, replacing ElementRef-based scrolling with the type-safe ScrollShadowDirective. The imports, component metadata, and ViewChild declaration are all properly structured.
Also applies to: 9-9, 27-27, 32-32
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)
4-4: LGTM: Clean integration with ScrollShadowDirective.The component correctly:
- Imports the necessary types (
ViewChildandScrollShadowDirective)- Adds the directive to the component's imports array
- Updates the ViewChild binding from ElementRef to ScrollShadowDirective
- Uses the
protectedaccess modifier for template access- Applies non-null assertion appropriately
This follows the established pattern across the PR for directive-based scroll shadow management.
Also applies to: 21-21, 39-39, 44-44
...odules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR implements a reusable ScrollShadowDirective to provide consistent scroll shadow effects across dashboard and module components, eliminating code duplication. The directive detects horizontal and vertical scroll positions and exposes signals to control shadow visibility.
Key Changes:
- Introduced
ScrollShadowDirectivewith horizontal/vertical scroll detection and configurable scroll distance - Refactored Recent Progress, Foundation Health, and Organization Involvement dashboard components to use the directive
- Consolidated filter bars into table components for Committee and Mailing List modules
- Moved time filter from separate section into top bar for Meetings dashboard
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| scroll-shadow.directive.ts | New directive providing scroll shadow detection and visibility management |
| sidebar.component.html | Removed shadow styling from sidebar border |
| recent-progress.component.* | Integrated scroll shadow directive, removed custom scroll methods |
| organization-involvement.component.* | Applied scroll shadow directive to carousel |
| foundation-health.component.* | Applied scroll shadow directive with custom scroll distance |
| meetings-dashboard.component.* | Consolidated time filter into top bar, removed form logic |
| meetings-top-bar.component.* | Added time filter control and event handling |
| mailing-list-manage.component.* | Removed unused service check logic and skip functionality, updated button label |
| mailing-list-dashboard.component.* | Added service availability check with empty state |
| mailing-list-table.component.* | Moved filter bar into table component |
| committee-dashboard.component.* | Moved filter bar from dashboard to table component |
| committee-table.component.* | Added filter bar inside table card |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...dules/dashboards/components/organization-involvement/organization-involvement.component.html
Outdated
Show resolved
Hide resolved
...one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts
Outdated
Show resolved
Hide resolved
...dules/dashboards/components/organization-involvement/organization-involvement.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
Show resolved
Hide resolved
...modules/dashboards/components/organization-involvement/organization-involvement.component.ts
Show resolved
Hide resolved
...x-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
Show resolved
Hide resolved
...lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
Outdated
Show resolved
Hide resolved
...lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
Outdated
Show resolved
Hide resolved
- Fix memory leak by properly cleaning up scroll event listener - Make directive SSR-safe using afterNextRender - Fix ExpressionChangedAfterItHasBeenCheckedError with setTimeout - Add aria-hidden to decorative shadow overlays for accessibility - Remove unused timeFilterValue input from meetings-top-bar component 🤖 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: 3
🤖 Fix all issues with AI Agents
In
@apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts:
- Line 22: Add an optional input to allow parent initialization: declare
timeFilterValue = input<'upcoming' | 'past'>() and then initialize the form
control using that input instead of hardcoding 'upcoming' (i.e., construct the
control with new FormControl<'upcoming' | 'past'>(this.timeFilterValue() ??
'upcoming')). Keep the existing public readonly timeFilterChange =
output<'upcoming' | 'past'>() and ensure the component emits via
onTimeFilterChange as before so parent and child stay in sync.
- Around line 64-66: The onTimeFilterChange handler currently emits the new
value without updating the reactive form control, causing potential
desynchronization; update the searchForm's timeFilter FormControl (e.g., call
setValue or patchValue on searchForm.get('timeFilter')) before calling
this.timeFilterChange.emit(value), and apply the same change to
onMeetingTypeChange by updating the meetingType FormControl on the searchForm
before emitting via this.meetingTypeChange.emit(value).
In @apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts:
- Around line 24-34: The constructor still uses setTimeout before calling
updateScrollShadows which reintroduces the
ExpressionChangedAfterItHasBeenCheckedError; remove the setTimeout and invoke
this.updateScrollShadows() directly inside the afterNextRender callback after
adding the scroll listener (in the public constructor where afterNextRender,
this.el, container, this.scrollHandler, and this.destroyRef are used) so the
initial shadow state is updated after the first render without extra microtask
scheduling.
🧹 Nitpick comments (2)
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts (1)
46-46: Consider throttling scroll updates for performance.Scroll events can fire at 60+ FPS. While likely acceptable for this use case, wrapping the handler in
requestAnimationFramewould batch updates to the render cycle and reduce redundant signal updates.🔎 Optional optimization using requestAnimationFrame
+ private rafId: number | null = null; + - private scrollHandler = (): void => this.updateScrollShadows(); + private scrollHandler = (): void => { + if (this.rafId !== null) return; + this.rafId = requestAnimationFrame(() => { + this.updateScrollShadows(); + this.rafId = null; + }); + };If using this, also cancel any pending frame in the destroy callback:
this.destroyRef.onDestroy(() => { container.removeEventListener('scroll', this.scrollHandler); if (this.rafId !== null) cancelAnimationFrame(this.rafId); });apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts (1)
28-32: Consider making timeFilterOptions a readonly property initialized at declaration.Since
timeFilterOptionscontains static data that never changes, initializing it in the constructor adds unnecessary overhead. Moving it to a readonly property initialized at declaration would be cleaner and more efficient.🔎 Proposed refactor
- public timeFilterOptions: { label: string; value: 'upcoming' | 'past' }[]; + public readonly timeFilterOptions = [ + { label: 'Upcoming', value: 'upcoming' as const }, + { label: 'Past', value: 'past' as const }, + ]; public constructor() { - // Initialize time filter options - this.timeFilterOptions = [ - { label: 'Upcoming', value: 'upcoming' }, - { label: 'Past', value: 'past' }, - ]; - // Initialize form
📜 Review details
Configuration used: Organization 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 (7)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.htmlapps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.html
- apps/lfx-one/src/app/modules/meetings/meetings-dashboard/meetings-dashboard.component.html
🧰 Additional context used
📓 Path-based instructions (8)
apps/lfx-one/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.{ts,tsx}: Use zoneless change detection with signals in Angular 19
Always use direct imports for standalone components - no barrel exports
Use ESLint and Prettier for code quality, following Angular-specific ESLint rules
Files:
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx,js,html,scss,css}: License headers are required on all source files
Always prepend 'Generated with Claude Code (https://claude.ai/code)' if Claude Code assisted with the code
Files:
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Do not nest ternary expressions
Always use async/await for promises in TypeScript/JavaScript
Use camelCase for variable and function names
Files:
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts
apps/lfx-one/**/*.{ts,tsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier configuration with Tailwind integration in lfx-one app
Files:
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
apps/lfx-one/src/**/*.component.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.html: Use data-testid naming convention: [section]-[component]-[element] for hierarchical structure
Use flex + flex-col + gap-* instead of space-y-* for spacing
Files:
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
apps/lfx-one/src/**/*.{html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS with PrimeUI plugin and LFX colors for styling
Files:
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
apps/lfx-one/src/**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.component.ts: All PrimeNG components must be wrapped in LFX components for UI library independence
Add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts
🧠 Learnings (1)
📚 Learning: 2025-12-10T23:20:29.281Z
Learnt from: asithade
Repo: linuxfoundation/lfx-v2-ui PR: 206
File: apps/lfx-one/src/app/modules/meetings/components/agenda-template-selector/agenda-template-selector.component.ts:11-15
Timestamp: 2025-12-10T23:20:29.281Z
Learning: For Angular 19+ projects, standalone defaults to true for components, directives, and pipes when not explicitly declared. Therefore, components with an imports array in the Component decorator do not require an explicit standalone: true declaration to be standalone. In reviews, verify that a component without an explicit standalone flag and with an imports array will behave as a standalone component according to Angular 19+ defaults. If you need to enforce explicit standalone behavior for clarity or consistency, you can still add standalone: true, but it is not required by default in Angular 19+. Ensure this guidance is considered when reviewing any component.ts files across the codebase.
Applied to files:
apps/lfx-one/src/app/modules/meetings/meetings-dashboard/components/meetings-top-bar/meetings-top-bar.component.ts
🔇 Additional comments (9)
apps/lfx-one/src/app/shared/directives/scroll-shadow.directive.ts (3)
1-9: LGTM: Proper license header and directive setup.The standalone directive with the
[lfxScrollShadow]selector follows Angular conventions.
36-44: LGTM: Clean scroll navigation implementation.The
scrollLeft()andscrollRight()methods correctly usescrollBywith smooth behavior and respect the configurablescrollDistanceinput.
48-80: LGTM: Robust scroll position detection.The shadow calculation logic correctly handles:
- Sub-pixel rounding with
-1tolerance- Non-scrollable containers (signals set to
false)- Both horizontal and vertical scroll axes
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html (2)
23-31: LGTM: Carousel controls correctly delegate to directive.The scroll buttons properly invoke the directive's
scrollLeft()andscrollRight()methods.
96-111: LGTM: Shadow overlays are accessible and well-styled.Good implementation:
aria-hidden="true"correctly marks decorative elements for screen readers- Tailwind gradient with
#f9fafbmatches the specified background colorpointer-events-noneensures shadows don't block interactionsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html (2)
41-56: LGTM: Directive integration follows established pattern.The carousel container correctly applies the
lfxScrollShadowdirective with appropriate Tailwind classes for horizontal scrolling (overflow-x-auto,scroll-smooth,hide-scrollbar).
57-72: LGTM: Shadow overlays are properly implemented.Consistent with the organization-involvement component:
aria-hidden="true"for accessibility- Correct gradient directions for left/right fades
- Proper
data-testidnaming conventionapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html (2)
55-56: LGTM: Good use of configurable scroll distance.The
[scrollDistance]="320"input appropriately customizes the scroll step for the foundation health cards, which appear wider than the default 300px.
179-194: LGTM: Shadow overlays follow the consistent pattern.Implementation matches other dashboard components with:
aria-hidden="true"for accessibility- Correct gradient colors (
#f9fafb) per coding guidelines- Proper
data-testidnaming (foundation-health-shadow-*)
Summary
Implements a reusable
ScrollShadowDirectiveand applies it across multiple dashboard and module components for consistent scroll fade effects. This eliminates code duplication and provides a unified pattern for scroll detection and visual feedback.Changes
New Features
Components Updated
Dashboard Components:
Module Components:
Technical Improvements
publickeywords)Related Tickets
Testing