From caae69b66056e9b95fcf2c4cdba7f2e0321eb1ed Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Wed, 20 Jan 2021 14:55:13 -0800 Subject: [PATCH 1/2] feat: adding all option and fixing some styling --- projects/common/src/color/color.ts | 1 + .../multi-select/multi-select.component.scss | 7 ++- .../multi-select.component.test.ts | 49 +++++++++++++++++-- .../multi-select/multi-select.component.ts | 48 ++++++++++++------ .../src/multi-select/multi-select.module.ts | 3 +- 5 files changed, 85 insertions(+), 23 deletions(-) diff --git a/projects/common/src/color/color.ts b/projects/common/src/color/color.ts index 995dcf915..1814e61c5 100644 --- a/projects/common/src/color/color.ts +++ b/projects/common/src/color/color.ts @@ -7,6 +7,7 @@ export const enum Color { BlueGray4 = '#4b5f77', Gray1 = '#f4f5f5', Gray2 = '#e1e4e5', + Gray4 = '#889499', Gray5 = '#657277', Gray7 = '#272c2e', Gray9 = '#080909', diff --git a/projects/components/src/multi-select/multi-select.component.scss b/projects/components/src/multi-select/multi-select.component.scss index 491b1f465..6a4d457c9 100644 --- a/projects/components/src/multi-select/multi-select.component.scss +++ b/projects/components/src/multi-select/multi-select.component.scss @@ -54,7 +54,6 @@ .multi-select-content { @include dropdown(); min-width: 120px; - max-height: 204px; .multi-select-option { display: flex; @@ -65,15 +64,15 @@ align-items: center; .checkbox { - margin-right: 8px; + margin: 0px; } .icon { - margin-right: 8px; + margin-left: 8px; } .label { - margin-right: 8px; + margin-left: 8px; } &:hover { diff --git a/projects/components/src/multi-select/multi-select.component.test.ts b/projects/components/src/multi-select/multi-select.component.test.ts index b5e48674d..41973f8ad 100644 --- a/projects/components/src/multi-select/multi-select.component.test.ts +++ b/projects/components/src/multi-select/multi-select.component.test.ts @@ -2,6 +2,7 @@ import { fakeAsync, flush } from '@angular/core/testing'; import { IconType } from '@hypertrace/assets-library'; import { createHostFactory, SpectatorHost } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; +import { DividerComponent } from '../divider/divider.component'; import { LabelComponent } from '../label/label.component'; import { LetAsyncModule } from '../let-async/let-async.module'; import { SelectJustify } from '../select/select-justify'; @@ -13,7 +14,7 @@ describe('Multi Select Component', () => { component: MultiSelectComponent, imports: [LetAsyncModule], entryComponents: [SelectOptionComponent], - declarations: [MockComponent(LabelComponent)], + declarations: [MockComponent(LabelComponent), MockComponent(DividerComponent)], shallow: true }); @@ -75,7 +76,7 @@ describe('Multi Select Component', () => { spectator.tick(); spectator.click('.trigger-content'); - const optionElements = spectator.queryAll('.multi-select-option', { root: true }); + const optionElements = spectator.queryAll('.multi-select-option:not(.all-options)', { root: true }); expect(spectator.query('.multi-select-content', { root: true })).toExist(); expect(optionElements.length).toBe(3); @@ -109,8 +110,9 @@ describe('Multi Select Component', () => { spectator.tick(); spectator.click('.trigger-content'); - const optionElements = spectator.queryAll('.multi-select-option', { root: true }); + const optionElements = spectator.queryAll('.multi-select-option:not(.all-options)', { root: true }); spectator.click(optionElements[2]); + spectator.tick(); expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith([selectionOptions[1].value, selectionOptions[2].value]); @@ -118,6 +120,45 @@ describe('Multi Select Component', () => { flush(); })); + test('should notify and update selection when all checkbox is selected', fakeAsync(() => { + const onChange = jest.fn(); + + spectator = hostFactory( + ` + + + + `, + { + hostProps: { + options: selectionOptions, + selected: [selectionOptions[1].value], + placeholder: 'Select options', + onChange: onChange + } + } + ); + + spectator.tick(); + spectator.click('.trigger-content'); + + const allOptionElement = spectator.query('.all-options', { root: true }); + expect(allOptionElement).toExist(); + spectator.click(allOptionElement!); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith(selectionOptions.map(option => option.value)); + expect(spectator.query(LabelComponent)?.label).toEqual('first and 2 more'); + + // De select all + spectator.click(allOptionElement!); + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenLastCalledWith([]); + expect(spectator.query(LabelComponent)?.label).toEqual('Select options'); + + flush(); + })); + test('should notify but not change trigger label if triggerLabelDisplayMode is placeholder', fakeAsync(() => { const onChange = jest.fn(); @@ -141,7 +182,7 @@ describe('Multi Select Component', () => { expect(spectator.query(LabelComponent)?.label).toEqual('Placeholder'); spectator.click('.trigger-content'); - const optionElements = spectator.queryAll('.multi-select-option', { root: true }); + const optionElements = spectator.queryAll('.multi-select-option:not(.all-options)', { root: true }); spectator.click(optionElements[2]); expect(onChange).toHaveBeenCalledTimes(1); diff --git a/projects/components/src/multi-select/multi-select.component.ts b/projects/components/src/multi-select/multi-select.component.ts index 35c10caad..93abe2e75 100644 --- a/projects/components/src/multi-select/multi-select.component.ts +++ b/projects/components/src/multi-select/multi-select.component.ts @@ -40,6 +40,13 @@ import { SelectSize } from '../select/select-size';
+
+ + All +
+ + +
implements AfterContentInit, OnChanges { this.setTriggerLabel(); } + public onAllSelectionChange(): void { + this.selected = this.areAllOptionsSelected() ? [] : this.items!.map(item => item.value); // Select All or none + this.setSelection(); + } + + public areAllOptionsSelected(): boolean { + return this.selected !== undefined && this.items !== undefined && this.selected.length === this.items.length; + } + + public onSelectionChange(item: SelectOptionComponent): void { + this.selected = this.isSelectedItem(item) + ? this.selected?.filter(value => value !== item.value) + : (this.selected ?? []).concat(item.value); + + this.setSelection(); + } + + public isSelectedItem(item: SelectOptionComponent): boolean { + return this.selected !== undefined && this.selected.filter(value => value === item.value).length > 0; + } + + private setSelection(): void { + this.setTriggerLabel(); + this.selected$ = this.buildObservableOfSelected(); + this.selectedChange.emit(this.selected); + } + private setTriggerLabel(): void { if (this.triggerLabelDisplayMode === TriggerLabelDisplayMode.Placeholder) { this.triggerLabel = this.placeholder; @@ -130,10 +164,6 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { } } - public isSelectedItem(item: SelectOptionComponent): boolean { - return this.selected !== undefined && this.selected.filter(value => value === item.value).length > 0; - } - private buildObservableOfSelected(): Observable[] | undefined> { if (!this.items) { return EMPTY; @@ -145,16 +175,6 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { ); } - public onSelectionChange(item: SelectOptionComponent): void { - this.selected = this.isSelectedItem(item) - ? this.selected?.filter(value => value !== item.value) - : (this.selected ?? []).concat(item.value); - - this.setTriggerLabel(); - this.selected$ = this.buildObservableOfSelected(); - this.selectedChange.emit(this.selected); - } - // Find the select option object for a value private findItems(value: V[] | undefined): SelectOption[] | undefined { if (this.items === undefined) { diff --git a/projects/components/src/multi-select/multi-select.module.ts b/projects/components/src/multi-select/multi-select.module.ts index 2382650e9..84e413994 100644 --- a/projects/components/src/multi-select/multi-select.module.ts +++ b/projects/components/src/multi-select/multi-select.module.ts @@ -1,6 +1,7 @@ import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; import { FormsModule } from '@angular/forms'; +import { DividerModule } from '../divider/divider.module'; import { IconModule } from '../icon/icon.module'; import { LabelModule } from '../label/label.module'; import { LetAsyncModule } from '../let-async/let-async.module'; @@ -8,7 +9,7 @@ import { PopoverModule } from '../popover/popover.module'; import { MultiSelectComponent } from './multi-select.component'; @NgModule({ - imports: [FormsModule, CommonModule, IconModule, LabelModule, LetAsyncModule, PopoverModule], + imports: [FormsModule, CommonModule, IconModule, LabelModule, LetAsyncModule, PopoverModule, DividerModule], declarations: [MultiSelectComponent], exports: [MultiSelectComponent] }) From 80abcadcb971468482d4b3afc31e825ad4c35d2f Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Thu, 21 Jan 2021 11:56:33 -0800 Subject: [PATCH 2/2] refactor: addressing review comments --- .../multi-select.component.test.ts | 26 ++++++++++++++++++- .../multi-select/multi-select.component.ts | 15 +++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/projects/components/src/multi-select/multi-select.component.test.ts b/projects/components/src/multi-select/multi-select.component.test.ts index 41973f8ad..6a870019e 100644 --- a/projects/components/src/multi-select/multi-select.component.test.ts +++ b/projects/components/src/multi-select/multi-select.component.test.ts @@ -120,12 +120,35 @@ describe('Multi Select Component', () => { flush(); })); + test('should show all checkbox only when enabled by input property', fakeAsync(() => { + spectator = hostFactory( + ` + + + + `, + { + hostProps: { + options: selectionOptions + } + } + ); + + spectator.tick(); + spectator.click('.trigger-content'); + + const allOptionElement = spectator.query('.all-options', { root: true }); + expect(allOptionElement).not.toExist(); + + flush(); + })); + test('should notify and update selection when all checkbox is selected', fakeAsync(() => { const onChange = jest.fn(); spectator = hostFactory( ` - + `, @@ -134,6 +157,7 @@ describe('Multi Select Component', () => { options: selectionOptions, selected: [selectionOptions[1].value], placeholder: 'Select options', + showAllOptionControl: true, onChange: onChange } } diff --git a/projects/components/src/multi-select/multi-select.component.ts b/projects/components/src/multi-select/multi-select.component.ts index 93abe2e75..a7f092087 100644 --- a/projects/components/src/multi-select/multi-select.component.ts +++ b/projects/components/src/multi-select/multi-select.component.ts @@ -40,12 +40,14 @@ import { SelectSize } from '../select/select-size';
-
- - All -
+ +
+ + All +
- + +
@@ -86,6 +88,9 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { @Input() public justify?: SelectJustify; + @Input() + public showAllOptionControl?: boolean = false; + @Input() public triggerLabelDisplayMode: TriggerLabelDisplayMode = TriggerLabelDisplayMode.Selection;