From ed8e7b4cda3a2f6651e077d031b11684bd824dc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Mart=C3=ADn?= Date: Tue, 7 Oct 2025 00:05:46 +0200 Subject: [PATCH] Apply suggested fixes. Mostly cosmetic changes --- CHANGELOG.md | 5 +- .../src/lib/assistant/assistant.component.ts | 36 ++--- .../src/lib/portfolio-filter-form/README.md | 130 ------------------ .../portfolio-filter-form.component.html | 2 +- .../portfolio-filter-form.component.scss | 6 +- ...portfolio-filter-form.component.stories.ts | 16 +-- .../portfolio-filter-form.component.ts | 107 +++++++------- 7 files changed, 77 insertions(+), 225 deletions(-) delete mode 100644 libs/ui/src/lib/portfolio-filter-form/README.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 1501d039c..e07033475 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added support for a date range query parameter in the data gathering endpoint - Added a _Storybook_ story for the activities table component +### Changed + +- Extracted Portfolio Fliter Form as a reusable component + ## 2.206.0 - 2025-10-04 ### Changed @@ -66,7 +70,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Removed the deprecated `ITEM` activity type -- Refactored Portfoli Fliter Form as a reusable component ## 2.202.0 - 2025-09-26 diff --git a/libs/ui/src/lib/assistant/assistant.component.ts b/libs/ui/src/lib/assistant/assistant.component.ts index 461b7a0d7..a75c8130c 100644 --- a/libs/ui/src/lib/assistant/assistant.component.ts +++ b/libs/ui/src/lib/assistant/assistant.component.ts @@ -119,12 +119,12 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { } } + @Input() accountsWithValue: AccountWithValue[] = []; @Input() deviceType: string; @Input() hasPermissionToAccessAdminControl: boolean; @Input() hasPermissionToChangeDateRange: boolean; @Input() hasPermissionToChangeFilters: boolean; @Input() user: User; - @Input() accountsWithValue: AccountWithValue[] = []; @Output() closed = new EventEmitter(); @Output() dateRangeChanged = new EventEmitter(); @@ -142,14 +142,6 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { public assetClasses: Filter[] = []; public dateRangeFormControl = new FormControl(undefined); public dateRangeOptions: IDateRangeOption[] = []; - public portfolioFilterFormControl = new FormControl( - { - account: null, - assetClass: null, - holding: null, - tag: null - } - ); public holdings: PortfolioPosition[] = []; public isLoading = { accounts: false, @@ -159,6 +151,14 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { }; public isOpen = false; public placeholder = $localize`Find account, holding or page...`; + public portfolioFilterFormControl = new FormControl( + { + account: null, + assetClass: null, + holding: null, + tag: null + } + ); public searchFormControl = new FormControl(''); public searchResults: ISearchResults = { accounts: [], @@ -360,15 +360,6 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { quickLinks: [] }; this.changeDetectorRef.markForCheck(); - }, - complete: () => { - this.isLoading = { - accounts: false, - assetProfiles: false, - holdings: false, - quickLinks: false - }; - this.changeDetectorRef.markForCheck(); } }); } @@ -398,7 +389,6 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { })) as AccountWithValue[]; } - // Handle portfolio filter form disabled state if (this.hasPermissionToChangeFilters) { this.portfolioFilterFormControl.enable({ emitEvent: false }); } else { @@ -486,12 +476,6 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { }) ?? []; } - public hasFilter(aFormValue: { [key: string]: string }) { - return Object.values(aFormValue).some((value) => { - return !!value; - }); - } - public initialize() { this.isLoading = { accounts: true, @@ -535,6 +519,7 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { .sort((a, b) => { return a.name?.localeCompare(b.name); }); + this.setPortfolioFilterFormValues(); this.changeDetectorRef.markForCheck(); @@ -574,6 +559,7 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { } public onCloseAssistant() { + this.portfolioFilterFormControl.reset(); this.setIsOpen(false); this.closed.emit(); diff --git a/libs/ui/src/lib/portfolio-filter-form/README.md b/libs/ui/src/lib/portfolio-filter-form/README.md deleted file mode 100644 index 5ee151441..000000000 --- a/libs/ui/src/lib/portfolio-filter-form/README.md +++ /dev/null @@ -1,130 +0,0 @@ -# Portfolio Filter Form Component - -## Overview - -The `GfPortfolioFilterFormComponent` is a reusable Angular component that provides a form interface for filtering portfolio data. It implements `ControlValueAccessor` to work seamlessly with Angular reactive forms. - -## Features - -- **Account filtering**: Select specific accounts to filter by -- **Asset class filtering**: Filter by asset classes (Equity, Fixed Income, etc.) -- **Holding filtering**: Filter by specific holdings/securities -- **Tag filtering**: Filter by user-defined tags -- **Form validation**: Built-in validation and state management -- **Accessibility**: Full support for Angular forms and accessibility features - -## Usage - -### Basic Implementation - -```typescript -import { GfPortfolioFilterFormComponent } from '@ghostfolio/ui/portfolio-filter-form'; - -@Component({ - selector: 'my-component', - template: ` - - - ` -}) -export class MyComponent { - portfolioFiltersControl = new FormControl({ - account: null, - assetClass: null, - holding: null, - tag: null - }); - - // ... other properties -} -``` - -### With Reactive Forms - -```typescript -import { PortfolioFilterFormValue } from '@ghostfolio/ui/portfolio-filter-form'; - -import { FormControl } from '@angular/forms'; - -const filterControl = new FormControl({ - account: null, - assetClass: null, - holding: null, - tag: null -}); - -// Subscribe to changes -filterControl.valueChanges.subscribe((filters) => { - console.log('Filter changes:', filters); -}); -``` - -## Inputs - -| Input | Type | Description | -| -------------- | --------------------- | ----------------------------------- | -| `accounts` | `Account[]` | Array of available accounts | -| `assetClasses` | `Filter[]` | Array of available asset classes | -| `holdings` | `PortfolioPosition[]` | Array of available holdings | -| `tags` | `Filter[]` | Array of available tags | -| `disabled` | `boolean` | Whether the form should be disabled | - -## Outputs - -| Output | Type | Description | -| -------------- | -------------------- | -------------------------------------------- | -| `applyFilters` | `EventEmitter` | Emitted when Apply Filters button is clicked | -| `resetFilters` | `EventEmitter` | Emitted when Reset Filters button is clicked | - -## Interface - -### PortfolioFilterFormValue - -```typescript -interface PortfolioFilterFormValue { - account: string | null; - assetClass: string | null; - holding: PortfolioPosition | null; - tag: string | null; -} -``` - -## Implementation Details - -- Implements `ControlValueAccessor` for seamless integration with Angular forms -- Uses Angular Material components for consistent UI -- Handles form state management internally -- Provides validation and dirty state tracking -- Supports disabled state management - -## Testing - -The component includes comprehensive unit tests covering: - -- Component creation and initialization -- Form value management -- Event emission -- Filter detection logic -- ControlValueAccessor implementation - -Run tests with: - -```bash -nx test ui -``` - -## Storybook - -Interactive component documentation and examples are available in Storybook: - -```bash -nx run ui:storybook -``` diff --git a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.html b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.html index 0fc9ec214..8c9e0a5c0 100644 --- a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.html +++ b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.html @@ -1,4 +1,4 @@ -
+
Account diff --git a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.scss b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.scss index 2baad569d..5d4e87f30 100644 --- a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.scss +++ b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.scss @@ -1,5 +1,3 @@ -.gf-portfolio-filter-form { - .gf-spacer { - flex: 1 1 auto; - } +:host { + display: block; } diff --git a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.stories.ts b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.stories.ts index 7869ea632..4cce1e17f 100644 --- a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.stories.ts +++ b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.stories.ts @@ -42,21 +42,21 @@ export const Default: Story = { ] as any, holdings: [ { - name: 'Apple Inc.', - symbol: 'AAPL', currency: 'USD', - dataSource: 'YAHOO' + dataSource: 'YAHOO', + name: 'Apple Inc.', + symbol: 'AAPL' }, { - name: 'Microsoft Corporation', - symbol: 'MSFT', currency: 'USD', - dataSource: 'YAHOO' + dataSource: 'YAHOO', + name: 'Microsoft Corporation', + symbol: 'MSFT' } ] as any, tags: [ - { id: 'tech', label: 'Technology', type: 'TAG' }, - { id: 'dividend', label: 'Dividend', type: 'TAG' } + { id: 'dividend', label: 'Dividend', type: 'TAG' }, + { id: 'tech', label: 'Technology', type: 'TAG' } ] as any, disabled: false } diff --git a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.ts b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.ts index 9e2f5229f..ccd3431ee 100644 --- a/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.ts +++ b/libs/ui/src/lib/portfolio-filter-form/portfolio-filter-form.component.ts @@ -46,9 +46,9 @@ import { PortfolioFilterFormValue } from './interfaces'; ], providers: [ { + multi: true, provide: NG_VALUE_ACCESSOR, - useExisting: forwardRef(() => GfPortfolioFilterFormComponent), - multi: true + useExisting: forwardRef(() => GfPortfolioFilterFormComponent) } ], schemas: [CUSTOM_ELEMENTS_SCHEMA], @@ -70,19 +70,17 @@ export class GfPortfolioFilterFormComponent public filterForm: FormGroup; - private onChange: (value: PortfolioFilterFormValue) => void = () => { - // ControlValueAccessor callback - implemented by parent - }; - private onTouched: () => void = () => { - // ControlValueAccessor callback - implemented by parent - }; + // eslint-disable-next-line @typescript-eslint/no-empty-function + private onChange: (value: PortfolioFilterFormValue) => void = () => {}; + + // eslint-disable-next-line @typescript-eslint/no-empty-function + private onTouched: () => void = () => {}; private unsubscribeSubject = new Subject(); public constructor( private changeDetectorRef: ChangeDetectorRef, private formBuilder: FormBuilder ) { - // Create form with initial state (will be updated in ngOnChanges) this.filterForm = this.formBuilder.group({ account: new FormControl(null), assetClass: new FormControl(null), @@ -92,7 +90,6 @@ export class GfPortfolioFilterFormComponent } public ngOnInit() { - // Subscribe to form changes to notify parent component this.filterForm.valueChanges .pipe(takeUntil(this.unsubscribeSubject)) .subscribe((value) => { @@ -101,15 +98,34 @@ export class GfPortfolioFilterFormComponent }); } + public hasFilters(): boolean { + const formValue = this.filterForm.value; + + return Object.values(formValue).some((value) => { + return !!value; + }); + } + + public holdingComparisonFunction( + option: PortfolioPosition, + value: PortfolioPosition + ): boolean { + if (value === null) { + return false; + } + + return ( + getAssetProfileIdentifier(option) === getAssetProfileIdentifier(value) + ); + } + public ngOnChanges() { - // Update form disabled state if (this.disabled) { this.filterForm.disable({ emitEvent: false }); } else { this.filterForm.enable({ emitEvent: false }); } - // Disable tag field if no tags available if (this.tags.length === 0) { this.filterForm.get('tag')?.disable({ emitEvent: false }); } @@ -117,26 +133,15 @@ export class GfPortfolioFilterFormComponent this.changeDetectorRef.markForCheck(); } - public ngOnDestroy() { - this.unsubscribeSubject.next(); - this.unsubscribeSubject.complete(); + public onApplyFilters(): void { + this.filterForm.markAsPristine(); + this.onChange(this.filterForm.value as PortfolioFilterFormValue); + this.applyFilters.emit(); } - // ControlValueAccessor implementation - public writeValue(value: PortfolioFilterFormValue | null): void { - if (value) { - this.filterForm.setValue( - { - account: value.account ?? null, - assetClass: value.assetClass ?? null, - holding: value.holding ?? null, - tag: value.tag ?? null - }, - { emitEvent: false } - ); - } else { - this.filterForm.reset({}, { emitEvent: false }); - } + public onResetFilters(): void { + this.filterForm.reset({}, { emitEvent: true }); + this.resetFilters.emit(); } public registerOnChange(fn: (value: PortfolioFilterFormValue) => void): void { @@ -150,7 +155,6 @@ export class GfPortfolioFilterFormComponent public setDisabledState(isDisabled: boolean): void { this.disabled = isDisabled; - // Update form disabled state manually since this is called by ControlValueAccessor if (this.disabled) { this.filterForm.disable({ emitEvent: false }); } else { @@ -160,33 +164,24 @@ export class GfPortfolioFilterFormComponent this.changeDetectorRef.markForCheck(); } - // Helper methods - public hasFilters(): boolean { - const formValue = this.filterForm.value; - return Object.values(formValue).some((value) => !!value); - } - - public holdingComparisonFunction( - option: PortfolioPosition, - value: PortfolioPosition - ): boolean { - if (value === null) { - return false; + public writeValue(value: PortfolioFilterFormValue | null): void { + if (value) { + this.filterForm.setValue( + { + account: value.account ?? null, + assetClass: value.assetClass ?? null, + holding: value.holding ?? null, + tag: value.tag ?? null + }, + { emitEvent: false } + ); + } else { + this.filterForm.reset({}, { emitEvent: false }); } - - return ( - getAssetProfileIdentifier(option) === getAssetProfileIdentifier(value) - ); - } - - public onApplyFilters(): void { - this.filterForm.markAsPristine(); - this.onChange(this.filterForm.value as PortfolioFilterFormValue); - this.applyFilters.emit(); } - public onResetFilters(): void { - this.filterForm.reset({}, { emitEvent: true }); - this.resetFilters.emit(); + public ngOnDestroy() { + this.unsubscribeSubject.next(); + this.unsubscribeSubject.complete(); } }