Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -254,4 +254,58 @@ describe('DynamicFormComponent', () => {
expect(wrapper?.clientHeight).toEqual(hint.offsetHeight);
});
});

describe('select multiple with "None of the above" option', () => {
const multiSelectIndex = '5';

beforeEach(() => {
component.createProfileForm([
...PROFILE_FORM,
{
question: 'Which statements are true?',
type: FormControlType.SELECT_MULTIPLE,
options: ['Option 1', 'Option 2', 'None of the above'],
validation: {
required: true,
},
},
]);
fixture.detectChanges();
});

it('should unselect "None of the above" when another option is selected after it', () => {
const multiSelect = component.getFormGroup(multiSelectIndex);

multiSelect.get('2')?.setValue(true);
expect(multiSelect.value).toEqual({ 0: false, 1: false, 2: true });

multiSelect.get('1')?.setValue(true);
expect(multiSelect.value).toEqual({ 0: false, 1: true, 2: false });
});

it('should unselect other options when "None of the above" is selected, and unselect "None of the above" when another option is selected again', () => {
const multiSelect = component.getFormGroup(multiSelectIndex);

multiSelect.get('0')?.setValue(true);
multiSelect.get('1')?.setValue(true);
expect(multiSelect.value).toEqual({ 0: true, 1: true, 2: false });

multiSelect.get('2')?.setValue(true);
expect(multiSelect.value).toEqual({ 0: false, 1: false, 2: true });

multiSelect.get('1')?.setValue(true);
expect(multiSelect.value).toEqual({ 0: false, 1: true, 2: false });
});

it('should unselect "None of the above" when another option is selected after form reset', () => {
const multiSelect = component.getFormGroup(multiSelectIndex);

component.formGroup.reset();
multiSelect.get('2')?.setValue(true);
multiSelect.get('1')?.setValue(true);

expect(multiSelect.get('1')?.value).toBeTrue();
expect(multiSelect.get('2')?.value).toBeFalse();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ import { ProfileValidators } from '../../pages/risk-assessment/profile-form/prof
import { DomSanitizer } from '@angular/platform-browser';
import { Subject } from 'rxjs/internal/Subject';
import { takeUntil } from 'rxjs/internal/operators/takeUntil';
import { startWith } from 'rxjs/internal/operators/startWith';
import { pairwise } from 'rxjs/internal/operators/pairwise';
@Component({
selector: 'app-dynamic-form',

Expand Down Expand Up @@ -141,7 +139,7 @@ export class DynamicFormComponent implements OnInit, OnDestroy {
const multiSelect = this.getMultiSelectGroup(question);
this.formGroup.addControl(index.toString(), multiSelect);
const noneKey = this.getNoneOptionIndex(question.options);
if (noneKey) {
if (noneKey !== null) {
this.applyNoneLogic(multiSelect, noneKey.toString());
}
} else {
Expand Down Expand Up @@ -224,38 +222,34 @@ export class DynamicFormComponent implements OnInit, OnDestroy {
});
}

private applyNoneLogic(control: AbstractControl, noneKey: string) {
control.valueChanges
.pipe(takeUntil(this.destroy$), startWith(control.value), pairwise())
.subscribe(([prev, curr]) => {
const changedKey = Object.keys(curr).find(
key => curr[key] === true && prev[key] === false
);

if (!changedKey) {
return;
}

if (changedKey == noneKey) {
const newValue = { ...curr };

Object.keys(newValue).forEach(key => {
if (key !== noneKey) {
newValue[key] = false;
}
});

control.setValue(newValue, { emitEvent: false });
} else if (curr[noneKey] === true) {
control.patchValue({ [noneKey]: false }, { emitEvent: false });
}
});
private applyNoneLogic(control: FormGroup, noneKey: string) {
Object.keys(control.controls).forEach(key => {
control
.get(key)
?.valueChanges.pipe(takeUntil(this.destroy$))
.subscribe(checked => {
if (!checked) {
return;
}

if (key === noneKey) {
Object.keys(control.controls).forEach(k => {
if (k !== noneKey) {
control.get(k)?.setValue(false, { emitEvent: false });
}
});
} else if (control.get(noneKey)?.value === true) {
control.get(noneKey)?.setValue(false, { emitEvent: false });
}
});
});
}
Comment on lines +225 to 246

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Optimizing control lookups and avoiding redundant setValue calls on controls that are already false prevents unnecessary form status/value updates and potential side effects.

  private applyNoneLogic(control: FormGroup, noneKey: string) {
    Object.keys(control.controls).forEach(key => {
      const childControl = control.get(key);
      childControl?.valueChanges
        .pipe(takeUntil(this.destroy$))
        .subscribe(checked => {
          if (!checked) {
            return;
          }

          if (key === noneKey) {
            Object.keys(control.controls).forEach(k => {
              if (k !== noneKey) {
                const otherControl = control.get(k);
                if (otherControl && otherControl.value !== false) {
                  otherControl.setValue(false, { emitEvent: false });
                }
              }
            });
          } else {
            const noneControl = control.get(noneKey);
            if (noneControl && noneControl.value === true) {
              noneControl.setValue(false, { emitEvent: false });
            }
          }
        });
    });
  }


private getNoneOptionIndex(options: OptionType[] | undefined): number | null {
if (!options) return null;
return options.findIndex(option =>
const index = options.findIndex(option =>
this.getOptionValue(option)?.toLowerCase().includes('none')
);
return index !== -1 ? index : null;
}
Comment on lines 248 to 254

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If getOptionValue returns a non-string value (such as an object or a number), calling .toLowerCase() directly on it will throw a runtime TypeError. It is safer to explicitly check if the value is a string before calling string methods.

  private getNoneOptionIndex(options: OptionType[] | undefined): number | null {
    if (!options) return null;
    const index = options.findIndex(option => {
      const value = this.getOptionValue(option);
      return typeof value === 'string' && value.toLowerCase().includes('none');
    });
    return index !== -1 ? index : null;
  }

}
Loading