From 1a00a018442619b3839f1392fc540cdd48b29c6a Mon Sep 17 00:00:00 2001 From: Benjamin Heiss <114084395+elite-benni@users.noreply.github.com> Date: Thu, 16 Nov 2023 15:52:00 +0100 Subject: [PATCH] refactor: icon bind to host in decorator (#52) * refactor: icon bind to host in decorator refactor to the preferred binding in the decorator. This way also computed is working directly for the binding. addition to issue #46 * fix(avatar): fix HostBinding classes for onPush Host The classes of the Hostbinding where not set correctly for onPush Hosts. Classes where not merged correctly. Now the classes of the icon are set and detected by the host correctly. fixed unit tests as they would have never failed cause of setTimeout. close #36 --- .../fallback/brn-avatar-fallback.directive.ts | 5 +++ .../hlm-avatar-fallback.directive.spec.ts | 26 ++++++-------- .../fallback/hlm-avatar-fallback.directive.ts | 31 +++++------------ .../icon/helm/src/lib/hlm-icon.component.ts | 34 ++++++------------- 4 files changed, 33 insertions(+), 63 deletions(-) diff --git a/libs/ui/avatar/brain/src/lib/fallback/brn-avatar-fallback.directive.ts b/libs/ui/avatar/brain/src/lib/fallback/brn-avatar-fallback.directive.ts index 85b5da5cd..16dafad21 100644 --- a/libs/ui/avatar/brain/src/lib/fallback/brn-avatar-fallback.directive.ts +++ b/libs/ui/avatar/brain/src/lib/fallback/brn-avatar-fallback.directive.ts @@ -7,10 +7,15 @@ import { ClassValue } from 'clsx'; exportAs: 'avatarFallback', }) export class BrnAvatarFallbackDirective { + private readonly element = inject(ElementRef).nativeElement; readonly userCls = signal(''); readonly useAutoColor = signal(false); readonly textContent = inject(ElementRef).nativeElement.textContent; + getTextContent(): string { + return this.element.textContent; + } + @Input() set class(cls: ClassValue | string) { this.userCls.set(cls); } diff --git a/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.spec.ts b/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.spec.ts index a2f0a84b0..1abc88018 100644 --- a/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.spec.ts +++ b/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.spec.ts @@ -1,5 +1,5 @@ import { Component, PLATFORM_ID } from '@angular/core'; -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { ComponentFixture, TestBed, fakeAsync } from '@angular/core/testing'; import { HlmAvatarFallbackDirective } from './hlm-avatar-fallback.directive'; import { hexColorFor, isBright } from '@spartan-ng/ui-avatar-brain'; @@ -7,7 +7,7 @@ import { hexColorFor, isBright } from '@spartan-ng/ui-avatar-brain'; selector: 'hlm-mock', standalone: true, imports: [HlmAvatarFallbackDirective], - template: `fallback2`, + template: `fallback2`, }) class HlmMockComponent { userCls = ''; @@ -36,12 +36,9 @@ describe('HlmAvatarFallbackDirective', () => { it('should add any user defined classes', async () => { component.userCls = 'test-class'; - fixture.detectChanges(); - // fallback uses Promise.resolve().then() so we need to wait for the next tick - setTimeout(() => { - expect(fixture.nativeElement.querySelector('img').className).toContain('test-class'); - }); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('span').className).toContain('test-class'); }); describe('autoColor', () => { @@ -50,19 +47,16 @@ describe('HlmAvatarFallbackDirective', () => { fixture.detectChanges(); }); - it('should remove the bg-muted class from the component', () => { - setTimeout(() => { - expect(fixture.nativeElement.querySelector('span').className).not.toContain('bg-muted'); - }); - }); + it('should remove the bg-muted class from the component', fakeAsync(() => { + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('span').className).not.toContain('bg-muted'); + })); it('should remove add a text color class and hex backgroundColor style depending on its content', () => { const hex = hexColorFor('fallback2'); const textCls = isBright(hex) ? 'text-black' : 'text-white'; - setTimeout(() => { - expect(fixture.nativeElement.querySelector('span').className).toContain(textCls); - expect(fixture.nativeElement.querySelector('span').style.backgroundColor).toBe(hex); - }); + expect(fixture.nativeElement.querySelector('span').className).toContain(textCls); + expect(fixture.nativeElement.querySelector('span').style.backgroundColor).toBe('rgb(144, 53, 149)'); }); }); }); diff --git a/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.ts b/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.ts index 74dbb53ec..b153b0d8e 100644 --- a/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.ts +++ b/libs/ui/avatar/helm/src/lib/fallback/hlm-avatar-fallback.directive.ts @@ -1,4 +1,4 @@ -import { computed, Directive, effect, HostBinding, inject } from '@angular/core'; +import { computed, Directive, inject } from '@angular/core'; import { BrnAvatarFallbackDirective, hexColorFor, isBright } from '@spartan-ng/ui-avatar-brain'; import { hlm } from '@spartan-ng/ui-core'; import { ClassValue } from 'clsx'; @@ -22,34 +22,19 @@ const generateAutoColorTextCls = (hex?: string) => { inputs: ['class:class', 'autoColor:autoColor'], }, ], + host: { + '[class]': 'generatedClasses()', + '[style.backgroundColor]': "hex() || ''", + }, }) export class HlmAvatarFallbackDirective { private readonly brn = inject(BrnAvatarFallbackDirective); private readonly hex = computed(() => { - if (!this.brn.useAutoColor() || !this.brn.textContent) return; - return hexColorFor(this.brn.textContent); + if (!this.brn.useAutoColor() || !this.brn.getTextContent()) return; + return hexColorFor(this.brn.getTextContent()); }); private readonly autoColorTextCls = computed(() => generateAutoColorTextCls(this.hex())); - @HostBinding('class') - protected cls = generateClasses(this.brn?.userCls(), this.autoColorTextCls()); - - @HostBinding('style.backgroundColor') - protected backgroundColor = this.hex() || ''; - - constructor() { - effect(() => { - const cls = generateClasses(this.brn.userCls(), this.autoColorTextCls()); - Promise.resolve().then(() => { - this.cls = cls; - }); - }); - effect(() => { - const hex = this.hex() || ''; - Promise.resolve().then(() => { - this.backgroundColor = hex; - }); - }); - } + protected readonly generatedClasses = computed(() => generateClasses(this.brn?.userCls(), this.autoColorTextCls())); } diff --git a/libs/ui/icon/helm/src/lib/hlm-icon.component.ts b/libs/ui/icon/helm/src/lib/hlm-icon.component.ts index b576513eb..c34d655bb 100644 --- a/libs/ui/icon/helm/src/lib/hlm-icon.component.ts +++ b/libs/ui/icon/helm/src/lib/hlm-icon.component.ts @@ -1,12 +1,4 @@ -import { - ChangeDetectionStrategy, - Component, - computed, - HostBinding, - Input, - signal, - ViewEncapsulation, -} from '@angular/core'; +import { ChangeDetectionStrategy, Component, computed, Input, signal, ViewEncapsulation } from '@angular/core'; import { IconName, NgIconComponent } from '@ng-icons/core'; import { hlm } from '@spartan-ng/ui-core'; import { cva } from 'class-variance-authority'; @@ -51,6 +43,9 @@ const isDefinedSize = (size: IconSize): size is DefinedSizes => { [color]="_color()" [strokeWidth]="_strokeWidth()" />`, + host: { + '[class]': 'generatedClasses()', + }, }) export class HlmIconComponent { protected readonly _name = signal(''); @@ -61,48 +56,39 @@ export class HlmIconComponent { protected readonly ngIconSize = computed(() => (isDefinedSize(this._size()) ? '100%' : (this._size() as string))); protected readonly ngIconCls = signal(''); + protected readonly generatedClasses = computed(() => { + const size: IconSize = this._size(); + const variant = isDefinedSize(size) ? size : 'none'; + return hlm(iconVariants({ variant }), this.userCls()); + }); + @Input({ required: true }) set name(value: IconName | string) { this._name.set(value); - this.cls = this.generateClasses(); } @Input() set size(value: IconSize) { this._size.set(value); - this.cls = this.generateClasses(); } @Input() set color(value: string | undefined) { this._color.set(value); - this.cls = this.generateClasses(); } @Input() set strokeWidth(value: string | number | undefined) { this._strokeWidth.set(value); - this.cls = this.generateClasses(); } @Input() set ngIconClass(cls: ClassValue) { this.ngIconCls.set(cls); - this.cls = this.generateClasses(); } @Input() set class(cls: ClassValue) { this.userCls.set(cls); - this.cls = this.generateClasses(); - } - - @HostBinding('class') - protected cls = this.generateClasses(); - - private generateClasses() { - const size: IconSize = this._size(); - const variant = isDefinedSize(size) ? size : 'none'; - return hlm(iconVariants({ variant }), this.userCls()); } }