From 0d20d2f07e03e17daaed17a1b2bc51682c2998d4 Mon Sep 17 00:00:00 2001 From: Fahri Gedik <53567152+fahrigedik@users.noreply.github.com> Date: Thu, 5 Feb 2026 16:58:28 +0300 Subject: [PATCH] Refactor chart, table, directives, and permissions Multiple fixes and refactors across components and directives: - chart.component.ts: Make initChart accept data and options, pass them when initializing/reinitializing, and avoid unnecessary early returns in effects. - extensible-form-prop.component.ts: Guard against missing data, reuse a local data variable, pass record to injector providers, and tighten autofocus condition checks. - extensible-table.component.ts: Extract record preparation into prepareRecord to simplify data mapping logic. - tree.component.ts: Cache checked keys once when toggling node selection to avoid repeated calls. - autofocus.directive.ts: Remove deprecated numberAttribute and use a transform that safely converts input to Number. - permission-management.component.ts: Cache providerName/providerKey, validate them before fetching, and use cached values in filters and checks. - visible.directive.ts: Avoid redundant condition updates by tracking the last input value. These changes improve safety (null checks), readability (helper extraction), and performance (reduced repeated computations). --- .../chart.js/src/chart.component.ts | 19 +++--- .../extensible-form-prop.component.ts | 15 +++-- .../extensible-table.component.ts | 66 +++++++++---------- .../tree/src/lib/components/tree.component.ts | 5 +- .../src/lib/directives/autofocus.directive.ts | 7 +- .../permission-management.component.ts | 21 ++++-- .../src/lib/directives/visible.directive.ts | 4 ++ 7 files changed, 77 insertions(+), 60 deletions(-) diff --git a/npm/ng-packs/packages/components/chart.js/src/chart.component.ts b/npm/ng-packs/packages/components/chart.js/src/chart.component.ts index d4e697b48d..2948aaa176 100644 --- a/npm/ng-packs/packages/components/chart.js/src/chart.component.ts +++ b/npm/ng-packs/packages/components/chart.js/src/chart.component.ts @@ -60,10 +60,9 @@ export class ChartComponent implements AfterViewInit, OnDestroy { const options = this.options(); untracked(() => { - if (this.chart) { - this.chart.destroy(); - this.initChart(); - } + if (!this.chart) return; + this.chart.destroy(); + this.initChart(data, options); }); }); } @@ -71,7 +70,7 @@ export class ChartComponent implements AfterViewInit, OnDestroy { ngAfterViewInit() { import('chart.js/auto').then(module => { Chart = module.default; - this.initChart(); + this.initChart(this.data(), this.options()); this.initialized.emit(true); }); } @@ -97,8 +96,8 @@ export class ChartComponent implements AfterViewInit, OnDestroy { } } - private initChart = () => { - const opts = this.options() || {}; + private initChart = (data: any, options: any) => { + const opts = options || {}; opts.responsive = this.responsive(); // allows chart to resize in responsive mode @@ -108,8 +107,8 @@ export class ChartComponent implements AfterViewInit, OnDestroy { this.chart = new Chart(this.canvas.nativeElement, { type: this.type() as any, - data: this.data(), - options: this.options(), + data: data, + options: opts, plugins: this.plugins(), }); }; @@ -138,7 +137,7 @@ export class ChartComponent implements AfterViewInit, OnDestroy { reinit = () => { if (!this.chart) return; this.chart.destroy(); - this.initChart(); + this.initChart(this.data(), this.options()); }; ngOnDestroy() { diff --git a/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-form/extensible-form-prop.component.ts b/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-form/extensible-form-prop.component.ts index fd40dd4c8e..13fca04c73 100644 --- a/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-form/extensible-form-prop.component.ts +++ b/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-form/extensible-form-prop.component.ts @@ -110,13 +110,16 @@ export class ExtensibleFormPropComponent implements AfterViewInit { disabledFn = (data: ReadonlyPropData) => false; get disabled() { - return this.disabledFn(this.data()?.data); + const data = this.data()?.data; + if (!data) return false; + return this.disabledFn(data); } constructor() { // Watch prop changes and update state effect(() => { const currentProp = this.prop(); + const data = this.data()?.data; if (!currentProp) return; const { options, readonly, disabled, validators, className, template } = currentProp; @@ -130,7 +133,7 @@ export class ExtensibleFormPropComponent implements AfterViewInit { }, { provide: EXTENSIONS_FORM_PROP_DATA, - useValue: this.data()?.data?.record, + useValue: data?.record, }, { provide: ControlContainer, useExisting: FormGroupDirective }, ], @@ -138,14 +141,14 @@ export class ExtensibleFormPropComponent implements AfterViewInit { }); } - if (options) this.options$ = options(this.data().data); - if (readonly) this.readonly = readonly(this.data().data); + if (options) this.options$ = options(data); + if (readonly) this.readonly = readonly(data); if (disabled) { this.disabledFn = disabled; } if (validators) { - this.validators = validators(this.data().data); + this.validators = validators(data); this.setAsterisk(); } if (className !== undefined) { @@ -200,7 +203,7 @@ export class ExtensibleFormPropComponent implements AfterViewInit { } ngAfterViewInit() { - if (this.isFirstGroup() && this.first() && this.fieldRef) { + if (!!this.isFirstGroup() && !!this.first() && this.fieldRef) { requestAnimationFrame(() => { this.fieldRef.nativeElement.focus(); }); diff --git a/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-table/extensible-table.component.ts b/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-table/extensible-table.component.ts index d4149f43e4..17f1b1bd20 100644 --- a/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-table/extensible-table.component.ts +++ b/npm/ng-packs/packages/components/extensible/src/lib/components/extensible-table/extensible-table.component.ts @@ -108,7 +108,7 @@ export class ExtensibleTableComponent implements AfterViewInit, OnDestr }); readonly actionsTemplate = input | undefined>(undefined); readonly selectable = input(false); - readonly selectionTypeInput = input(SelectionType.multiClick, { + readonly selectionTypeInput = input(SelectionType.multiClick, { alias: 'selectionType', }); readonly selected = input([]); @@ -205,41 +205,41 @@ export class ExtensibleTableComponent implements AfterViewInit, OnDestr this.list().totalCount = this.recordsTotal(); } - this._data.set( - dataValue.map((record: any, index: number) => { - this.propList.forEach(prop => { - const propData = { getInjected: this.getInjected, record, index } as ReadonlyPropData; - const value = this.getContent(prop.value, propData); - - const propKey = `_${prop.value.name}`; - record[propKey] = { - visible: prop.value.visible(propData), - value, - }; - if (prop.value.component) { - record[propKey].injector = Injector.create({ - providers: [ - { - provide: PROP_DATA_STREAM, - useValue: value, - }, - { - provide: ROW_RECORD, - useValue: record, - }, - ], - parent: this.#injector, - }); - record[propKey].component = prop.value.component; - } - }); - - return record; - }), - ); + this._data.set(dataValue.map((record, index) => this.prepareRecord(record, index))); }); } + private prepareRecord(record: any, index: number): any { + this.propList.forEach(prop => { + const propData = { getInjected: this.getInjected, record, index } as ReadonlyPropData; + const value = this.getContent(prop.value, propData); + + const propKey = `_${prop.value.name}`; + record[propKey] = { + visible: prop.value.visible(propData), + value, + }; + if (prop.value.component) { + record[propKey].injector = Injector.create({ + providers: [ + { + provide: PROP_DATA_STREAM, + useValue: value, + }, + { + provide: ROW_RECORD, + useValue: record, + }, + ], + parent: this.#injector, + }); + record[propKey].component = prop.value.component; + } + }); + + return record; + } + private getIcon(value: boolean) { return value ? '
' diff --git a/npm/ng-packs/packages/components/tree/src/lib/components/tree.component.ts b/npm/ng-packs/packages/components/tree/src/lib/components/tree.component.ts index f8c624ec9d..e09c03f82d 100644 --- a/npm/ng-packs/packages/components/tree/src/lib/components/tree.component.ts +++ b/npm/ng-packs/packages/components/tree/src/lib/components/tree.component.ts @@ -150,11 +150,12 @@ export class TreeComponent implements OnInit { onSelectedNodeChange(node: NzTreeNode) { this._selectedNode.set(node.origin.entity); if (this.changeCheckboxWithNode()) { + const keys = this._checkedKeys(); let newVal; if (node.isChecked) { - newVal = this._checkedKeys().filter(x => x !== node.key); + newVal = keys.filter(x => x !== node.key); } else { - newVal = [...this._checkedKeys(), node.key]; + newVal = [...keys, node.key]; } this.selectedNodeChange.emit(node); this._checkedKeys.set(newVal); diff --git a/npm/ng-packs/packages/core/src/lib/directives/autofocus.directive.ts b/npm/ng-packs/packages/core/src/lib/directives/autofocus.directive.ts index 65be9c348a..4400c6d5af 100644 --- a/npm/ng-packs/packages/core/src/lib/directives/autofocus.directive.ts +++ b/npm/ng-packs/packages/core/src/lib/directives/autofocus.directive.ts @@ -1,4 +1,4 @@ -import { AfterViewInit, Directive, ElementRef, inject, input, numberAttribute } from '@angular/core'; +import { AfterViewInit, Directive, ElementRef, inject, input } from '@angular/core'; @Directive({ selector: '[autofocus]', @@ -6,7 +6,10 @@ import { AfterViewInit, Directive, ElementRef, inject, input, numberAttribute } export class AutofocusDirective implements AfterViewInit { private elRef = inject(ElementRef); - readonly delay = input(0, { alias: 'autofocus', transform: numberAttribute }); + readonly delay = input(0, { + alias: 'autofocus', + transform: (v: unknown) => Number(v) || 0, + }); ngAfterViewInit(): void { setTimeout(() => this.elRef.nativeElement.focus(), this.delay()); diff --git a/npm/ng-packs/packages/permission-management/src/lib/components/permission-management.component.ts b/npm/ng-packs/packages/permission-management/src/lib/components/permission-management.component.ts index 1a0298113d..e006820d57 100644 --- a/npm/ng-packs/packages/permission-management/src/lib/components/permission-management.component.ts +++ b/npm/ng-packs/packages/permission-management/src/lib/components/permission-management.component.ts @@ -364,8 +364,9 @@ export class PermissionManagementComponent { } setTabCheckboxState() { + const providerName = this.providerName(); const selectablePermissions = this.selectedGroupPermissions.filter(per => - per.grantedProviders.every(p => p.providerName === this.providerName()), + per.grantedProviders.every(p => p.providerName === providerName), ); const selectedPermissions = selectablePermissions.filter(per => per.isGranted); @@ -386,8 +387,9 @@ export class PermissionManagementComponent { } setGrantCheckboxState() { + const providerName = this.providerName(); const selectablePermissions = this.permissions.filter(per => - per.grantedProviders.every(p => p.providerName === this.providerName()), + per.grantedProviders.every(p => p.providerName === providerName), ); const selectedAllPermissions = selectablePermissions.filter(per => per.isGranted); const checkboxElement = this.document.querySelector('#select-all-in-all-tabs') as any; @@ -487,11 +489,14 @@ export class PermissionManagementComponent { } openModal() { - if (!this.providerKey() || !this.providerName()) { + const providerName = this.providerName(); + const providerKey = this.providerKey(); + + if (!providerKey || !providerName) { throw new Error('Provider Key and Provider Name are required.'); } - return this.service.get(this.providerName(), this.providerKey()).pipe( + return this.service.get(providerName, providerKey).pipe( tap((permissionRes: GetPermissionListResultDto) => { const { groups } = permissionRes || {}; @@ -503,7 +508,7 @@ export class PermissionManagementComponent { this.disabledSelectAllInAllTabs = this.permissions.every( per => per.isGranted && - per.grantedProviders.every(provider => provider.providerName !== this.providerName()), + per.grantedProviders.every(provider => provider.providerName !== providerName), ); }), ); @@ -527,10 +532,12 @@ export class PermissionManagementComponent { shouldFetchAppConfig() { const currentUser = this.configState.getOne('currentUser') as CurrentUserDto; + const providerName = this.providerName(); + const providerKey = this.providerKey(); - if (this.providerName() === 'R') return currentUser.roles.some(role => role === this.providerKey()); + if (providerName === 'R') return currentUser.roles.some(role => role === providerKey); - if (this.providerName() === 'U') return currentUser.id === this.providerKey(); + if (providerName === 'U') return currentUser.id === providerKey; return false; } diff --git a/npm/ng-packs/packages/theme-shared/src/lib/directives/visible.directive.ts b/npm/ng-packs/packages/theme-shared/src/lib/directives/visible.directive.ts index 0f5e428aa0..df8ebf9197 100644 --- a/npm/ng-packs/packages/theme-shared/src/lib/directives/visible.directive.ts +++ b/npm/ng-packs/packages/theme-shared/src/lib/directives/visible.directive.ts @@ -16,9 +16,13 @@ export class AbpVisibleDirective implements OnDestroy { readonly abpVisible = input(); + private lastInput: VisibleInput; + constructor() { effect(() => { const value = this.abpVisible(); + if (value === this.lastInput) return; + this.lastInput = value; this.condition$ = checkType(value); this.subscribeToCondition(); });