From 89b8f27a6a230ec683b14729ae4b00ca72884ad2 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 00:02:28 +0530 Subject: [PATCH 1/8] feat: show selected item's icon in trigger label if applicable --- .../src/select/select.component.test.ts | 52 +++++++++++++++++-- .../components/src/select/select.component.ts | 27 +++++++++- .../components/src/select/select.module.ts | 4 +- 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/projects/components/src/select/select.component.test.ts b/projects/components/src/select/select.component.test.ts index 91780cbaf..f6f92870d 100644 --- a/projects/components/src/select/select.component.test.ts +++ b/projects/components/src/select/select.component.test.ts @@ -1,8 +1,11 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; import { fakeAsync, flush } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; import { IconLibraryTestingModule, IconType } from '@hypertrace/assets-library'; -import { NavigationService } from '@hypertrace/common'; +import { MemoizeModule, NavigationService } from '@hypertrace/common'; +import { IconComponent } from '@hypertrace/components'; import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest'; +import { MockComponent } from 'ng-mocks'; import { EMPTY } from 'rxjs'; import { SelectControlOptionPosition } from './select-control-option.component'; import { SelectJustify } from './select-justify'; @@ -12,8 +15,9 @@ import { SelectModule } from './select.module'; describe('Select Component', () => { const hostFactory = createHostFactory>({ component: SelectComponent, - imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule], + imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule, MemoizeModule], declareComponent: false, + declarations: [MockComponent(IconComponent)], providers: [ mockProvider(NavigationService, { navigation$: EMPTY, @@ -27,7 +31,7 @@ describe('Select Component', () => { const selectionOptions = [ { label: 'first', value: 'first-value' }, { label: 'second', value: 'second-value' }, - { label: 'third', value: 'third-value', selectedLabel: 'Third Value!!!' } + { label: 'third', value: 'third-value', selectedLabel: 'Third Value!!!', icon: 'test-icon', iconColor: 'red' } ]; test('should display initial selection', fakeAsync(() => { @@ -146,12 +150,52 @@ describe('Select Component', () => { const optionElements = spectator.queryAll('.select-option', { root: true }); spectator.click(optionElements[2]); - expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(selectionOptions[2].value); expect(spectator.query('.trigger-content')).toHaveText(selectionOptions[2].selectedLabel!); flush(); })); + test('should set trigger-prefix-icon correctly', fakeAsync(() => { + spectator = hostFactory( + ` + + + + `, + { + hostProps: { + options: selectionOptions, + icon: 'select-level-icon' + } + } + ); + spectator.tick(); + + // No selection -> select component level icon and no color + expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.icon).toBe( + 'select-level-icon' + ); + expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.color).toBe(undefined); + + // Selection with no icon -> no icon and no color + spectator.click('.trigger-content'); + let optionElements = spectator.queryAll('.select-option', { root: true }); + spectator.click(optionElements[1]); + spectator.tick(); + expect(spectator.query('.trigger-prefix-icon')).not.toExist(); + + // Selection with icon and color + spectator.click('.trigger-content'); + optionElements = spectator.queryAll('.select-option', { root: true }); + spectator.click(optionElements[2]); + spectator.tick(); + + expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.icon).toBe('test-icon'); + expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.color).toBe('red'); + + flush(); + })); + test('should set correct label alignment', fakeAsync(() => { spectator = hostFactory( ` diff --git a/projects/components/src/select/select.component.ts b/projects/components/src/select/select.component.ts index 445604f31..1a9ced4ac 100644 --- a/projects/components/src/select/select.component.ts +++ b/projects/components/src/select/select.component.ts @@ -55,7 +55,13 @@ import { SelectSize } from './select-size'; class="trigger-content menu-with-border" [ngClass]="[this.justifyClass]" > - + @@ -204,6 +210,25 @@ export class SelectComponent implements AfterContentInit, OnChanges { } } + public getPrefixIcon( + selectedOption: SelectOptionComponent | undefined, + triggerIcon: string | undefined + ): string | undefined { + if (selectedOption !== undefined && selectedOption?.icon !== undefined) { + return selectedOption.icon; + } + + if (selectedOption !== undefined) { + return undefined; + } + + return triggerIcon; + } + + public getPrefixIconColor(selectedOption: SelectOptionComponent): string | undefined { + return selectedOption !== undefined && selectedOption?.iconColor ? selectedOption.iconColor : undefined; + } + public ngOnChanges(changes: TypedSimpleChanges): void { if (this.items !== undefined && changes.selected !== undefined) { this.selected$ = this.buildObservableOfSelected(); diff --git a/projects/components/src/select/select.module.ts b/projects/components/src/select/select.module.ts index 75088454e..7a49b5802 100644 --- a/projects/components/src/select/select.module.ts +++ b/projects/components/src/select/select.module.ts @@ -1,6 +1,7 @@ import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; import { FormsModule } from '@angular/forms'; +import { MemoizeModule } from '@hypertrace/common'; import { DividerModule } from '../divider/divider.module'; import { IconModule } from '../icon/icon.module'; import { LabelModule } from '../label/label.module'; @@ -21,7 +22,8 @@ import { SelectComponent } from './select.component'; LetAsyncModule, PopoverModule, TooltipModule, - DividerModule + DividerModule, + MemoizeModule ], declarations: [SelectComponent, SelectOptionComponent, SelectGroupComponent, SelectControlOptionComponent], exports: [SelectComponent, SelectOptionComponent, SelectGroupComponent, SelectControlOptionComponent] From b30f50d047ee5685557853590069f1d61f1baf20 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 00:08:12 +0530 Subject: [PATCH 2/8] fix: explicit undefined check --- projects/components/src/select/select.component.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/projects/components/src/select/select.component.ts b/projects/components/src/select/select.component.ts index 1a9ced4ac..859935c8a 100644 --- a/projects/components/src/select/select.component.ts +++ b/projects/components/src/select/select.component.ts @@ -226,7 +226,9 @@ export class SelectComponent implements AfterContentInit, OnChanges { } public getPrefixIconColor(selectedOption: SelectOptionComponent): string | undefined { - return selectedOption !== undefined && selectedOption?.iconColor ? selectedOption.iconColor : undefined; + return selectedOption !== undefined && selectedOption?.iconColor !== undefined + ? selectedOption.iconColor + : undefined; } public ngOnChanges(changes: TypedSimpleChanges): void { From 4f23fc9e08dd06617e20aafdc74064c5a86b0e4d Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 00:15:44 +0530 Subject: [PATCH 3/8] fix: typedef --- projects/components/src/select/select.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/components/src/select/select.component.ts b/projects/components/src/select/select.component.ts index 859935c8a..7c9b88114 100644 --- a/projects/components/src/select/select.component.ts +++ b/projects/components/src/select/select.component.ts @@ -225,7 +225,7 @@ export class SelectComponent implements AfterContentInit, OnChanges { return triggerIcon; } - public getPrefixIconColor(selectedOption: SelectOptionComponent): string | undefined { + public getPrefixIconColor(selectedOption: SelectOptionComponent | undefined): string | undefined { return selectedOption !== undefined && selectedOption?.iconColor !== undefined ? selectedOption.iconColor : undefined; From cfcbda351bbd5e89379b81f47907f465d240eb69 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 08:43:57 +0530 Subject: [PATCH 4/8] fix: update logic --- .../components/src/select/select.component.ts | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/projects/components/src/select/select.component.ts b/projects/components/src/select/select.component.ts index 7c9b88114..ea1fd70c5 100644 --- a/projects/components/src/select/select.component.ts +++ b/projects/components/src/select/select.component.ts @@ -56,11 +56,11 @@ import { SelectSize } from './select-size'; [ngClass]="[this.justifyClass]" > @@ -210,22 +210,11 @@ export class SelectComponent implements AfterContentInit, OnChanges { } } - public getPrefixIcon( - selectedOption: SelectOptionComponent | undefined, - triggerIcon: string | undefined - ): string | undefined { - if (selectedOption !== undefined && selectedOption?.icon !== undefined) { - return selectedOption.icon; - } - - if (selectedOption !== undefined) { - return undefined; - } - - return triggerIcon; + public getPrefixIcon(selectedOption: SelectOption | undefined): string | undefined { + return selectedOption !== undefined ? selectedOption.icon : this.icon; } - public getPrefixIconColor(selectedOption: SelectOptionComponent | undefined): string | undefined { + public getPrefixIconColor(selectedOption: SelectOption | undefined): string | undefined { return selectedOption !== undefined && selectedOption?.iconColor !== undefined ? selectedOption.iconColor : undefined; From 1b69a1881a4ea2609239bba218947b6db39b3570 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 08:49:40 +0530 Subject: [PATCH 5/8] simplify logic. update test --- projects/components/src/select/select.component.test.ts | 6 +++--- projects/components/src/select/select.component.ts | 8 +------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/projects/components/src/select/select.component.test.ts b/projects/components/src/select/select.component.test.ts index f6f92870d..2c77fb4ef 100644 --- a/projects/components/src/select/select.component.test.ts +++ b/projects/components/src/select/select.component.test.ts @@ -2,7 +2,7 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; import { fakeAsync, flush } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { IconLibraryTestingModule, IconType } from '@hypertrace/assets-library'; -import { MemoizeModule, NavigationService } from '@hypertrace/common'; +import { NavigationService } from '@hypertrace/common'; import { IconComponent } from '@hypertrace/components'; import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; @@ -15,7 +15,7 @@ import { SelectModule } from './select.module'; describe('Select Component', () => { const hostFactory = createHostFactory>({ component: SelectComponent, - imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule, MemoizeModule], + imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule], declareComponent: false, declarations: [MockComponent(IconComponent)], providers: [ @@ -175,7 +175,7 @@ describe('Select Component', () => { expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.icon).toBe( 'select-level-icon' ); - expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.color).toBe(undefined); + expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.color).toBe(null); // Selection with no icon -> no icon and no color spectator.click('.trigger-content'); diff --git a/projects/components/src/select/select.component.ts b/projects/components/src/select/select.component.ts index ea1fd70c5..98a5bf2cf 100644 --- a/projects/components/src/select/select.component.ts +++ b/projects/components/src/select/select.component.ts @@ -60,7 +60,7 @@ import { SelectSize } from './select-size'; class="trigger-prefix-icon" [icon]="this.getPrefixIcon(selected)" [size]="this.iconSize" - [color]="this.getPrefixIconColor(selected)" + [color]="selected?.iconColor" > @@ -214,12 +214,6 @@ export class SelectComponent implements AfterContentInit, OnChanges { return selectedOption !== undefined ? selectedOption.icon : this.icon; } - public getPrefixIconColor(selectedOption: SelectOption | undefined): string | undefined { - return selectedOption !== undefined && selectedOption?.iconColor !== undefined - ? selectedOption.iconColor - : undefined; - } - public ngOnChanges(changes: TypedSimpleChanges): void { if (this.items !== undefined && changes.selected !== undefined) { this.selected$ = this.buildObservableOfSelected(); From b8f0dc3b6911a39d8a20360a72b90e02fe526df1 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 08:58:23 +0530 Subject: [PATCH 6/8] fix: override no-null --- projects/components/src/select/select.component.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/projects/components/src/select/select.component.test.ts b/projects/components/src/select/select.component.test.ts index 2c77fb4ef..df69b4705 100644 --- a/projects/components/src/select/select.component.test.ts +++ b/projects/components/src/select/select.component.test.ts @@ -175,6 +175,7 @@ describe('Select Component', () => { expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.icon).toBe( 'select-level-icon' ); + // tslint:disable-next-line:no-null-keyword expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.color).toBe(null); // Selection with no icon -> no icon and no color From 3aa2e25fa1df446799ae8e2d9a21344e3bba0e4d Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 09:48:58 +0530 Subject: [PATCH 7/8] fix: remove unused module --- projects/components/src/select/select.module.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/projects/components/src/select/select.module.ts b/projects/components/src/select/select.module.ts index 7a49b5802..75088454e 100644 --- a/projects/components/src/select/select.module.ts +++ b/projects/components/src/select/select.module.ts @@ -1,7 +1,6 @@ import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; import { FormsModule } from '@angular/forms'; -import { MemoizeModule } from '@hypertrace/common'; import { DividerModule } from '../divider/divider.module'; import { IconModule } from '../icon/icon.module'; import { LabelModule } from '../label/label.module'; @@ -22,8 +21,7 @@ import { SelectComponent } from './select.component'; LetAsyncModule, PopoverModule, TooltipModule, - DividerModule, - MemoizeModule + DividerModule ], declarations: [SelectComponent, SelectOptionComponent, SelectGroupComponent, SelectControlOptionComponent], exports: [SelectComponent, SelectOptionComponent, SelectGroupComponent, SelectControlOptionComponent] From 16983c3c8709f538559dd6c7ffa2493492e42594 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Fri, 28 May 2021 17:03:21 +0530 Subject: [PATCH 8/8] fix: update behaviour --- projects/components/src/select/select.component.test.ts | 7 +++++-- projects/components/src/select/select.component.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/projects/components/src/select/select.component.test.ts b/projects/components/src/select/select.component.test.ts index df69b4705..af86bcf6c 100644 --- a/projects/components/src/select/select.component.test.ts +++ b/projects/components/src/select/select.component.test.ts @@ -178,12 +178,15 @@ describe('Select Component', () => { // tslint:disable-next-line:no-null-keyword expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.color).toBe(null); - // Selection with no icon -> no icon and no color + // Selection with no icon -> default icon and no color spectator.click('.trigger-content'); let optionElements = spectator.queryAll('.select-option', { root: true }); spectator.click(optionElements[1]); spectator.tick(); - expect(spectator.query('.trigger-prefix-icon')).not.toExist(); + expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.icon).toBe( + 'select-level-icon' + ); + expect(spectator.debugElement.query(By.css('.trigger-prefix-icon')).componentInstance.color).toBe(undefined); // Selection with icon and color spectator.click('.trigger-content'); diff --git a/projects/components/src/select/select.component.ts b/projects/components/src/select/select.component.ts index 98a5bf2cf..79cdc9ef9 100644 --- a/projects/components/src/select/select.component.ts +++ b/projects/components/src/select/select.component.ts @@ -211,7 +211,7 @@ export class SelectComponent implements AfterContentInit, OnChanges { } public getPrefixIcon(selectedOption: SelectOption | undefined): string | undefined { - return selectedOption !== undefined ? selectedOption.icon : this.icon; + return selectedOption?.icon ?? this.icon; } public ngOnChanges(changes: TypedSimpleChanges): void {