Skip to content

Commit 22921bf

Browse files
jake-bassettjake-bassett
authored
Fix multi select view update (#1228)
* fix: multiselect updating trigger label * style: prettier * feat: remove subscription * test: fix test
1 parent d86f11e commit 22921bf

File tree

2 files changed

+89
-29
lines changed

2 files changed

+89
-29
lines changed

‎projects/components/src/multi-select/multi-select.component.test.ts‎

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { CommonModule } from '@angular/common';
44
import { fakeAsync, flush } from '@angular/core/testing';
55
import { IconLibraryTestingModule, IconType } from '@hypertrace/assets-library';
66
import { NavigationService } from '@hypertrace/common';
7+
import { PopoverComponent } from '@hypertrace/components';
8+
import { runFakeRxjs } from '@hypertrace/test-utils';
79
import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest';
810
import { MockComponent } from 'ng-mocks';
911
import { NEVER } from 'rxjs';
@@ -12,7 +14,6 @@ import { CheckboxComponent } from '../checkbox/checkbox.component';
1214
import { DividerComponent } from '../divider/divider.component';
1315
import { LabelComponent } from '../label/label.component';
1416
import { LoadAsyncModule } from '../load-async/load-async.module';
15-
import { PopoverComponent } from '../popover/popover.component';
1617
import { PopoverModule } from '../popover/popover.module';
1718
import { SearchBoxComponent } from '../search-box/search-box.component';
1819
import { SelectOptionComponent } from '../select/select-option.component';
@@ -69,15 +70,20 @@ describe('Multi Select Component', () => {
6970

7071
spectator.tick();
7172

72-
expect(spectator.component.triggerLabel).toEqual(selectionOptions[1].label);
7373
expect(spectator.query('.trigger-content')).toExist();
7474
expect(spectator.query('.trigger-label-container')).toExist();
7575
expect(spectator.query('.trigger-label')).toExist();
7676
expect(spectator.query('.trigger-icon')).toExist();
7777
expect(spectator.query('.trigger-more-items')).not.toExist();
7878

79-
// Selected element is 1 as set in hostProps
80-
expect(spectator.component.selectedItemsCount).toBe(1);
79+
runFakeRxjs(({ expectObservable }) => {
80+
expectObservable(spectator.component.triggerValues$).toBe('x', {
81+
x: {
82+
label: selectionOptions[1].label,
83+
selectedItemsCount: 1 // Selected element is 1 as set in hostProps
84+
}
85+
});
86+
});
8187

8288
const popoverComponent = spectator.query(PopoverComponent);
8389
expect(popoverComponent?.closeOnClick).toEqual(false);
@@ -101,11 +107,19 @@ describe('Multi Select Component', () => {
101107
spectator.tick();
102108
const selectedCheckboxElements = spectator.queryAll('ht-checkbox', { root: true });
103109
expect(spectator.query('.trigger-more-items')).toExist();
104-
expect(spectator.component.selectedItemsCount).toBe(2);
105110
expect(
106111
selectedCheckboxElements.filter(checkboxElement => checkboxElement.getAttribute('ng-reflect-checked') === 'true')
107112
.length
108113
).toBe(2);
114+
115+
runFakeRxjs(({ expectObservable }) => {
116+
expectObservable(spectator.component.triggerValues$).toBe('x', {
117+
x: {
118+
label: selectionOptions[1].label,
119+
selectedItemsCount: 2
120+
}
121+
});
122+
});
109123
}));
110124

111125
test('should display provided options with icons when clicked', fakeAsync(() => {
@@ -136,12 +150,20 @@ describe('Multi Select Component', () => {
136150

137151
const selectedCheckboxElements = spectator.queryAll('ht-checkbox', { root: true });
138152
expect(spectator.query('.trigger-more-items')).toExist();
139-
expect(spectator.component.selectedItemsCount).toBe(2);
140153
expect(
141154
selectedCheckboxElements.filter(checkboxElement => checkboxElement.getAttribute('ng-reflect-checked') === 'true')
142155
.length
143156
).toBe(2);
144157

158+
runFakeRxjs(({ expectObservable }) => {
159+
expectObservable(spectator.component.triggerValues$).toBe('x', {
160+
x: {
161+
label: 'second',
162+
selectedItemsCount: 2
163+
}
164+
});
165+
});
166+
145167
optionElements.forEach((element, index) => {
146168
expect(element).toHaveText(selectionOptions[index].label);
147169
expect(element.querySelector('ht-icon')).toExist();
@@ -204,8 +226,17 @@ describe('Multi Select Component', () => {
204226
expect(onChange).toHaveBeenCalledTimes(1);
205227
expect(onChange).toHaveBeenCalledWith([selectionOptions[1].value, selectionOptions[2].value]);
206228
expect(spectator.query('.trigger-more-items')).toExist();
207-
expect(spectator.component.selectedItemsCount).toBe(2);
208229
expect(spectator.query(LabelComponent)?.label).toEqual('second');
230+
231+
runFakeRxjs(({ expectObservable }) => {
232+
expectObservable(spectator.component.triggerValues$).toBe('x', {
233+
x: {
234+
label: 'second',
235+
selectedItemsCount: 2
236+
}
237+
});
238+
});
239+
209240
flush();
210241
}));
211242

@@ -332,7 +363,16 @@ describe('Multi Select Component', () => {
332363
expect(onChange).toHaveBeenCalledWith([selectionOptions[1].value, selectionOptions[2].value]);
333364
expect(spectator.query(LabelComponent)?.label).toEqual('Placeholder');
334365
expect(spectator.query('.trigger-more-items')).not.toExist();
335-
expect(spectator.component.selectedItemsCount).toBe(0);
366+
367+
runFakeRxjs(({ expectObservable }) => {
368+
expectObservable(spectator.component.triggerValues$).toBe('(x|)', {
369+
x: {
370+
label: 'Placeholder',
371+
selectedItemsCount: 0
372+
}
373+
});
374+
});
375+
336376
flush();
337377
}));
338378

@@ -354,7 +394,15 @@ describe('Multi Select Component', () => {
354394
);
355395
spectator.tick();
356396

357-
expect(spectator.component.triggerLabel).toEqual(selectionOptions[1].label);
397+
runFakeRxjs(({ expectObservable }) => {
398+
expectObservable(spectator.component.triggerValues$).toBe('x', {
399+
x: {
400+
label: selectionOptions[1].label,
401+
selectedItemsCount: 1
402+
}
403+
});
404+
});
405+
358406
expect(spectator.query('.trigger-content')).toExist();
359407
expect(spectator.query('.trigger-label-container')).toExist();
360408
expect(spectator.query('.trigger-label')).toExist();

‎projects/components/src/multi-select/multi-select.component.ts‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import {
1010
QueryList
1111
} from '@angular/core';
1212
import { IconType } from '@hypertrace/assets-library';
13-
import { queryListAndChanges$ } from '@hypertrace/common';
14-
import { BehaviorSubject, combineLatest, EMPTY, Observable, Subject } from 'rxjs';
13+
import { queryListAndChanges$, SubscriptionLifecycle } from '@hypertrace/common';
14+
import { BehaviorSubject, combineLatest, EMPTY, Observable, of, Subject } from 'rxjs';
1515
import { map } from 'rxjs/operators';
1616
import { ButtonRole, ButtonStyle } from '../button/button';
1717
import { IconSize } from '../icon/icon-size';
@@ -24,6 +24,7 @@ import { MultiSelectJustify } from './multi-select-justify';
2424
selector: 'ht-multi-select',
2525
styleUrls: ['./multi-select.component.scss'],
2626
changeDetection: ChangeDetectionStrategy.OnPush,
27+
providers: [SubscriptionLifecycle],
2728
template: `
2829
<div
2930
class="multi-select"
@@ -47,14 +48,16 @@ import { MultiSelectJustify } from './multi-select-justify';
4748
[ngClass]="[this.triggerLabelDisplayMode, this.popoverOpen ? 'open' : '']"
4849
#triggerContainer
4950
>
50-
<ht-icon *ngIf="this.icon" [icon]="this.icon" [size]="this.iconSize"> </ht-icon>
51-
<div *ngIf="!this.isIconOnlyMode()" class="trigger-label-container">
52-
<ht-label class="trigger-label" [label]="this.triggerLabel"></ht-label>
53-
<span *ngIf="this.selectedItemsCount > 1" class="trigger-more-items"
54-
>+{{ this.selectedItemsCount - 1 }}</span
55-
>
56-
<ht-icon class="trigger-icon" icon="${IconType.ChevronDown}" size="${IconSize.Small}"></ht-icon>
57-
</div>
51+
<ht-icon *ngIf="this.icon" [icon]="this.icon" [size]="this.iconSize"></ht-icon>
52+
<ng-container *htLoadAsync="this.triggerValues$ as triggerValues">
53+
<div *ngIf="!this.isIconOnlyMode()" class="trigger-label-container">
54+
<ht-label class="trigger-label" [label]="triggerValues.label"></ht-label>
55+
<span *ngIf="triggerValues.selectedItemsCount > 1" class="trigger-more-items"
56+
>+{{ triggerValues.selectedItemsCount - 1 }}</span
57+
>
58+
<ht-icon class="trigger-icon" icon="${IconType.ChevronDown}" size="${IconSize.Small}"></ht-icon>
59+
</div>
60+
</ng-container>
5861
</div>
5962
</ht-popover-trigger>
6063
<ht-popover-content>
@@ -163,8 +166,7 @@ export class MultiSelectComponent<V> implements AfterContentInit, OnChanges {
163166
private readonly searchSubject: Subject<string> = new BehaviorSubject('');
164167

165168
public popoverOpen: boolean = false;
166-
public triggerLabel?: string;
167-
public selectedItemsCount: number = 0;
169+
public triggerValues$: Observable<TriggerValues> = new Observable();
168170

169171
public ngAfterContentInit(): void {
170172
this.allOptions$ = this.allOptionsList !== undefined ? queryListAndChanges$(this.allOptionsList) : EMPTY;
@@ -236,22 +238,32 @@ export class MultiSelectComponent<V> implements AfterContentInit, OnChanges {
236238

237239
private setTriggerLabel(): void {
238240
if (this.triggerLabelDisplayMode === TriggerLabelDisplayMode.Placeholder) {
239-
this.triggerLabel = this.placeholder;
241+
this.triggerValues$ = of({
242+
label: this.placeholder,
243+
selectedItemsCount: 0
244+
});
240245

241246
return;
242247
}
243248

244-
const selectedItems: SelectOptionComponent<V>[] | undefined = this.allOptionsList?.filter(item =>
245-
this.isSelectedItem(item)
246-
);
249+
this.triggerValues$ = this.allOptions$?.pipe(
250+
map(options => {
251+
const selectedItems: SelectOptionComponent<V>[] = options.filter(item => this.isSelectedItem(item));
247252

248-
this.selectedItemsCount = selectedItems?.length ?? 0;
249-
250-
// Trigger label is placeholder in case there is element selected on multiselect
251-
this.triggerLabel = this.selectedItemsCount === 0 ? this.placeholder : (selectedItems || [])[0]?.label;
253+
return {
254+
label: selectedItems.length === 0 ? this.placeholder : selectedItems[0]?.label,
255+
selectedItemsCount: selectedItems.length
256+
};
257+
})
258+
);
252259
}
253260
}
254261

262+
interface TriggerValues {
263+
label: string | undefined;
264+
selectedItemsCount: number;
265+
}
266+
255267
export const enum TriggerLabelDisplayMode {
256268
// These may be used as css classes
257269
Placeholder = 'placeholder-mode',

0 commit comments

Comments
 (0)