Add registerOption implementation to Combobox.

packages/nimble-components/src/combobox/index.ts

@@ -17,10 +17,14 @@ import { iconExclamationMarkTag } from '../icons/exclamation-mark';
 
 import { styles } from './styles';
 import type { ErrorPattern } from '../patterns/error/types';
-import type { DropdownPattern } from '../patterns/dropdown/types';
+import type {
+    DropdownPattern,
+    ListOptionOwner
+} from '../patterns/dropdown/types';
 import { DropdownAppearance } from '../patterns/dropdown/types';
 import type { AnchoredRegion } from '../anchored-region';
 import { template } from './template';
+import type { ListOption } from '../list-option';
 
 declare global {
     interface HTMLElementTagNameMap {
@@ -33,7 +37,7 @@ declare global {
  */
 export class Combobox
     extends FoundationCombobox
-    implements DropdownPattern, ErrorPattern {
+    implements DropdownPattern, ErrorPattern, ListOptionOwner {
     @attr
     public appearance: DropdownAppearance = DropdownAppearance.underline;
 
@@ -207,6 +211,23 @@ export class Combobox
         return returnValue;
     }
 
+    /**
+     * @internal
+     */
+    public registerOption(option: ListOption): void {
+        if (this.options.includes(option)) {
+            return;
+        }
+
+        // Adding an option to the end, ultimately, isn't the correct
+        // thing to do, as this will mean the option's index in the options,
+        // at least temporarily, does not match the DOM order. However, it
+        // is expected that a successive run of `slottedOptionsChanged` will
+        // correct this order issue. See 'https://github.com/ni/nimble/issues/1915'
+        // for more info.
+        this.options.push(option);
+    }
+
     protected override focusAndScrollOptionIntoView(): void {
         if (this.open) {
             super.focusAndScrollOptionIntoView();

packages/nimble-components/src/combobox/tests/combobox.spec.ts

@@ -2,7 +2,7 @@ import { html, repeat } from '@microsoft/fast-element';
 import { keyArrowDown, keyEnter } from '@microsoft/fast-web-utilities';
 import { fixture, Fixture } from '../../utilities/tests/fixture';
 import { Combobox, comboboxTag } from '..';
-import { listOptionTag } from '../../list-option';
+import { ListOption, listOptionTag } from '../../list-option';
 import { ComboboxAutocomplete } from '../types';
 import { waitForUpdatesAsync } from '../../testing/async-helpers';
 import {
@@ -127,6 +127,33 @@ describe('Combobox', () => {
         await disconnect();
     });
 
+    it('option added directly to DOM synchronously registers with Combobox', async () => {
+        const { element, connect, disconnect } = await setup();
+        await connect();
+        element.selectedIndex = 0;
+        await waitForUpdatesAsync();
+        const newOption = new ListOption('foo', 'foo');
+        const registerOptionSpy = spyOn(
+            element,
+            'registerOption'
+        ).and.callThrough();
+        registerOptionSpy.calls.reset();
+        element.insertBefore(newOption, element.options[0]!);
+
+        expect(registerOptionSpy.calls.count()).toBe(1);
+        expect(element.options).toContain(newOption);
+        // The below assertion is simply showing a current expected, but
+        // incorrect, behavior, as the new option was added before the currently
+        // selected one. See 'https://github.com/ni/nimble/issues/1915'
+        // for details.
+        expect(element.selectedIndex).toBe(0);
+        await waitForUpdatesAsync();
+        // This assertion shows that after 'slottedOptionsChanged' runs, the
+        // 'selectedIndex' state has been corrected to expected DOM order.
+        expect(element.selectedIndex).toBe(1);
+        await disconnect();
+    });
+
     const ariaTestData: {
         attrName: string,
         propSetter: (x: Combobox, value: string) => void

packages/nimble-components/src/select/index.ts

@@ -681,6 +681,12 @@ export class Select
             return;
         }
 
+        // Adding an option to the end, ultimately, isn't the correct
+        // thing to do, as this will mean the option's index in the options,
+        // at least temporarily, does not match the DOM order. However, it
+        // is expected that a successive run of `slottedOptionsChanged` will
+        // correct this order issue. See 'https://github.com/ni/nimble/issues/1915'
+        // for more info.
         this.options.push(option);
     }
 

packages/nimble-components/src/select/tests/select.spec.ts

@@ -295,9 +295,14 @@ describe('Select', () => {
 
         expect(registerOptionSpy.calls.count()).toBe(1);
         expect(element.options).toContain(newOption);
+        // The below assertion is simply showing a current expected, but
+        // incorrect, behavior. See 'https://github.com/ni/nimble/issues/1915'
+        // for details.
         expect(element.selectedIndex).toBe(0);
         await waitForUpdatesAsync();
         expect(element.value).toBe('one');
+        // This assertion shows that after 'slottedOptionsChanged' runs, the
+        // 'selectedIndex' state has been corrected to expected DOM order.
         expect(element.selectedIndex).toBe(1);
         await disconnect();
     });