feat(combo): Added defaultValue support (#1494)

* refactor(combo): Synchronous form flow for combo

This commit migrates most of the properties related
to form submission to be sync instead of the old
`@watch` handlers.

Additionally there are several other changes in
order to "modernize" the combo code base:

* moved any `@property` decorators over the setters
* deletegated the click handlers of the combo items
to a single one on the parent list
* removed some old code which was no longer needed

* feat(combo): Added defaultValue support

* fix(combo): Form reset hooks

Author: rkaraivanov

src/components/combo/combo-item.ts

@@ -3,7 +3,6 @@ import { property } from 'lit/decorators.js';
 
 import { themes } from '../../theming/theming-decorator.js';
 import IgcCheckboxComponent from '../checkbox/checkbox.js';
-import { watch } from '../common/decorators/watch.js';
 import { registerComponent } from '../common/definitions/register.js';
 import { all } from '../dropdown/themes/item.js';
 import { styles as shared } from '../dropdown/themes/shared/item/dropdown-item.common.css.js';
@@ -21,16 +20,25 @@ export default class IgcComboItemComponent extends LitElement {
   }
 
   private _internals: ElementInternals;
+  private _selected = false;
 
   @property({ attribute: false })
   public index!: number;
 
   /**
    * Determines whether the item is selected.
    * @attr selected
+   * @default false
    */
   @property({ type: Boolean, reflect: true })
-  public selected = false;
+  public set selected(value: boolean) {
+    this._selected = value;
+    this._internals.ariaSelected = this._selected.toString();
+  }
+
+  public get selected(): boolean {
+    return this._selected;
+  }
 
   /**
    * Determines whether the item is active.
@@ -42,19 +50,15 @@ export default class IgcComboItemComponent extends LitElement {
    * Determines whether the item is active.
    * @attr hide-checkbox
    */
-  @property({ attribute: 'hide-checkbox', type: Boolean, reflect: false })
+  @property({ type: Boolean, attribute: 'hide-checkbox' })
   public hideCheckbox = false;
 
-  @watch('selected')
-  protected selectedChange() {
-    this._internals.ariaSelected = `${this.selected}`;
-  }
-
   constructor() {
     super();
 
     this._internals = this.attachInternals();
     this._internals.role = 'option';
+    this._internals.ariaSelected = 'false';
   }
 
   public override connectedCallback() {
@@ -63,13 +67,15 @@ export default class IgcComboItemComponent extends LitElement {
   }
 
   private renderCheckbox() {
-    return html`<section part="prefix">
-      <igc-checkbox
-        inert
-        ?checked=${this.selected}
-        exportparts="control: checkbox, indicator: checkbox-indicator, checked"
-      ></igc-checkbox>
-    </section>`;
+    return html`
+      <section part="prefix">
+        <igc-checkbox
+          .inert=${true}
+          ?checked=${this.selected}
+          exportparts="control: checkbox, indicator: checkbox-indicator, checked"
+        ></igc-checkbox>
+      </section>
+    `;
   }
 
   protected override render() {

src/components/combo/combo.spec.ts

@@ -2,6 +2,7 @@ import { elementUpdated, expect, fixture, html } from '@open-wc/testing';
 import { spy } from 'sinon';
 
 import { defineComponents } from '../common/definitions/defineComponents.js';
+import { first } from '../common/util.js';
 import {
   type ValidationContainerTestsParams,
   createFormAssociatedTestBed,
@@ -335,7 +336,7 @@ describe('Combo', () => {
       const cityNames: string[] = [];
 
       items(combo).forEach((item) => {
-        cityNames.push(item.textContent!);
+        cityNames.push(item.innerText);
       });
 
       cities.forEach((city) => {
@@ -373,7 +374,7 @@ describe('Combo', () => {
       });
 
       const selected = items(combo).find((item) => item.selected);
-      expect(selected?.textContent).to.equal(item[combo.displayKey!]);
+      expect(selected?.innerText).to.equal(item[combo.displayKey!]);
 
       combo.deselect([item[combo.valueKey!]]);
 
@@ -401,7 +402,7 @@ describe('Combo', () => {
       });
 
       const selected = items(combo).find((item) => item.selected);
-      expect(selected?.textContent).to.equal(item[combo.displayKey!]);
+      expect(selected?.innerText).to.equal(item[combo.displayKey!]);
 
       combo.deselect([item]);
 
@@ -807,7 +808,7 @@ describe('Combo', () => {
       await filterCombo('sof');
 
       expect(items(combo).length).to.equal(1);
-      expect(items(combo)[0].textContent).to.equal('Sofia');
+      expect(items(combo)[0].innerText).to.equal('Sofia');
     });
 
     it('should select the first matched item upon pressing enter after search', async () => {
@@ -975,7 +976,7 @@ describe('Combo', () => {
       const selected = items(combo).filter((i) => i.selected);
 
       expect(selected.length).to.equal(1);
-      expect(selected[0].textContent).to.equal(match?.name);
+      expect(selected[0].innerText).to.equal(match?.name);
     });
 
     it('should deselect a single item using valueKey as argument with the Selection API', async () => {
@@ -1017,7 +1018,7 @@ describe('Combo', () => {
       const selected = items(combo).filter((i) => i.selected);
 
       expect(selected.length).to.equal(1);
-      expect(selected[0].textContent).to.equal(item?.name);
+      expect(selected[0].innerText).to.equal(item?.name);
     });
 
     it('should deselect the item passed as argument with the Selection API', async () => {
@@ -1056,7 +1057,7 @@ describe('Combo', () => {
 
       // Verify we can only see one item in the list
       expect(items(combo).length).to.equal(1);
-      expect(items(combo)[0].textContent).to.equal('Sofia');
+      expect(items(combo)[0].innerText).to.equal('Sofia');
 
       // Select an item not visible in the list using the API
       const selection = 'US01';
@@ -1079,7 +1080,7 @@ describe('Combo', () => {
       expect(selected.length).to.equal(1);
 
       // It should match the one selected via the API
-      expect(selected[0].textContent).to.equal('New York');
+      expect(selected[0].innerText).to.equal('New York');
     });
 
     it('should deselect item(s) even if the list of items has been filtered', async () => {
@@ -1097,7 +1098,7 @@ describe('Combo', () => {
       expect(selected.length).to.equal(1);
 
       // It should match the one selected via the API
-      expect(selected[0].textContent).to.equal('New York');
+      expect(selected[0].innerText).to.equal('New York');
       expect(combo.value[0]).to.equal(selection);
 
       // Filter the list of items
@@ -1108,7 +1109,7 @@ describe('Combo', () => {
 
       // Verify we can only see one item in the list
       expect(items(combo).length).to.equal(1);
-      expect(items(combo)[0].textContent).to.equal('Sofia');
+      expect(items(combo)[0].innerText).to.equal('Sofia');
 
       // Deselect the previously selected item while the list is filtered
       combo.deselect(selection);
@@ -1258,16 +1259,14 @@ describe('Combo', () => {
       expect(spec.element.form).to.equal(spec.form);
     });
 
-    it('is not associated on submit if no value', async () => {
-      // FIXME: The combo value setter does not update the form state in a synchronous manner
-      // FIXME: delegating to a @watch callback.
-      await spec.setProperties({ value: [] });
+    it('is not associated on submit if no value', () => {
+      spec.setProperties({ value: [] });
       spec.assertSubmitHasValue(null);
     });
 
-    it('is associated on submit with value-key (single)', async () => {
-      await spec.setProperties({ singleSelect: true });
-      await spec.setProperties({ value: ['BG01', 'BG02'] });
+    it('is associated on submit with value-key (single)', () => {
+      spec.setProperties({ singleSelect: true });
+      spec.setProperties({ value: ['BG01', 'BG02'] });
       spec.assertSubmitHasValue('BG01');
     });
 
@@ -1276,64 +1275,62 @@ describe('Combo', () => {
       spec.assertSubmitHasValues(['BG01', 'BG02']);
     });
 
-    it('is associated on submit without value-key (single)', async () => {
+    it('is associated on submit without value-key (single)', () => {
       const [first, second, _] = cities;
 
-      await spec.setProperties({ valueKey: undefined, singleSelect: true });
+      spec.setProperties({ valueKey: undefined, singleSelect: true });
       spec.element.select([first, second]);
-      await elementUpdated(spec.element);
 
       expect(spec.formData.getAll(spec.element.name)).lengthOf(1);
       spec.assertSubmitPasses();
     });
 
-    it('is associated on submit without value-key (multiple)', async () => {
+    it('is associated on submit without value-key (multiple)', () => {
       const [first, second, _] = cities;
 
-      await spec.setProperties({ valueKey: undefined });
+      spec.setProperties({ valueKey: undefined });
       spec.element.select([first, second]);
-      await elementUpdated(spec.element);
 
       expect(spec.formData.getAll(spec.element.name)).lengthOf(2);
       spec.assertSubmitPasses();
     });
 
-    it('is correctly reset on form reset (multiple)', async () => {
+    it('is correctly reset on form reset (multiple)', () => {
       const initial = spec.element.value;
 
-      await spec.setAttributes({ value: JSON.stringify(['BG01', 'BG02']) });
-      await spec.setProperties({ value: [] });
+      spec.setAttributes({ value: JSON.stringify(['BG01', 'BG02']) });
+      spec.setProperties({ value: [] });
 
       spec.reset();
       expect(spec.element.value).to.eql(initial);
     });
 
-    it('is correctly reset on form reset (single)', async () => {
+    it('is correctly reset on form reset (single)', () => {
       // Initial value is a multiple array. The combo defaults to the first item
-      const initial = [spec.element.value[0]];
+      const initial = [first(spec.element.value)];
 
-      await spec.setProperties({ singleSelect: true });
-      await spec.setProperties({ value: ['US01'] });
+      spec.setProperties({ singleSelect: true });
+      spec.setProperties({ value: ['US01'] });
 
       expect(spec.element.value).to.eql(['US01']);
 
       spec.reset();
       expect(spec.element.value).to.eql(initial);
     });
 
-    it('should reset to the new default value after setAttribute() call (multiple)', async () => {
-      await spec.setAttributes({ value: JSON.stringify(['US01', 'US02']) });
-      await spec.setProperties({ value: [] });
+    it('should reset to the new default value after setAttribute() call (multiple)', () => {
+      spec.setAttributes({ value: JSON.stringify(['US01', 'US02']) });
+      spec.setProperties({ value: [] });
 
       spec.reset();
       expect(spec.element.value).to.eql(['US01', 'US02']);
       spec.assertSubmitHasValues(spec.element.value);
     });
 
-    it('should reset to the new default value after setAttribute() call (single)', async () => {
-      await spec.setProperties({ singleSelect: true });
-      await spec.setAttributes({ value: JSON.stringify(['US01']) });
-      await spec.setProperties({ value: [] });
+    it('should reset to the new default value after setAttribute() call (single)', () => {
+      spec.setProperties({ singleSelect: true });
+      spec.setAttributes({ value: JSON.stringify(['US01']) });
+      spec.setProperties({ value: [] });
 
       spec.reset();
 
@@ -1349,11 +1346,11 @@ describe('Combo', () => {
       expect(spec.element.disabled).to.be.false;
     });
 
-    it('fulfils required constraint', async () => {
-      await spec.setProperties({ value: [], required: true });
+    it('fulfils required constraint', () => {
+      spec.setProperties({ value: [], required: true });
       spec.assertSubmitFails();
 
-      await spec.setProperties({ value: ['BG01', 'BG02'] });
+      spec.setProperties({ value: ['BG01', 'BG02'] });
       spec.assertSubmitPasses();
     });
 
@@ -1369,7 +1366,40 @@ describe('Combo', () => {
   describe('defaultValue', () => {
     const value = ['BG01', 'BG02'];
 
-    describe('Form integration', () => {
+    describe('Form integration (single)', () => {
+      const spec = createFormAssociatedTestBed<IgcComboComponent<City>>(html`
+        <igc-combo
+          name="combo"
+          .data=${cities}
+          .defaultValue=${value}
+          single-select
+          value-key="id"
+          display-key="name"
+        ></igc-combo>
+      `);
+
+      beforeEach(async () => {
+        await spec.setup(IgcComboComponent.tagName);
+      });
+
+      it('correct initial state', () => {
+        spec.assertIsPristine();
+        expect(spec.element.value).to.eql([first(value)]);
+      });
+
+      it('is correctly submitted', () => {
+        spec.assertSubmitHasValue(first(value));
+      });
+
+      it('is correctly reset', () => {
+        spec.setProperties({ value: [] });
+        spec.reset();
+
+        expect(spec.element.value).to.eql([first(value)]);
+      });
+    });
+
+    describe('Form integration (multiple)', () => {
       const spec = createFormAssociatedTestBed<IgcComboComponent<City>>(html`
         <igc-combo
           name="combo"
@@ -1393,8 +1423,8 @@ describe('Combo', () => {
         spec.assertSubmitHasValues(value);
       });
 
-      it('is correctly reset', async () => {
-        await spec.setProperties({ value: [] });
+      it('is correctly reset', () => {
+        spec.setProperties({ value: [] });
         spec.reset();
 
         expect(spec.element.value).to.eql(value);
@@ -1407,6 +1437,7 @@ describe('Combo', () => {
           name="combo"
           .data=${cities}
           .defaultValue=${[]}
+          required
           value-key="id"
           display-key="name"
         ></igc-combo>
@@ -1421,8 +1452,8 @@ describe('Combo', () => {
         spec.assertSubmitFails();
       });
 
-      it('passes required validation', async () => {
-        await spec.setProperties({ defaultValue: value });
+      it('passes required validation', () => {
+        spec.setProperties({ defaultValue: value });
         spec.assertIsPristine();
 
         spec.assertSubmitHasValues(value);
@@ -1431,7 +1462,7 @@ describe('Combo', () => {
   });
 
   describe('Validation message slots', () => {
-    it('', async () => {
+    it('', () => {
       const testParameters: ValidationContainerTestsParams<IgcComboComponent>[] =
         [
           { slots: ['valueMissing'], props: { required: true } }, // value-missing slot