fix: set shouldEmitValueChange to true on user change event for select (

#304)

* fix: skip only initial writeValue run for select

* fix: track performance improvement

* fix: select async patch should work

apps/app/src/app/pages/(home)/components/three-hundred.component.ts

@@ -10,10 +10,10 @@ import { ThreeHundredItemComponent } from './th-item.component';
 		class: 'grid gap-2 grid-cols-5 md:grid-cols-10',
 	},
 	template: `
-		@for (contributor of _contributors; track contributor) {
+		@for (contributor of _contributors; track $index) {
 			<spartan-th-item class="mb-2" [href]="'https://github.com/' + contributor">{{ contributor }}</spartan-th-item>
 		}
-		@for (item of _rest; track item) {
+		@for (item of _rest; track $index) {
 			<spartan-th-item-placeholder class="mb-2" />
 		}
 	`,

libs/ui/select/brain/src/lib/brn-select-option.directive.ts

@@ -31,7 +31,6 @@ export class BrnSelectOptionDirective implements FocusableOption, OnDestroy {
 		this._selectService.registerOption(this._cdkSelectOption);
 
 		toObservable(this._selectService.value)
-			.pipe(takeUntilDestroyed())
 			.subscribe((selectedValues: string | string[]) => {
 				if (Array.isArray(selectedValues)) {
 					const itemFound = (selectedValues as Array<unknown>).find((val) => val === this._cdkSelectOption.value);

libs/ui/select/brain/src/lib/brn-select.component.ts

@@ -198,11 +198,15 @@ export class BrnSelectComponent implements ControlValueAccessor, AfterContentIni
 			this.ngControl.valueAccessor = this;
 		}
 
-		// Watch for Listbox Selection Changes to trigger Collapse
+		// Watch for Listbox Selection Changes to trigger Collapse and Value Change
 		this._selectService.listBoxValueChangeEvent$.pipe(takeUntilDestroyed()).subscribe(() => {
 			if (!this._multiple()) {
 				this.close();
 			}
+
+			// we set shouldEmitValueChange to true because we want to propagate the value change
+			// as a result of user interaction
+			this._shouldEmitValueChange.set(true);
 		});
 
 		/**
@@ -211,24 +215,16 @@ export class BrnSelectComponent implements ControlValueAccessor, AfterContentIni
 		 * we dont propagate changes made from outside the component (ex. patch value or initial value from form control)
 		 */
 		toObservable(this._selectService.value)
-			.pipe(
-				filter(() => {
-					const shouldEmitValueChange = this._shouldEmitValueChange();
-					this._shouldEmitValueChange.set(true);
-					return shouldEmitValueChange;
-				}),
-				takeUntilDestroyed(),
-			)
-			.subscribe((value) => this._onChange(value ?? null));
-
-		toObservable(this.dir)
-			.pipe(takeUntilDestroyed())
-			.subscribe(() =>
-				this._selectService.state.update((state) => ({
-					...state,
-					dir: this.dir(),
-				})),
-			);
+			.subscribe((value) => {
+				if (this._shouldEmitValueChange()) {
+					this._onChange(value ?? null)
+				}
+				this._shouldEmitValueChange.set(true);
+			});
+
+		toObservable(this.dir).subscribe((dir) =>
+			this._selectService.state.update((state) => ({ ...state, dir })),
+		);
 	}
 
 	public ngAfterContentInit(): void {
@@ -291,7 +287,7 @@ export class BrnSelectComponent implements ControlValueAccessor, AfterContentIni
 	}
 
 	public writeValue(value: any): void {
-		// 'shouldEmitValueChange' ensures we don't propagate changes when we recieve value from from form control
+		// 'shouldEmitValueChange' ensures we don't propagate changes when we receive value from form control
 		// set to false until next value change and then reset back to true
 		this._shouldEmitValueChange.set(false);
 		this._selectService.setInitialSelectedOptions(value);

libs/ui/select/brain/src/lib/tests/select-reactive-form.ts

@@ -96,6 +96,46 @@ export class SelectSingleValueWithInitialValueTestComponent {
 	form = new FormGroup({ fruit: new FormControl('apple') });
 }
 
+@Component({
+	standalone: true,
+	imports: [CommonModule, FormsModule, ReactiveFormsModule, BrnSelectImports],
+	// eslint-disable-next-line @angular-eslint/component-selector
+	selector: 'select-reactive-field-fixture',
+	template: `
+		<form [formGroup]="form">
+			<brn-select class="w-56" formControlName="fruit" placeholder="Select a Fruit">
+				<button brnSelectTrigger data-testid="brn-select-trigger">
+					<brn-select-value data-testid="brn-select-value" />
+				</button>
+				<brn-select-content class="w-56" data-testid="brn-select-content">
+					<label brnSelectLabel>Fruits</label>
+					<div brnOption value="apple">Apple</div>
+					<div brnOption value="banana">Banana</div>
+					<div brnOption value="blueberry">Blueberry</div>
+					<div brnOption value="grapes">Grapes</div>
+					<div brnOption value="pineapple">Pineapple</div>
+					<div>Clear</div>
+				</brn-select-content>
+			</brn-select>
+			@if (form.controls.fruit.invalid && form.controls.fruit.touched) {
+				<span class="text-destructive">Required</span>
+			}
+		</form>
+	`,
+})
+export class SelectSingleValueWithInitialValueWithAsyncUpdateTestComponent {
+	form = new FormGroup({ fruit: new FormControl('apple') });
+
+	constructor() {
+		// queueMicrotask(() => {
+		// 	this.form.patchValue({ fruit: 'apple' });
+		// });
+		setTimeout(() => {
+			this.form.patchValue({ fruit: 'apple' });
+		});
+	}
+}
+
 @Component({
 	standalone: true,
 	imports: [CommonModule, FormsModule, ReactiveFormsModule, BrnSelectImports],

libs/ui/select/brain/src/lib/tests/select-single-mode.spec.ts

@@ -1,7 +1,7 @@
 import { Validators } from '@angular/forms';
 import { render, screen } from '@testing-library/angular';
 import userEvent from '@testing-library/user-event';
-import { SelectSingleValueTestComponent, SelectSingleValueWithInitialValueTestComponent } from './select-reactive-form';
+import { SelectSingleValueTestComponent, SelectSingleValueWithInitialValueTestComponent, SelectSingleValueWithInitialValueWithAsyncUpdateTestComponent } from './select-reactive-form';
 import { getFormControlStatus, getFormValidationClasses } from './utils';
 
 describe('Brn Select Component in single-mode', () => {
@@ -29,6 +29,17 @@ describe('Brn Select Component in single-mode', () => {
 		};
 	};
 
+
+	const setupWithFormValidationAndInitialValueAndAsyncUpdate = async () => {
+		const { fixture } = await render(SelectSingleValueWithInitialValueWithAsyncUpdateTestComponent);
+		return {
+			user: userEvent.setup(),
+			fixture,
+			trigger: screen.getByTestId('brn-select-trigger'),
+			value: screen.getByTestId('brn-select-value'),
+		};
+	};
+
 	describe('form validation - single mode', () => {
 		it('should reflect correct formcontrol status and value with no initial value', async () => {
 			const { fixture, trigger, value } = await setupWithFormValidation();
@@ -174,6 +185,61 @@ describe('Brn Select Component in single-mode', () => {
 			expect(getFormValidationClasses(trigger)).toStrictEqual(afterSelectionExpected);
 
 			expect(cmpInstance.form?.get('fruit')?.value).toEqual('banana');
+			expect(screen.getByTestId('brn-select-value').textContent?.trim()).toBe('Banana');
+		});
+
+		it('should reflect correct formcontrol status after first user selection with initial value and async update', async () => {
+			const { user, trigger, fixture, value } = await setupWithFormValidationAndInitialValueAndAsyncUpdate();
+			const cmpInstance = fixture.componentInstance as SelectSingleValueWithInitialValueTestComponent;
+
+			expect(value.textContent?.trim()).toBe(INITIAL_VALUE_TEXT);
+			expect(cmpInstance.form?.get('fruit')?.value).toEqual(INITIAL_VALUE);
+
+			const expected = {
+				untouched: true,
+				touched: false,
+				valid: true,
+				invalid: false,
+				pristine: true,
+				dirty: false,
+			};
+
+			expect(getFormControlStatus(cmpInstance.form?.get('fruit'))).toStrictEqual(expected);
+			expect(getFormValidationClasses(trigger)).toStrictEqual(expected);
+
+			// Open Select
+			await user.click(trigger);
+
+			const afterOpenExpected = {
+				untouched: true,
+				touched: false,
+				valid: true,
+				invalid: false,
+				pristine: true,
+				dirty: false,
+			};
+
+			expect(getFormControlStatus(cmpInstance.form?.get('fruit'))).toStrictEqual(afterOpenExpected);
+			expect(getFormValidationClasses(trigger)).toStrictEqual(afterOpenExpected);
+
+			// Select option
+			const options = await screen.getAllByRole('option');
+			await user.click(options[1]);
+
+			const afterSelectionExpected = {
+				untouched: false,
+				touched: true,
+				valid: true,
+				invalid: false,
+				pristine: false,
+				dirty: true,
+			};
+
+			expect(getFormControlStatus(cmpInstance.form?.get('fruit'))).toStrictEqual(afterSelectionExpected);
+			expect(getFormValidationClasses(trigger)).toStrictEqual(afterSelectionExpected);
+
+			expect(cmpInstance.form?.get('fruit')?.value).toEqual('banana');
+			expect(value.textContent?.trim()).toBe('Banana');
 		});
 	});