From fbc3739a6c79cea6cd5e585ff06fdc77bdd78a5d Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Mon, 8 Apr 2024 10:53:50 -0500
Subject: [PATCH 01/18] Working implementation.

---
 .../nimble-components/src/select/index.ts     | 26 ++++++++++++++++++-
 .../nimble-components/src/select/styles.ts    |  4 +++
 .../src/select/tests/select.stories.ts        |  5 +++-
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
index 5cd9a29f4b..0891536f98 100644
--- a/packages/nimble-components/src/select/index.ts
+++ b/packages/nimble-components/src/select/index.ts
@@ -4,7 +4,8 @@ import {
     html,
     observable,
     Observable,
-    volatile
+    volatile,
+    when
 } from '@microsoft/fast-element';
 import {
     AnchoredRegion,
@@ -42,6 +43,7 @@ import { ListOption } from '../list-option';
 import { FilterMode } from './types';
 import { diacriticInsensitiveStringNormalizer } from '../utilities/models/string-normalizers';
 import { FormAssociatedSelect } from './models/select-form-associated';
+import { iconCircleXTag } from '../icons/circle-x';
 
 declare global {
     interface HTMLElementTagNameMap {
@@ -90,6 +92,9 @@ export class Select
     @attr({ attribute: 'filter-mode' })
     public filterMode: FilterMode = FilterMode.none;
 
+    @attr({ attribute: 'clearable', mode: 'boolean' })
+    public clearable = false;
+
     /**
      * @internal
      */
@@ -193,6 +198,7 @@ export class Select
     private _value = '';
     private forcedPosition = false;
     private indexWhenOpened?: number;
+    private placeholderOption?: ListboxOption;
 
     /**
      * @internal
@@ -301,6 +307,7 @@ export class Select
             notifier.subscribe(this, 'hidden');
             notifier.subscribe(this, 'disabled');
         });
+        this.findAndCachePlaceholderOption();
         this.setProxyOptions();
         this.updateValue();
         // We need to force an update to the filteredOptions observable
@@ -437,6 +444,16 @@ export class Select
         );
     }
 
+    public clearClickHandler(e: MouseEvent): void {
+        if (this.placeholderOption) {
+            this.selectedIndex = this.options.indexOf(this.placeholderOption);
+            this.committedSelectedOption = this.placeholderOption;
+        } else {
+            this.selectedIndex = -1;
+        }
+        e.stopPropagation();
+    }
+
     /**
      * @internal
      */
@@ -940,6 +957,10 @@ export class Select
         });
     }
 
+    private findAndCachePlaceholderOption(): void {
+        this.placeholderOption = this.options.find(o => o.selected && o.disabled && o.hidden);
+    }
+
     private filterChanged(): void {
         this.filterOptions();
     }
@@ -985,6 +1006,9 @@ const nimbleSelect = Select.compose<SelectOptions>({
             class="error-icon"
         ></${iconExclamationMarkTag}>
         ${errorTextTemplate}
+        ${when(x => x.clearable && !x.displayPlaceholder && x.selectedIndex >= 0, html<Select>`
+            <${iconCircleXTag} class="clear-icon" @click="${(x, c) => x.clearClickHandler(c.event as MouseEvent)}"></${iconCircleXTag}>
+        `)}
     `
 });
 
diff --git a/packages/nimble-components/src/select/styles.ts b/packages/nimble-components/src/select/styles.ts
index d543d15819..a9f34ad852 100644
--- a/packages/nimble-components/src/select/styles.ts
+++ b/packages/nimble-components/src/select/styles.ts
@@ -35,6 +35,10 @@ export const styles = css`
     }
 
     [part='indicator'] {
+        order: 4;
+    }
+
+    .clear-icon {
         order: 3;
     }
 
diff --git a/packages/nimble-components/src/select/tests/select.stories.ts b/packages/nimble-components/src/select/tests/select.stories.ts
index 70ae09b458..04a9538d23 100644
--- a/packages/nimble-components/src/select/tests/select.stories.ts
+++ b/packages/nimble-components/src/select/tests/select.stories.ts
@@ -21,6 +21,7 @@ interface SelectArgs {
     appearance: string;
     filterMode: keyof typeof FilterMode;
     placeholder: boolean;
+    clearable: boolean;
 }
 
 interface OptionArgs {
@@ -96,6 +97,7 @@ const metadata: Meta<SelectArgs> = {
             ?error-visible="${x => x.errorVisible}"
             error-text="${x => x.errorText}"
             ?disabled="${x => x.disabled}"
+            ?clearable="${x => x.clearable}"
             position="${x => x.dropDownPosition}"
             appearance="${x => x.appearance}"
             filter-mode="${x => (x.filterMode === 'none' ? undefined : x.filterMode)}"
@@ -165,7 +167,8 @@ const metadata: Meta<SelectArgs> = {
         dropDownPosition: 'below',
         appearance: DropdownAppearance.underline,
         optionsType: ExampleOptionsType.simpleOptions,
-        placeholder: false
+        placeholder: false,
+        clearable: false
     }
 };
 

From 87173a3ee9bb9e4e11d05b128a4150ed02d18638 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Fri, 12 Apr 2024 14:37:55 -0500
Subject: [PATCH 02/18] Update to spec.

---
 .../src/patterns/dropdown/styles.ts               |  2 +-
 packages/nimble-components/src/select/index.ts    | 15 +++++++--------
 packages/nimble-components/src/select/styles.ts   |  5 ++++-
 packages/nimble-components/src/select/template.ts | 11 +++++++++++
 4 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/packages/nimble-components/src/patterns/dropdown/styles.ts b/packages/nimble-components/src/patterns/dropdown/styles.ts
index 194bb55ad3..677283f590 100644
--- a/packages/nimble-components/src/patterns/dropdown/styles.ts
+++ b/packages/nimble-components/src/patterns/dropdown/styles.ts
@@ -188,7 +188,7 @@ export const styles = css`
 
     .indicator {
         flex: none;
-        margin-inline-start: 1em;
+        margin-left: ${smallPadding};
         padding-right: 8px;
         display: flex;
         justify-content: center;
diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
index 0891536f98..51ab62c762 100644
--- a/packages/nimble-components/src/select/index.ts
+++ b/packages/nimble-components/src/select/index.ts
@@ -44,6 +44,8 @@ import { FilterMode } from './types';
 import { diacriticInsensitiveStringNormalizer } from '../utilities/models/string-normalizers';
 import { FormAssociatedSelect } from './models/select-form-associated';
 import { iconCircleXTag } from '../icons/circle-x';
+import { buttonTag } from '../button';
+import { iconXmarkTag } from '../icons/xmark';
 
 declare global {
     interface HTMLElementTagNameMap {
@@ -245,10 +247,9 @@ export class Select
                 );
             }
             Observable.notify(this, 'value');
-            if (this.collapsible) {
-                Observable.notify(this, 'displayValue');
-            }
         }
+
+        Observable.notify(this, 'displayValue');
     }
 
     /**
@@ -342,8 +343,8 @@ export class Select
 
         this.open = this.collapsible && !this.open;
 
-        if (!this.open && this.indexWhenOpened !== this.selectedIndex) {
-            this.updateValue(true);
+        if (!this.open) {
+            this.updateValue(this.indexWhenOpened !== this.selectedIndex);
         }
     }
 
@@ -919,6 +920,7 @@ export class Select
     private updateValue(shouldEmit?: boolean): void {
         if (this.$fastController.isConnected) {
             this.value = this.firstSelectedOption?.value ?? '';
+            this.committedSelectedOption = this.firstSelectedOption;
         }
 
         if (shouldEmit) {
@@ -1006,9 +1008,6 @@ const nimbleSelect = Select.compose<SelectOptions>({
             class="error-icon"
         ></${iconExclamationMarkTag}>
         ${errorTextTemplate}
-        ${when(x => x.clearable && !x.displayPlaceholder && x.selectedIndex >= 0, html<Select>`
-            <${iconCircleXTag} class="clear-icon" @click="${(x, c) => x.clearClickHandler(c.event as MouseEvent)}"></${iconCircleXTag}>
-        `)}
     `
 });
 
diff --git a/packages/nimble-components/src/select/styles.ts b/packages/nimble-components/src/select/styles.ts
index a9f34ad852..c043369bc1 100644
--- a/packages/nimble-components/src/select/styles.ts
+++ b/packages/nimble-components/src/select/styles.ts
@@ -38,8 +38,11 @@ export const styles = css`
         order: 4;
     }
 
-    .clear-icon {
+    .clear-button {
         order: 3;
+        width: auto;
+        height: auto;
+        margin-left: ${smallPadding};
     }
 
     .error-icon {
diff --git a/packages/nimble-components/src/select/template.ts b/packages/nimble-components/src/select/template.ts
index 3266934432..703b321cd9 100644
--- a/packages/nimble-components/src/select/template.ts
+++ b/packages/nimble-components/src/select/template.ts
@@ -22,6 +22,8 @@ import {
     filterSearchLabel
 } from '../label-provider/core/label-tokens';
 import { FilterMode } from './types';
+import { buttonTag } from '../button';
+import { iconXmarkTag } from '../icons/xmark';
 
 /* eslint-disable @typescript-eslint/indent */
 // prettier-ignore
@@ -65,6 +67,15 @@ SelectOptions
                     <div class="selected-value ${x => (x.displayPlaceholder ? 'placeholder' : '')}" part="selected-value" ${overflow('hasOverflow')} title=${x => (x.hasOverflow && x.displayValue ? x.displayValue : null)}>
                         <slot name="selected-value">${x => x.displayValue}</slot>
                     </div>
+                    ${when(x => x.clearable && !x.displayPlaceholder && x.selectedIndex >= 0, html<Select>`
+                        <${buttonTag} 
+                            class="clear-button"
+                            content-hidden
+                            appearance="ghost"
+                            @click="${(x, c) => x.clearClickHandler(c.event as MouseEvent)}">
+                            <${iconXmarkTag} slot="start"></${iconXmarkTag}>
+                        </${buttonTag}>
+                    `)}
                     <div aria-hidden="true" class="indicator" part="indicator">
                         <slot name="indicator">
                             ${definition.indicator || ''}

From f5d2430f4663376dbeea416949b48661e549c004 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Tue, 16 Apr 2024 10:59:39 -0500
Subject: [PATCH 03/18] Cleanup, unit tests, and matrix tests.

---
 .../nimble-components/src/select/index.ts     | 10 +-
 .../src/select/testing/select.pageobject.ts   | 44 ++++++++-
 .../src/select/tests/select-matrix.stories.ts | 13 ++-
 .../src/select/tests/select.spec.ts           | 91 ++++++++++++++++++-
 4 files changed, 142 insertions(+), 16 deletions(-)

diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
index 51ab62c762..43936b6496 100644
--- a/packages/nimble-components/src/select/index.ts
+++ b/packages/nimble-components/src/select/index.ts
@@ -4,8 +4,7 @@ import {
     html,
     observable,
     Observable,
-    volatile,
-    when
+    volatile
 } from '@microsoft/fast-element';
 import {
     AnchoredRegion,
@@ -43,9 +42,6 @@ import { ListOption } from '../list-option';
 import { FilterMode } from './types';
 import { diacriticInsensitiveStringNormalizer } from '../utilities/models/string-normalizers';
 import { FormAssociatedSelect } from './models/select-form-associated';
-import { iconCircleXTag } from '../icons/circle-x';
-import { buttonTag } from '../button';
-import { iconXmarkTag } from '../icons/xmark';
 
 declare global {
     interface HTMLElementTagNameMap {
@@ -960,7 +956,9 @@ export class Select
     }
 
     private findAndCachePlaceholderOption(): void {
-        this.placeholderOption = this.options.find(o => o.selected && o.disabled && o.hidden);
+        this.placeholderOption = this.options.find(
+            o => o.selected && o.disabled && o.hidden
+        );
     }
 
     private filterChanged(): void {
diff --git a/packages/nimble-components/src/select/testing/select.pageobject.ts b/packages/nimble-components/src/select/testing/select.pageobject.ts
index 4ea4cda6db..c3fe01c3a0 100644
--- a/packages/nimble-components/src/select/testing/select.pageobject.ts
+++ b/packages/nimble-components/src/select/testing/select.pageobject.ts
@@ -9,6 +9,7 @@ import type { Select } from '..';
 import type { ListOption } from '../../list-option';
 import { waitForUpdatesAsync } from '../../testing/async-helpers';
 import { FilterMode } from '../types';
+import type { Button } from '../../button';
 
 /**
  * Page object for the `nimble-select` component to provide consistent ways
@@ -109,6 +110,11 @@ export class SelectPageObject {
         this.clickOption(optionIndex);
     }
 
+    public clickClearButton(): void {
+        const clearButton = this.getClearButton();
+        clearButton?.click();
+    }
+
     public async clickAway(): Promise<void> {
         this.selectElement.dispatchEvent(new Event('focusout'));
         await waitForUpdatesAsync();
@@ -168,13 +174,18 @@ export class SelectPageObject {
         );
     }
 
-    public isFilterInputVisible(): boolean {
+    public isClearButtonVisible(): boolean {
         return (
-            this.selectElement.shadowRoot?.querySelector('.filter-field')
-            !== null
+            this.selectElement.shadowRoot?.querySelector<Button>(
+                '.clear-button'
+            ) != null
         );
     }
 
+    public isFilterInputVisible(): boolean {
+        return this.getFilterInput() !== null;
+    }
+
     public isOptionVisible(index: number): boolean {
         if (index >= this.selectElement.options.length) {
             throw new Error('Indexing past number of options');
@@ -196,6 +207,12 @@ export class SelectPageObject {
         return this.getFilterInput()?.value ?? '';
     }
 
+    public getDisplayText(): string {
+        const displayText = this.selectElement.shadowRoot?.querySelector('.selected-value')
+            ?.textContent ?? '';
+        return displayText.trim();
+    }
+
     private getFilterInput(): HTMLInputElement | null | undefined {
         if (this.selectElement.filterMode === FilterMode.none) {
             throw new Error(
@@ -206,4 +223,25 @@ export class SelectPageObject {
             '.filter-input'
         );
     }
+
+    private getClearButton(): Button | null | undefined {
+        if (!this.selectElement.clearable) {
+            throw new Error(
+                'Select must set "clearable" in order to click clear button'
+            );
+        }
+
+        if (
+            this.selectElement.selectedIndex === -1
+            || this.selectElement.displayPlaceholder
+        ) {
+            throw new Error(
+                'Select must have a selected element in order to click clear button'
+            );
+        }
+
+        return this.selectElement.shadowRoot?.querySelector<Button>(
+            '.clear-button'
+        );
+    }
 }
diff --git a/packages/nimble-components/src/select/tests/select-matrix.stories.ts b/packages/nimble-components/src/select/tests/select-matrix.stories.ts
index 7e88584772..97cb70eed4 100644
--- a/packages/nimble-components/src/select/tests/select-matrix.stories.ts
+++ b/packages/nimble-components/src/select/tests/select-matrix.stories.ts
@@ -38,6 +38,12 @@ const valueStates = [
 ] as const;
 type ValueState = (typeof valueStates)[number];
 
+const clearableStates = [
+    ['', false],
+    ['Clearable', true]
+] as const;
+type ClearableState = (typeof clearableStates)[number];
+
 const metadata: Meta = {
     title: 'Tests/Select',
     parameters: {
@@ -53,6 +59,7 @@ const component = (
     [appearanceName, appearance]: AppearanceState,
     [errorName, errorVisible, errorText]: ErrorState,
     [valueName, valueValue]: ValueState,
+    [clearableName, clearable]: ClearableState
 ): ViewTemplate => html`
     <div style="
         display: inline-flex;
@@ -61,11 +68,12 @@ const component = (
         font: var(${controlLabelFont.cssCustomProperty});
         color: var(${controlLabelFontColor.cssCustomProperty});"
     >
-        <label>${() => errorName} ${() => disabledName} ${() => appearanceName} ${() => valueName}</label>
+        <label>${() => errorName} ${() => disabledName} ${() => appearanceName} ${() => valueName} ${() => clearableName}</label>
         <${selectTag}
             ?error-visible="${() => errorVisible}"
             error-text="${() => errorText}"
             ?disabled="${() => disabled}"
+            ?clearable="${() => clearable}"
             appearance="${() => appearance}"
             style="width: var(${menuMinWidth.cssCustomProperty});"
         >
@@ -82,7 +90,8 @@ export const selectThemeMatrix: StoryFn = createMatrixThemeStory(
         disabledStates,
         appearanceStates,
         errorStates,
-        valueStates
+        valueStates,
+        clearableStates
     ])
 );
 
diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 5b58069bf4..f6afffa3b5 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -199,7 +199,7 @@ describe('Select', () => {
         pageObject.pressArrowDownKey();
         await waitForUpdatesAsync();
 
-        expect(element.displayValue).toBe('One');
+        expect(pageObject.getDisplayText()).toBe('One');
         await disconnect();
     });
 
@@ -289,7 +289,7 @@ describe('Select', () => {
         const selectedOption = pageObject.getSelectedOption();
         selectedOption!.textContent = 'foo';
         await waitForUpdatesAsync();
-        expect(element.displayValue).toBe('foo');
+        expect(pageObject.getDisplayText()).toBe('foo');
 
         await disconnect();
     });
@@ -321,6 +321,59 @@ describe('Select', () => {
         await disconnect();
     });
 
+    it('clear button is visible by default with no placholder', async () => {
+        const { element, connect, disconnect } = await setup();
+        element.clearable = true;
+        const pageObject = new SelectPageObject(element);
+        await connect();
+        await waitForUpdatesAsync();
+
+        expect(pageObject.isClearButtonVisible()).toBeTrue();
+        await disconnect();
+    });
+
+    it('clicking clear button does not open dropdown', async () => {
+        const { element, connect, disconnect } = await setup();
+        element.clearable = true;
+        const pageObject = new SelectPageObject(element);
+        await connect();
+        await waitForUpdatesAsync();
+        pageObject.clickClearButton();
+        await waitForUpdatesAsync();
+        expect(element.open).toBeFalse();
+
+        await disconnect();
+    });
+
+    it('after clicking clear button, display text is empty and clear button is hidden', async () => {
+        const { element, connect, disconnect } = await setup();
+        element.clearable = true;
+        const pageObject = new SelectPageObject(element);
+        await connect();
+        await waitForUpdatesAsync();
+        pageObject.clickClearButton();
+        await waitForUpdatesAsync();
+
+        expect(pageObject.getDisplayText()).toBe('');
+        expect(pageObject.isClearButtonVisible()).toBeFalse();
+        await disconnect();
+    });
+
+    it('after clicking clear button and then selecting an option, clear button is visible again', async () => {
+        const { element, connect, disconnect } = await setup();
+        element.clearable = true;
+        const pageObject = new SelectPageObject(element);
+        await connect();
+        await waitForUpdatesAsync();
+        pageObject.clickClearButton();
+        await clickAndWaitForOpen(element);
+        pageObject.clickOption(1);
+        await waitForUpdatesAsync();
+        expect(pageObject.isClearButtonVisible()).toBeTrue();
+
+        await disconnect();
+    });
+
     describe('with 500 options', () => {
         async function setup500Options(): Promise<Fixture<Select>> {
             // prettier-ignore
@@ -800,12 +853,12 @@ describe('Select', () => {
         });
 
         it('selecting option will replace placeholder text with selected option text', async () => {
-            expect(element.displayValue).toBe('One');
+            expect(pageObject.getDisplayText()).toBe('One');
             await clickAndWaitForOpen(element);
             pageObject.clickOption(1);
             await waitForUpdatesAsync();
 
-            expect(element.displayValue).toBe('Two');
+            expect(pageObject.getDisplayText()).toBe('Two');
         });
 
         it('placeholder can be changed to another option programmatically', async () => {
@@ -816,12 +869,40 @@ describe('Select', () => {
             element.options[1]!.selected = true;
             await waitForUpdatesAsync();
 
-            expect(element.displayValue).toBe('Two');
+            expect(pageObject.getDisplayText()).toBe('Two');
             expect(element.value).toBe('two');
             await clickAndWaitForOpen(element);
             expect(pageObject.isOptionVisible(0)).toBeTrue();
             expect(pageObject.isOptionVisible(1)).toBeFalse();
         });
+
+        describe('clearable', () => {
+            beforeEach(() => {
+                element.clearable = true;
+            });
+
+            it('clear button is not visible when placeholder is displayed', () => {
+                expect(pageObject.isClearButtonVisible()).toBeFalse();
+            });
+
+            it('clear button is visible after selecting an option', async () => {
+                await clickAndWaitForOpen(element);
+                pageObject.clickOption(1);
+                await waitForUpdatesAsync();
+
+                expect(pageObject.isClearButtonVisible()).toBeTrue();
+            });
+
+            it('after clicking clear button, placeholder is visible', async () => {
+                await clickAndWaitForOpen(element);
+                pageObject.clickOption(1);
+                await waitForUpdatesAsync();
+                pageObject.clickClearButton();
+                await waitForUpdatesAsync();
+
+                expect(pageObject.getDisplayText()).toBe('One');
+            });
+        });
     });
 
     describe('PageObject', () => {

From 74a2e58777ad51f6df59186694e3b4414b7ddd83 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Tue, 16 Apr 2024 11:10:46 -0500
Subject: [PATCH 04/18] Change files

---
 ...le-components-d350a4f5-967b-4b4d-9f31-96c60b0c63cf.json | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 change/@ni-nimble-components-d350a4f5-967b-4b4d-9f31-96c60b0c63cf.json

diff --git a/change/@ni-nimble-components-d350a4f5-967b-4b4d-9f31-96c60b0c63cf.json b/change/@ni-nimble-components-d350a4f5-967b-4b4d-9f31-96c60b0c63cf.json
new file mode 100644
index 0000000000..67502f4257
--- /dev/null
+++ b/change/@ni-nimble-components-d350a4f5-967b-4b4d-9f31-96c60b0c63cf.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Add clear button for Select",
+  "packageName": "@ni/nimble-components",
+  "email": "26874831+atmgrifter00@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

From 4f4ea9a02c3cc8d9d86d7696ae469b3a210ed8a1 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Tue, 7 May 2024 13:11:01 -0500
Subject: [PATCH 05/18] Merge fix.

---
 .../nimble-mapping-icon.directive.spec.ts     | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/angular-workspace/projects/ni/nimble-angular/mapping/icon/tests/nimble-mapping-icon.directive.spec.ts b/angular-workspace/projects/ni/nimble-angular/mapping/icon/tests/nimble-mapping-icon.directive.spec.ts
index 65df5228f7..45fedba909 100644
--- a/angular-workspace/projects/ni/nimble-angular/mapping/icon/tests/nimble-mapping-icon.directive.spec.ts
+++ b/angular-workspace/projects/ni/nimble-angular/mapping/icon/tests/nimble-mapping-icon.directive.spec.ts
@@ -2,7 +2,7 @@ import { Component, ElementRef, ViewChild } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { IconSeverity } from '@ni/nimble-angular';
 import { NimbleTableModule } from '../../../table/nimble-table.module';
-import { NimbleTableColumnIconModule } from '../../../table-column/icon/nimble-table-column-icon.module';
+import { NimbleTableColumnMappingModule } from '../../../table-column/mapping/nimble-table-column-mapping.module';
 import { NimbleMappingIconDirective, type MappingIcon } from '../nimble-mapping-icon.directive';
 import { NimbleMappingIconModule } from '../nimble-mapping-icon.module';
 
@@ -10,7 +10,7 @@ describe('NimbleMappingIcon', () => {
     describe('module', () => {
         beforeEach(() => {
             TestBed.configureTestingModule({
-                imports: [NimbleMappingIconModule]
+                imports: [NimbleTableColumnMappingModule]
             });
         });
 
@@ -23,7 +23,7 @@ describe('NimbleMappingIcon', () => {
         @Component({
             template: `
                 <nimble-table>
-                    <nimble-table-column-icon key-type="boolean">
+                    <nimble-table-column-mapping key-type="boolean">
                         <nimble-mapping-icon
                             #mapping
                             key="false"
@@ -33,7 +33,7 @@ describe('NimbleMappingIcon', () => {
                             text-hidden
                         >
                         </nimble-mapping-icon>
-                    </nimble-table-column-icon>
+                    </nimble-table-column-mapping>
                 </nimble-table>
             `
         })
@@ -48,7 +48,7 @@ describe('NimbleMappingIcon', () => {
         beforeEach(() => {
             TestBed.configureTestingModule({
                 declarations: [TestHostComponent],
-                imports: [NimbleMappingIconModule, NimbleTableColumnIconModule, NimbleTableModule]
+                imports: [NimbleMappingIconModule, NimbleTableColumnMappingModule, NimbleTableModule]
             });
 
             fixture = TestBed.createComponent(TestHostComponent);
@@ -87,7 +87,7 @@ describe('NimbleMappingIcon', () => {
         @Component({
             template: `
                 <nimble-table>
-                    <nimble-table-column-icon key-type="boolean">
+                    <nimble-table-column-mapping key-type="boolean">
                         <nimble-mapping-icon
                             #mapping
                             [key]="key"
@@ -97,7 +97,7 @@ describe('NimbleMappingIcon', () => {
                             [textHidden]="textHidden"
                         >
                         </nimble-mapping-icon>
-                    </nimble-table-column-icon>
+                    </nimble-table-column-mapping>
                 </nimble-table>
             `
         })
@@ -117,7 +117,7 @@ describe('NimbleMappingIcon', () => {
         beforeEach(() => {
             TestBed.configureTestingModule({
                 declarations: [TestHostComponent],
-                imports: [NimbleMappingIconModule, NimbleTableColumnIconModule, NimbleTableModule]
+                imports: [NimbleMappingIconModule, NimbleTableColumnMappingModule, NimbleTableModule]
             });
 
             fixture = TestBed.createComponent(TestHostComponent);
@@ -186,7 +186,7 @@ describe('NimbleMappingIcon', () => {
         @Component({
             template: `
                 <nimble-table>
-                    <nimble-table-column-icon key-type="boolean">
+                    <nimble-table-column-mapping key-type="boolean">
                         <nimble-mapping-icon
                             #mapping
                             [attr.key]="key"
@@ -196,7 +196,7 @@ describe('NimbleMappingIcon', () => {
                             [attr.text-hidden]="textHidden"
                         >
                         </nimble-mapping-icon>
-                    </nimble-table-column-icon>
+                    </nimble-table-column-mapping>
                 </nimble-table>
             `
         })
@@ -216,7 +216,7 @@ describe('NimbleMappingIcon', () => {
         beforeEach(() => {
             TestBed.configureTestingModule({
                 declarations: [TestHostComponent],
-                imports: [NimbleMappingIconModule, NimbleTableColumnIconModule, NimbleTableModule]
+                imports: [NimbleMappingIconModule, NimbleTableColumnMappingModule, NimbleTableModule]
             });
 
             fixture = TestBed.createComponent(TestHostComponent);

From c9359b85e67abce897c7722a96108b468712991b Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Wed, 8 May 2024 12:19:18 -0500
Subject: [PATCH 06/18] Don't cache placeholder. Update tests.

---
 .../nimble-components/src/select/index.ts     |  78 +++---
 .../src/select/testing/select.pageobject.ts   |  25 +-
 .../src/select/tests/select.spec.ts           | 226 ++++++++++++------
 3 files changed, 208 insertions(+), 121 deletions(-)

diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
index 5bc0ae4165..da6d16a49d 100644
--- a/packages/nimble-components/src/select/index.ts
+++ b/packages/nimble-components/src/select/index.ts
@@ -61,6 +61,10 @@ const isOptionSelectable = (el: ListOption): boolean => {
     return !el.visuallyHidden && !el.disabled && !el.hidden;
 };
 
+const isOptionPlaceholder = (el: ListboxOption): boolean => {
+    return el.disabled && el.hidden;
+};
+
 /**
  * A nimble-styled HTML select.
  */
@@ -310,7 +314,6 @@ export class Select
             notifier.subscribe(this, 'hidden');
             notifier.subscribe(this, 'disabled');
         });
-        this.findAndCachePlaceholderOption();
         this.setProxyOptions();
         this.updateValue();
         // We need to force an update to the filteredOptions observable
@@ -367,10 +370,18 @@ export class Select
                 break;
             }
             case 'selected': {
-                if (isNimbleListOption(sourceElement)) {
+                if (
+                    isNimbleListOption(sourceElement)
+                    && sourceElement.selected
+                ) {
                     this.selectedIndex = this.options.indexOf(sourceElement);
+                } else {
+                    const placeholderOption = this.getPlaceholderOption();
+                    this.selectedIndex = placeholderOption
+                        ? this.options.indexOf(placeholderOption)
+                        : -1;
                 }
-                this.setSelectedOptions();
+                this.updateValue();
                 this.updateDisplayValue();
                 break;
             }
@@ -446,13 +457,16 @@ export class Select
         );
     }
 
+    /**
+     * @internal
+     */
     public clearClickHandler(e: MouseEvent): void {
-        if (this.placeholderOption) {
-            this.selectedIndex = this.options.indexOf(this.placeholderOption);
-            this.committedSelectedOption = this.placeholderOption;
-        } else {
-            this.selectedIndex = -1;
-        }
+        this.open = false;
+        const placeholderOption = this.getPlaceholderOption();
+        this.selectedIndex = placeholderOption
+            ? this.options.indexOf(placeholderOption)
+            : -1;
+        this.updateValue(true);
         e.stopPropagation();
     }
 
@@ -460,10 +474,10 @@ export class Select
      * @internal
      */
     public updateDisplayValue(): void {
+        const placeholderOption = this.getPlaceholderOption();
         if (
-            this.committedSelectedOption?.disabled
-            && this.committedSelectedOption?.hidden
-            && this.committedSelectedOption?.selected
+            placeholderOption
+            && this.committedSelectedOption === placeholderOption
         ) {
             this.displayPlaceholder = true;
         } else {
@@ -517,24 +531,13 @@ export class Select
             return true;
         }
 
+        this.open = false;
         const focusTarget = e.relatedTarget as HTMLElement;
         if (this.isSameNode(focusTarget)) {
             this.focus();
             return true;
         }
 
-        if (!this.options?.includes(focusTarget as ListboxOption)) {
-            let currentActiveIndex = this.openActiveIndex ?? this.selectedIndex;
-            this.open = false;
-            if (currentActiveIndex === -1) {
-                currentActiveIndex = this.selectedIndex;
-            }
-
-            if (this.selectedIndex !== currentActiveIndex) {
-                this.selectedIndex = currentActiveIndex;
-                this.updateValue(true);
-            }
-        }
         return true;
     }
 
@@ -689,6 +692,7 @@ export class Select
         if (this.selectedIndex === -1) {
             this.selectedIndex = 0;
         }
+        this.updateValue();
     }
 
     /**
@@ -767,10 +771,10 @@ export class Select
         this.options.push(option);
     }
 
-    // Prevents parent classes from resetting selectedIndex to a positive
-    // value while filtering, which can result in a disabled option being
-    // selected.
     protected override setSelectedOptions(): void {
+        // Prevents parent classes from resetting selectedIndex to a positive
+        // value while filtering, which can result in a disabled option being
+        // selected.
         if (this.open && this.selectedIndex === -1) {
             return;
         }
@@ -887,14 +891,18 @@ export class Select
         };
         let selectedIndex = -1;
         let firstValidOptionIndex = -1;
+        let placeholderIndex = -1;
         for (let i = 0; i < options?.length; i++) {
-            const option = options[i];
-            if (optionIsSelected(option!) || option?.value === this.value) {
+            const option = options[i]!;
+            if (optionIsSelected(option) || option.value === this.value) {
                 selectedIndex = i;
-            }
-            if (
+                break;
+            } else if (placeholderIndex === -1 && isOptionPlaceholder(option)) {
+                placeholderIndex = i;
+            } else if (
                 firstValidOptionIndex === -1
-                && isOptionSelectable(option! as ListOption)
+                && placeholderIndex === -1
+                && isOptionSelectable(option as ListOption)
             ) {
                 firstValidOptionIndex = i;
             }
@@ -902,6 +910,8 @@ export class Select
 
         if (selectedIndex !== -1) {
             this.selectedIndex = selectedIndex;
+        } else if (placeholderIndex !== -1) {
+            this.selectedIndex = placeholderIndex;
         } else if (firstValidOptionIndex !== -1) {
             this.selectedIndex = firstValidOptionIndex;
         } else {
@@ -950,6 +960,10 @@ export class Select
         }
     }
 
+    private getPlaceholderOption(): ListOption | undefined {
+        return this.options.find(o => o.hidden && o.disabled) as ListOption;
+    }
+
     private committedSelectedOptionChanged(): void {
         this.updateDisplayValue();
     }
diff --git a/packages/nimble-components/src/select/testing/select.pageobject.ts b/packages/nimble-components/src/select/testing/select.pageobject.ts
index ee40a0c52d..d3f0faa1b9 100644
--- a/packages/nimble-components/src/select/testing/select.pageobject.ts
+++ b/packages/nimble-components/src/select/testing/select.pageobject.ts
@@ -233,7 +233,10 @@ export class SelectPageObject {
     }
 
     public isFilterInputVisible(): boolean {
-        return this.getFilterInput() !== null;
+        return (
+            this.selectElement.shadowRoot?.querySelector('.filter-field')
+            !== null
+        );
     }
 
     public isOptionVisible(index: number): boolean {
@@ -257,26 +260,6 @@ export class SelectPageObject {
         return this.selectElement.filterInput?.value ?? '';
     }
 
-    public async waitForChange(): Promise<string> {
-        return new Promise(resolve => {
-            this.selectElement.addEventListener(
-                'change',
-                () => {
-                    resolve(this.selectElement.value);
-                },
-                {
-                    once: true
-                }
-            );
-        });
-    }
-
-    public getDisplayText(): string {
-        const displayText = this.selectElement.shadowRoot?.querySelector('.selected-value')
-            ?.textContent ?? '';
-        return displayText.trim();
-    }
-
     private getFilterInput(): HTMLInputElement | null | undefined {
         if (this.selectElement.filterMode === FilterMode.none) {
             throw new Error(
diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 573a3fed70..2ba57d342e 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -15,11 +15,13 @@ import { filterSearchLabel } from '../../label-provider/core/label-tokens';
 
 const disabledOption = 'disabled';
 const disabledSelectedOption = 'disabled selected';
-const placeholderOption = 'disabled selected hidden';
+const placeholderSelectedOption = 'disabled selected hidden';
 
 type OptionInitialState =
+    | 'selected'
     | 'disabled'
     | 'disabled selected'
+    | 'disabled hidden'
     | 'disabled selected hidden'
     | 'hidden'
     | 'visually-hidden';
@@ -177,7 +179,7 @@ describe('Select', () => {
         await disconnect();
     });
 
-    it('pressing Esc after navigating to new option in dropdown maintains original selected option', async () => {
+    it('pressing <Esc> after navigating to new option in dropdown maintains original selected option', async () => {
         const { element, connect, disconnect } = await setup();
         await connect();
         const pageObject = new SelectPageObject(element);
@@ -234,19 +236,46 @@ describe('Select', () => {
 
         const defaultOptionTestCases: {
             name: string,
-            value: OptionInitialState
+            option1State: OptionInitialState,
+            option2State?: OptionInitialState,
+            expected: string
         }[] = [
+            {
+                name: 'first option selected, first is default',
+                option1State: 'selected',
+                expected: 'one'
+            },
             {
                 name: 'first option disabled, second is default',
-                value: 'disabled'
+                option1State: 'disabled',
+                expected: 'two'
             },
             {
                 name: 'first option hidden, second is default',
-                value: 'hidden'
+                option1State: 'hidden',
+                expected: 'two'
             },
             {
                 name: 'first option visually-hidden, second is default',
-                value: 'visually-hidden'
+                option1State: 'visually-hidden',
+                expected: 'two'
+            },
+            {
+                name: 'first option hidden and disabled, first is default as placeholder',
+                option1State: 'disabled hidden',
+                expected: 'one'
+            },
+            {
+                name: 'first option disabled, second is disabled hidden, second is default as placeholder',
+                option1State: 'disabled',
+                option2State: 'disabled hidden',
+                expected: 'two'
+            },
+            {
+                name: 'first option disabled, second is disabled, third is default',
+                option1State: 'disabled',
+                option2State: 'disabled',
+                expected: 'three'
             }
         ];
         parameterizeSpec(defaultOptionTestCases, (spec, name, value) => {
@@ -254,12 +283,12 @@ describe('Select', () => {
                 const { element, connect, disconnect } = await setup(
                     undefined,
                     false,
-                    value.value,
-                    undefined
+                    value.option1State,
+                    value.option2State
                 );
                 await connect();
                 await waitForUpdatesAsync();
-                expect(element.value).toBe('two');
+                expect(element.value).toBe(value.expected);
 
                 await disconnect();
             });
@@ -427,72 +456,30 @@ describe('Select', () => {
         await disconnect();
     });
 
-    it('clear button is visible by default with no placholder', async () => {
-        const { element, connect, disconnect } = await setup();
-        element.clearable = true;
-        const pageObject = new SelectPageObject(element);
-        await connect();
-        await waitForUpdatesAsync();
-
-        expect(pageObject.isClearButtonVisible()).toBeTrue();
-        await disconnect();
-    });
-
-    it('clicking clear button does not open dropdown', async () => {
+    it('programmatically setting selected of current selected option to false results in blank display', async () => {
         const { element, connect, disconnect } = await setup();
-        element.clearable = true;
-        const pageObject = new SelectPageObject(element);
         await connect();
-        await waitForUpdatesAsync();
-        pageObject.clickClearButton();
-        await waitForUpdatesAsync();
-        expect(element.open).toBeFalse();
-
-        await disconnect();
-    });
-
-    it('after clicking clear button, display text is empty and clear button is hidden', async () => {
-        const { element, connect, disconnect } = await setup();
-        element.clearable = true;
         const pageObject = new SelectPageObject(element);
-        await connect();
-        await waitForUpdatesAsync();
-        pageObject.clickClearButton();
+        const selectedOption = pageObject.getSelectedOption();
+        selectedOption!.selected = false;
         await waitForUpdatesAsync();
 
         expect(pageObject.getDisplayText()).toBe('');
-        expect(pageObject.isClearButtonVisible()).toBeFalse();
-        await disconnect();
-    });
-
-    it('after clicking clear button and then selecting an option, clear button is visible again', async () => {
-        const { element, connect, disconnect } = await setup();
-        element.clearable = true;
-        const pageObject = new SelectPageObject(element);
-        await connect();
-        await waitForUpdatesAsync();
-        pageObject.clickClearButton();
-        await clickAndWaitForOpen(element);
-        pageObject.clickOption(1);
-        await waitForUpdatesAsync();
-        expect(pageObject.isClearButtonVisible()).toBeTrue();
 
         await disconnect();
     });
 
-    it('when handling change event, value of select element matches what was selected in dropdown on focusout', async () => {
+    it('navigating to different option in dropdown and then clicking away does not change value', async () => {
         const { element, connect, disconnect } = await setup();
         await connect();
         await waitForUpdatesAsync();
         const pageObject = new SelectPageObject(element);
         await clickAndWaitForOpen(element);
 
-        const changeValuePromise = pageObject.waitForChange();
         pageObject.pressArrowDownKey();
         await pageObject.clickAway();
-        const selectValue = await changeValuePromise;
 
-        expect(selectValue).toBe('two');
+        expect(element.value).toBe('one');
 
         await disconnect();
     });
@@ -809,14 +796,14 @@ describe('Select', () => {
             expect(element.open).toBeFalse();
         });
 
-        it('filtering out current selected item and then pressing <Tab> changes value and closes popup', async () => {
+        it('filtering out current selected item and then pressing <Tab> does not change value and closes popup', async () => {
             const currentSelection = pageObject.getSelectedOption();
             expect(currentSelection?.text).toBe('One');
             expect(element.value).toBe('one');
 
             await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three'
             pageObject.pressTabKey();
-            expect(element.value).toBe('two'); // 'Two' is first option in list so it should be selected now
+            expect(element.value).toBe('one');
             expect(element.open).toBeFalse();
         });
 
@@ -842,14 +829,14 @@ describe('Select', () => {
             expect(element.open).toBeFalse();
         });
 
-        it('filtering out current selected item and then losing focus changes value and closes popup', async () => {
+        it('filtering out current selected item and then losing focus does not change value and closes popup', async () => {
             const currentSelection = pageObject.getSelectedOption();
             expect(currentSelection?.text).toBe('One');
             expect(element.value).toBe('one');
 
             await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three'
             await pageObject.clickAway();
-            expect(element.value).toBe('two'); // 'Two' is first option in list so it should be selected now
+            expect(element.value).toBe('one'); // 'Two' is first option in list so it should be selected now
             expect(element.open).toBeFalse();
         });
 
@@ -1097,11 +1084,12 @@ describe('Select', () => {
             ({ element, connect, disconnect } = await setup(
                 undefined,
                 false,
-                placeholderOption
+                placeholderSelectedOption
             ));
             element.style.width = '200px';
             element.filterMode = FilterMode.standard;
             await connect();
+            await waitForUpdatesAsync();
             pageObject = new SelectPageObject(element);
             await waitForUpdatesAsync();
         });
@@ -1149,22 +1137,98 @@ describe('Select', () => {
             expect(pageObject.getDisplayText()).toBe('One one');
         });
 
-        describe('clearable', () => {
-            beforeEach(() => {
-                element.clearable = true;
+        it('programmatically setting selected of current selected option to false results in placeholder being displayed', async () => {
+            await clickAndWaitForOpen(element);
+            pageObject.clickOption(1);
+            const selectedOption = pageObject.getSelectedOption();
+            selectedOption!.selected = false;
+            await waitForUpdatesAsync();
+
+            expect(pageObject.getDisplayText()).toBe('One');
+        });
+    });
+
+    describe('clearable', () => {
+        let element: Select;
+        let connect: () => Promise<void>;
+        let disconnect: () => Promise<void>;
+        let pageObject: SelectPageObject;
+        beforeEach(async () => {
+            ({ element, connect, disconnect } = await setup());
+            element.style.width = '200px';
+            element.clearable = true;
+            await connect();
+            await waitForUpdatesAsync();
+            pageObject = new SelectPageObject(element);
+        });
+
+        afterEach(async () => {
+            await disconnect();
+        });
+
+        it('clear button is visible after selecting an option', async () => {
+            await clickAndWaitForOpen(element);
+            pageObject.clickOption(1);
+            await waitForUpdatesAsync();
+
+            expect(pageObject.isClearButtonVisible()).toBeTrue();
+        });
+
+        it('clicking clear button does not open dropdown', async () => {
+            pageObject.clickClearButton();
+            await waitForUpdatesAsync();
+            expect(element.open).toBeFalse();
+        });
+
+        it('clicking clear button fires change event', () => {
+            const changeEvent = jasmine.createSpy();
+            element.addEventListener('change', changeEvent);
+            pageObject.clickClearButton();
+            expect(changeEvent).toHaveBeenCalledTimes(1);
+        });
+
+        it('if dropdown open clicking clear button closes dropdown and fires change event', async () => {
+            pageObject.clickSelect();
+            await waitForUpdatesAsync();
+            const changeEvent = jasmine.createSpy();
+            element.addEventListener('change', changeEvent);
+            pageObject.clickClearButton();
+            expect(changeEvent).toHaveBeenCalledTimes(1);
+        });
+
+        describe('without placeholder', () => {
+            it('clear button is visible by default', () => {
+                expect(pageObject.isClearButtonVisible()).toBeTrue();
             });
 
-            it('clear button is not visible when placeholder is displayed', () => {
+            it('after clicking clear button, display text is empty and clear button is hidden', async () => {
+                pageObject.clickClearButton();
+                await waitForUpdatesAsync();
+
+                expect(pageObject.getDisplayText()).toBe('');
                 expect(pageObject.isClearButtonVisible()).toBeFalse();
             });
 
-            it('clear button is visible after selecting an option', async () => {
+            it('after clicking clear button and then selecting an option, clear button is visible again', async () => {
+                pageObject.clickClearButton();
                 await clickAndWaitForOpen(element);
                 pageObject.clickOption(1);
                 await waitForUpdatesAsync();
-
                 expect(pageObject.isClearButtonVisible()).toBeTrue();
             });
+        });
+
+        describe('with placeholder', () => {
+            beforeEach(async () => {
+                const firstOption = element.options[0]!;
+                firstOption.disabled = true;
+                firstOption.hidden = true;
+                await waitForUpdatesAsync();
+            });
+
+            it('clear button is not visible when placeholder is displayed', () => {
+                expect(pageObject.isClearButtonVisible()).toBeFalse();
+            });
 
             it('after clicking clear button, placeholder is visible', async () => {
                 await clickAndWaitForOpen(element);
@@ -1175,6 +1239,32 @@ describe('Select', () => {
 
                 expect(pageObject.getDisplayText()).toBe('One');
             });
+
+            const removePlaceholderData = [
+                {
+                    name: 'remove hidden from placeholder, clear button becomes visible and clicking it results in empty display text',
+                    updatePlaceholder: (option: ListOption) => {
+                        option.hidden = false;
+                    }
+                },
+                {
+                    name: 'remove disabled from placeholder, clear button becomes visible and clicking it results in empty display text',
+                    updatePlaceholder: (option: ListOption) => {
+                        option.disabled = false;
+                    }
+                }
+            ];
+            parameterizeSpec(removePlaceholderData, (spec, name, value) => {
+                spec(name, async () => {
+                    value.updatePlaceholder(element.options[0]! as ListOption);
+                    await waitForUpdatesAsync();
+                    expect(pageObject.isClearButtonVisible()).toBeTrue();
+                    pageObject.clickClearButton();
+                    await waitForUpdatesAsync();
+
+                    expect(pageObject.getDisplayText()).toBe('');
+                });
+            });
         });
     });
 
@@ -1188,7 +1278,7 @@ describe('Select', () => {
             ({ element, connect, disconnect } = await setup(
                 undefined,
                 false,
-                placeholderOption
+                placeholderSelectedOption
             ));
             element.style.width = '200px';
             element.filterMode = FilterMode.standard;

From 5a0caf5f8472099af35dfc43e2a83b26ce7fbd38 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Wed, 8 May 2024 15:09:46 -0500
Subject: [PATCH 07/18] Remove unused members.

---
 packages/nimble-components/src/select/index.ts | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
index 965694b0d4..1e88982cc5 100644
--- a/packages/nimble-components/src/select/index.ts
+++ b/packages/nimble-components/src/select/index.ts
@@ -203,7 +203,6 @@ export class Select
 
     private _value = '';
     private forcedPosition = false;
-    private placeholderOption?: ListboxOption;
     private openActiveIndex?: number;
 
     /**
@@ -1081,12 +1080,6 @@ export class Select
         }
     }
 
-    private findAndCachePlaceholderOption(): void {
-        this.placeholderOption = this.options.find(
-            o => o.selected && o.disabled && o.hidden
-        );
-    }
-
     private filterChanged(): void {
         this.filterOptions();
     }

From 884c85e716d7c6af2df1ce518e21fe2ea1c46da8 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Thu, 9 May 2024 09:40:26 -0500
Subject: [PATCH 08/18] Update
 packages/nimble-components/src/select/testing/select.pageobject.ts

Co-authored-by: m-akinc <7282195+m-akinc@users.noreply.github.com>
---
 .../src/select/testing/select.pageobject.ts                 | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/packages/nimble-components/src/select/testing/select.pageobject.ts b/packages/nimble-components/src/select/testing/select.pageobject.ts
index d3f0faa1b9..ae5a5edaab 100644
--- a/packages/nimble-components/src/select/testing/select.pageobject.ts
+++ b/packages/nimble-components/src/select/testing/select.pageobject.ts
@@ -225,11 +225,7 @@ export class SelectPageObject {
     }
 
     public isClearButtonVisible(): boolean {
-        return (
-            this.selectElement.shadowRoot?.querySelector<Button>(
-                '.clear-button'
-            ) != null
-        );
+        return this.getClearButton() != null;
     }
 
     public isFilterInputVisible(): boolean {

From d4f92027840f1a07dccb0b8d135aaba45ed23683 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Thu, 9 May 2024 09:57:49 -0500
Subject: [PATCH 09/18] Handle PR feedback.

---
 .../nimble-components/src/select/index.ts     | 60 ++++---------------
 .../nimble-components/src/select/styles.ts    |  8 +--
 .../nimble-components/src/select/template.ts  |  2 +-
 .../src/select/testing/select.pageobject.ts   | 30 +++++-----
 4 files changed, 33 insertions(+), 67 deletions(-)

diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
index 1e88982cc5..6e0a49959f 100644
--- a/packages/nimble-components/src/select/index.ts
+++ b/packages/nimble-components/src/select/index.ts
@@ -177,12 +177,6 @@ export class Select
     @observable
     public filter = '';
 
-    /**
-     * @internal
-     */
-    @observable
-    public committedSelectedOption?: ListboxOption;
-
     /**
      * The max height for the listbox when opened.
      *
@@ -246,15 +240,9 @@ export class Select
         if (prev !== newValue && !(this.open && this.selectedIndex < 0)) {
             this._value = newValue;
             super.valueChanged(prev, newValue);
-            if (!this.open) {
-                this.committedSelectedOption = this.options.find(
-                    o => o.value === newValue
-                );
-            }
             Observable.notify(this, 'value');
+            this.updateDisplayValue();
         }
-
-        Observable.notify(this, 'displayValue');
     }
 
     /**
@@ -263,7 +251,7 @@ export class Select
     @volatile
     public get displayValue(): string {
         Observable.track(this, 'displayValue');
-        return this.committedSelectedOption?.text ?? '';
+        return this.firstSelectedOption?.text ?? '';
     }
 
     /**
@@ -321,7 +309,6 @@ export class Select
         if (value) {
             this.value = value;
         }
-        this.committedSelectedOption = this.options[this.selectedIndex];
     }
 
     /**
@@ -382,13 +369,8 @@ export class Select
                 ) {
                     this.selectedIndex = this.options.indexOf(sourceElement);
                 } else {
-                    const placeholderOption = this.getPlaceholderOption();
-                    this.selectedIndex = placeholderOption
-                        ? this.options.indexOf(placeholderOption)
-                        : -1;
+                    this.clearSelect();
                 }
-                this.updateValue();
-                this.updateDisplayValue();
                 break;
             }
             case 'hidden': {
@@ -454,24 +436,12 @@ export class Select
         e.stopPropagation(); // clicking in filter input shouldn't close dropdown
     }
 
-    /**
-     * @internal
-     */
-    public changeValueHandler(): void {
-        this.committedSelectedOption = this.options.find(
-            option => option.selected
-        );
-    }
-
     /**
      * @internal
      */
     public clearClickHandler(e: MouseEvent): void {
         this.open = false;
-        const placeholderOption = this.getPlaceholderOption();
-        this.selectedIndex = placeholderOption
-            ? this.options.indexOf(placeholderOption)
-            : -1;
+        this.clearSelect();
         this.updateValue(true);
         e.stopPropagation();
     }
@@ -483,16 +453,14 @@ export class Select
         const placeholderOption = this.getPlaceholderOption();
         if (
             placeholderOption
-            && this.committedSelectedOption === placeholderOption
+            && this.firstSelectedOption === placeholderOption
         ) {
             this.displayPlaceholder = true;
         } else {
             this.displayPlaceholder = false;
         }
 
-        if (this.collapsible) {
-            Observable.notify(this, 'displayValue');
-        }
+        Observable.notify(this, 'displayValue');
     }
 
     /**
@@ -698,7 +666,6 @@ export class Select
         if (this.selectedIndex === -1) {
             this.selectedIndex = 0;
         }
-        this.updateValue();
     }
 
     /**
@@ -907,7 +874,6 @@ export class Select
                 placeholderIndex = i;
             } else if (
                 firstValidOptionIndex === -1
-                && placeholderIndex === -1
                 && isOptionSelectable(option as ListOption)
             ) {
                 firstValidOptionIndex = i;
@@ -923,7 +889,6 @@ export class Select
         } else {
             this.selectedIndex = 0;
         }
-        this.committedSelectedOption = options[this.selectedIndex];
     }
 
     private setActiveOption(newActiveIndex: number): void {
@@ -970,10 +935,6 @@ export class Select
         return this.options.find(o => o.hidden && o.disabled) as ListOption;
     }
 
-    private committedSelectedOptionChanged(): void {
-        this.updateDisplayValue();
-    }
-
     private setPositioning(): void {
         if (!this.$fastController.isConnected) {
             // Don't call setPositioning() until we're connected,
@@ -1047,7 +1008,6 @@ export class Select
     private updateValue(shouldEmit?: boolean): void {
         if (this.$fastController.isConnected) {
             this.value = this.firstSelectedOption?.value ?? '';
-            this.committedSelectedOption = this.firstSelectedOption;
         }
 
         if (shouldEmit) {
@@ -1059,6 +1019,13 @@ export class Select
         }
     }
 
+    private clearSelect(): void {
+        const placeholder = this.getPlaceholderOption();
+        this.selectedIndex = placeholder
+            ? this.options.indexOf(placeholder)
+            : -1;
+    }
+
     /**
      * Resets and fills the proxy to match the component's options.
      *
@@ -1089,7 +1056,6 @@ export class Select
     }
 
     private initializeOpenState(): void {
-        this.committedSelectedOption = this.options[this.selectedIndex];
         this.setActiveOption(this.selectedIndex);
         this.ariaControls = this.listboxId;
         this.ariaExpanded = 'true';
diff --git a/packages/nimble-components/src/select/styles.ts b/packages/nimble-components/src/select/styles.ts
index 8fefa9b91b..ca12bad57c 100644
--- a/packages/nimble-components/src/select/styles.ts
+++ b/packages/nimble-components/src/select/styles.ts
@@ -37,10 +37,6 @@ export const styles = css`
         order: 1;
     }
 
-    [part='indicator'] {
-        order: 4;
-    }
-
     .clear-button {
         order: 3;
         width: auto;
@@ -48,6 +44,10 @@ export const styles = css`
         margin-left: ${smallPadding};
     }
 
+    [part='indicator'] {
+        order: 4;
+    }
+
     .error-icon {
         order: 2;
     }
diff --git a/packages/nimble-components/src/select/template.ts b/packages/nimble-components/src/select/template.ts
index 703b321cd9..d12c108780 100644
--- a/packages/nimble-components/src/select/template.ts
+++ b/packages/nimble-components/src/select/template.ts
@@ -48,7 +48,6 @@ SelectOptions
         role="combobox"
         tabindex="${x => (!x.disabled ? '0' : null)}"
         @click="${(x, c) => x.clickHandler(c.event as MouseEvent)}"
-        @change="${x => x.changeValueHandler()}"
         @contentchange="${x => x.updateDisplayValue()}"
         @focusin="${(x, c) => x.focusinHandler(c.event as FocusEvent)}"
         @focusout="${(x, c) => x.focusoutHandler(c.event as FocusEvent)}"
@@ -70,6 +69,7 @@ SelectOptions
                     ${when(x => x.clearable && !x.displayPlaceholder && x.selectedIndex >= 0, html<Select>`
                         <${buttonTag} 
                             class="clear-button"
+                            part="clear-button"
                             content-hidden
                             appearance="ghost"
                             @click="${(x, c) => x.clearClickHandler(c.event as MouseEvent)}">
diff --git a/packages/nimble-components/src/select/testing/select.pageobject.ts b/packages/nimble-components/src/select/testing/select.pageobject.ts
index d3f0faa1b9..10e8c8faba 100644
--- a/packages/nimble-components/src/select/testing/select.pageobject.ts
+++ b/packages/nimble-components/src/select/testing/select.pageobject.ts
@@ -130,6 +130,21 @@ export class SelectPageObject {
     }
 
     public clickClearButton(): void {
+        if (!this.selectElement.clearable) {
+            throw new Error(
+                'Select must set "clearable" in order to click clear button'
+            );
+        }
+
+        if (
+            this.selectElement.selectedIndex === -1
+            || this.selectElement.displayPlaceholder
+        ) {
+            throw new Error(
+                'Select must have a selected element in order to click clear button'
+            );
+        }
+
         const clearButton = this.getClearButton();
         clearButton?.click();
     }
@@ -270,21 +285,6 @@ export class SelectPageObject {
     }
 
     private getClearButton(): Button | null | undefined {
-        if (!this.selectElement.clearable) {
-            throw new Error(
-                'Select must set "clearable" in order to click clear button'
-            );
-        }
-
-        if (
-            this.selectElement.selectedIndex === -1
-            || this.selectElement.displayPlaceholder
-        ) {
-            throw new Error(
-                'Select must have a selected element in order to click clear button'
-            );
-        }
-
         return this.selectElement.shadowRoot?.querySelector<Button>(
             '.clear-button'
         );

From 44719b0b4d5c13be5209660a805bf110887cc9b6 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Thu, 9 May 2024 10:31:49 -0500
Subject: [PATCH 10/18] Handle PR feedback.

---
 .../src/select/tests/select.spec.ts           | 69 ++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 9e2a798613..9cba6e6dec 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -236,7 +236,7 @@ describe('Select', () => {
 
         const defaultOptionTestCases: {
             name: string,
-            option1State: OptionInitialState,
+            option1State?: OptionInitialState,
             option2State?: OptionInitialState,
             expected: string
         }[] = [
@@ -276,6 +276,12 @@ describe('Select', () => {
                 option1State: 'disabled',
                 option2State: 'disabled',
                 expected: 'three'
+            },
+            {
+                name: 'second option selected, second is default',
+                option1State: undefined,
+                option2State: 'selected',
+                expected: 'two'
             }
         ];
         parameterizeSpec(defaultOptionTestCases, (spec, name, value) => {
@@ -502,6 +508,64 @@ describe('Select', () => {
         await disconnect();
     });
 
+    describe('with all options disabled', () => {
+        async function setupAllDisabled(): Promise<Fixture<Select>> {
+            const viewTemplate = html`
+                <nimble-select>
+                    <nimble-list-option disabled value="one"
+                        >One</nimble-list-option
+                    >
+                    <nimble-list-option disabled value="two"
+                        >Two</nimble-list-option
+                    >
+                    <nimble-list-option disabled value="three"
+                        >Three</nimble-list-option
+                    >
+                </nimble-select>
+            `;
+            return fixture<Select>(viewTemplate);
+        }
+
+        it('when all options disabled, first option is selected', async () => {
+            const { element, connect, disconnect } = await setupAllDisabled();
+            await connect();
+            await waitForUpdatesAsync();
+
+            expect(element.value).toBe('one');
+
+            await disconnect();
+        });
+    });
+
+    describe('with all options hidden', () => {
+        async function setupAllHidden(): Promise<Fixture<Select>> {
+            const viewTemplate = html`
+                <nimble-select>
+                    <nimble-list-option hidden value="one"
+                        >One</nimble-list-option
+                    >
+                    <nimble-list-option hidden value="two"
+                        >Two</nimble-list-option
+                    >
+                    <nimble-list-option hidden value="three"
+                        >Three</nimble-list-option
+                    >
+                </nimble-select>
+            `;
+            return fixture<Select>(viewTemplate);
+        }
+
+        it('when all options hidden, first option is selected', async () => {
+            const { element, connect, disconnect } = await setupAllHidden();
+            await connect();
+            await waitForUpdatesAsync();
+
+            expect(element.value).toBe('one');
+
+            await disconnect();
+        });
+    });
+
     describe('with 500 options', () => {
         async function setup500Options(): Promise<Fixture<Select>> {
             // prettier-ignore
@@ -1185,6 +1249,9 @@ describe('Select', () => {
         });
 
         it('clear button is visible after selecting an option', async () => {
+            pageObject.clickClearButton();
+            await waitForUpdatesAsync();
+            expect(pageObject.isClearButtonVisible()).toBeFalse();
             await clickAndWaitForOpen(element);
             pageObject.clickOption(1);
             await waitForUpdatesAsync();

From 84cbdd2dd7e3ab5890aa255e1247a6ed0270f63a Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Thu, 9 May 2024 10:37:41 -0500
Subject: [PATCH 11/18] Slight test description change.

---
 packages/nimble-components/src/select/tests/select.spec.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 9cba6e6dec..dcb6d8bf72 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -1311,7 +1311,7 @@ describe('Select', () => {
                 await waitForUpdatesAsync();
             });
 
-            it('clear button is not visible when placeholder is displayed', () => {
+            it('clear button is not visible by default', () => {
                 expect(pageObject.isClearButtonVisible()).toBeFalse();
             });
 

From 1086748cedf724e7fe729ccff21102487b1041ce Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Thu, 9 May 2024 14:52:44 -0500
Subject: [PATCH 12/18] Handle PR feedback.

---
 .../nimble-components/src/select/tests/select.spec.ts     | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index dcb6d8bf72..8f45f16514 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -1248,6 +1248,10 @@ describe('Select', () => {
             await disconnect();
         });
 
+        it('clear button is visible by default', () => {
+            expect(pageObject.isClearButtonVisible()).toBeTrue();
+        });
+
         it('clear button is visible after selecting an option', async () => {
             pageObject.clickClearButton();
             await waitForUpdatesAsync();
@@ -1282,10 +1286,6 @@ describe('Select', () => {
         });
 
         describe('without placeholder', () => {
-            it('clear button is visible by default', () => {
-                expect(pageObject.isClearButtonVisible()).toBeTrue();
-            });
-
             it('after clicking clear button, display text is empty and clear button is hidden', async () => {
                 pageObject.clickClearButton();
                 await waitForUpdatesAsync();

From 5beb1ce1fc35fb07b8d3a86d23fdb3e9e8547ac1 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Fri, 10 May 2024 10:57:27 -0500
Subject: [PATCH 13/18] Handle PR feedback.

---
 .../nimble-components/src/select/template.ts  |  1 +
 .../src/nimble/select/select.stories.ts       | 29 +++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/packages/nimble-components/src/select/template.ts b/packages/nimble-components/src/select/template.ts
index d12c108780..5f046ac706 100644
--- a/packages/nimble-components/src/select/template.ts
+++ b/packages/nimble-components/src/select/template.ts
@@ -69,6 +69,7 @@ SelectOptions
                     ${when(x => x.clearable && !x.displayPlaceholder && x.selectedIndex >= 0, html<Select>`
                         <${buttonTag} 
                             class="clear-button"
+                            tabindex="-1"
                             part="clear-button"
                             content-hidden
                             appearance="ghost"
diff --git a/packages/storybook/src/nimble/select/select.stories.ts b/packages/storybook/src/nimble/select/select.stories.ts
index be21a3cfa3..c99273abac 100644
--- a/packages/storybook/src/nimble/select/select.stories.ts
+++ b/packages/storybook/src/nimble/select/select.stories.ts
@@ -67,6 +67,18 @@ const optionSets = {
     [ExampleOptionsType.manyOptions]: manyOptions
 } as const;
 
+const dropdownPositionDescription = `
+The \`dropDownPosition\` attribute controls the position of the dropdown relative to the \`Select\`. The default is \`below\`, which will display the dropdown below the \`Select\`. The \`above\` setting will display the dropdown above the \`Select\`.
+`;
+
+const appearanceDescription = `
+This attribute affects the appearance of the \`Select\`. The default appearance is \`underline\`, which displays a line beneath the selected value. The \`outline\` appearance displays a border around the entire component. The \`block\` appearance applies a background for the entire component.
+`;
+
+const errorTextDescription = `
+A message to be displayed when the text field is in the invalid state explaining why the value is invalid.
+`;
+
 const filterModeDescription = `
 This attribute controls the filtering behavior of the \`Select\`. The default of \`none\` results in a dropdown with no input for filtering. A non-'none' setting results in a search input placed at the top or the bottom of the dropdown when opened (depending on where the popup is shown relative to the component). The \`standard\` setting will perform a case-insensitive and diacritic-insensitive filtering of the available options anywhere within the text of each option. 
 
@@ -79,6 +91,10 @@ To display placeholder text within the \`Select\` you must provide an option tha
 Any \`Select\` without a default selected option should provide placeholder text. Placeholder text should always follow the pattern "Select [thing(s)]", for example "Select country". Use sentence casing and don't include punctuation at the end of the prompt.
 `;
 
+const clearableDescription = `
+When the \`clearable\` attribute is set, a clear button will be displayed in the \`Select\` when a value is selected. Clicking the clear button will clear the selected value and display the placeholder text, if available, or will result in a blank display.
+`;
+
 const metadata: Meta<SelectArgs> = {
     title: 'Components/Select',
     decorators: [withActions<HtmlRenderer>],
@@ -124,11 +140,13 @@ const metadata: Meta<SelectArgs> = {
     argTypes: {
         dropDownPosition: {
             options: ['above', 'below'],
-            control: { type: 'select' }
+            control: { type: 'select' },
+            description: dropdownPositionDescription
         },
         appearance: {
             options: Object.values(DropdownAppearance),
-            control: { type: 'radio' }
+            control: { type: 'radio' },
+            description: appearanceDescription
         },
         filterMode: {
             options: Object.keys(FilterMode),
@@ -137,7 +155,8 @@ const metadata: Meta<SelectArgs> = {
             description: filterModeDescription
         },
         errorText: {
-            name: 'error-text'
+            name: 'error-text',
+            description: errorTextDescription
         },
         errorVisible: {
             name: 'error-visible'
@@ -146,6 +165,10 @@ const metadata: Meta<SelectArgs> = {
             name: 'placeholder',
             description: placeholderDescription
         },
+        clearable: {
+            name: 'clearable',
+            description: clearableDescription
+        },
         optionsType: {
             name: 'options',
             options: Object.values(ExampleOptionsType),

From 2d5b25da534a4524b053139c5ae2b02773375a7e Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Fri, 10 May 2024 11:31:52 -0500
Subject: [PATCH 14/18] Handle pressing Esc to clear value when dropdown is
 closed.

---
 packages/nimble-components/src/select/index.ts     |  6 ++++++
 .../src/select/tests/select.spec.ts                | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
index 6e0a49959f..c29b2c0664 100644
--- a/packages/nimble-components/src/select/index.ts
+++ b/packages/nimble-components/src/select/index.ts
@@ -564,6 +564,12 @@ export class Select
             }
             case keyEscape: {
                 if (!this.open) {
+                    if (this.clearable) {
+                        this.clearSelect();
+                        this.updateValue(true);
+                        return true;
+                    }
+
                     break;
                 }
 
diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 8f45f16514..26677fe6fa 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -1285,6 +1285,20 @@ describe('Select', () => {
             expect(changeEvent).toHaveBeenCalledTimes(1);
         });
 
+        it('if dropdown open pressing <Esc> closes dropdown and does not clear value', async () => {
+            pageObject.clickSelect();
+            await waitForUpdatesAsync();
+            pageObject.pressEscapeKey();
+            expect(element.open).toBeFalse();
+            expect(element.value).toBe('one');
+        });
+
+        it('if dropdown is closed, pressing <Esc> clears value', async () => {
+            pageObject.pressEscapeKey();
+            await waitForUpdatesAsync();
+            expect(element.value).toBe('');
+        });
+
         describe('without placeholder', () => {
             it('after clicking clear button, display text is empty and clear button is hidden', async () => {
                 pageObject.clickClearButton();

From 9a8143cf9bae2a829238ebfdaf23c786a2e76b7b Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Fri, 10 May 2024 14:28:49 -0500
Subject: [PATCH 15/18] Handle PR feedback.

---
 .../nimble-components/src/select/template.ts  |  2 +-
 .../src/select/tests/select.spec.ts           | 22 +++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/packages/nimble-components/src/select/template.ts b/packages/nimble-components/src/select/template.ts
index 5f046ac706..ab8fc1539b 100644
--- a/packages/nimble-components/src/select/template.ts
+++ b/packages/nimble-components/src/select/template.ts
@@ -66,7 +66,7 @@ SelectOptions
                     <div class="selected-value ${x => (x.displayPlaceholder ? 'placeholder' : '')}" part="selected-value" ${overflow('hasOverflow')} title=${x => (x.hasOverflow && x.displayValue ? x.displayValue : null)}>
                         <slot name="selected-value">${x => x.displayValue}</slot>
                     </div>
-                    ${when(x => x.clearable && !x.displayPlaceholder && x.selectedIndex >= 0, html<Select>`
+                    ${when(x => !x.disabled && x.clearable && !x.displayPlaceholder && x.selectedIndex >= 0, html<Select>`
                         <${buttonTag} 
                             class="clear-button"
                             tabindex="-1"
diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 26677fe6fa..2baad75f81 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -12,6 +12,7 @@ import {
     waitAnimationFrame
 } from '../../utilities/tests/component';
 import { filterSearchLabel } from '../../label-provider/core/label-tokens';
+import type { Button } from '../../button';
 
 const disabledOption = 'disabled';
 const disabledSelectedOption = 'disabled selected';
@@ -1228,6 +1229,16 @@ describe('Select', () => {
 
             expect(pageObject.getDisplayText()).toBe('One');
         });
+
+        it('pressing ESC does not clear placeholder text', async () => {
+            pageObject.pressEscapeKey();
+            await waitForUpdatesAsync();
+            expect(pageObject.getDisplayText()).toBe('One');
+        });
+
+        it('clear button is not visible when placeholder is selected', () => {
+            expect(pageObject.isClearButtonVisible()).toBeFalse();
+        });
     });
 
     describe('clearable', () => {
@@ -1252,6 +1263,17 @@ describe('Select', () => {
             expect(pageObject.isClearButtonVisible()).toBeTrue();
         });
 
+        it('when select is disabled, clear button is not visible', async () => {
+            element.disabled = true;
+            await waitForUpdatesAsync();
+            expect(pageObject.isClearButtonVisible()).toBeFalse();
+        });
+
+        it('can not Tab to clear button', () => {
+            const clearButton = element.shadowRoot?.querySelector<Button>('.clear-button');
+            expect(clearButton?.getAttribute('tabindex')).toBe('-1');
+        });
+
         it('clear button is visible after selecting an option', async () => {
             pageObject.clickClearButton();
             await waitForUpdatesAsync();

From ae4fa8b3c549cb322daffa3ee33a1e9805284742 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Fri, 10 May 2024 14:34:12 -0500
Subject: [PATCH 16/18] Update docs.

---
 packages/storybook/src/nimble/select/select.stories.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/storybook/src/nimble/select/select.stories.ts b/packages/storybook/src/nimble/select/select.stories.ts
index c99273abac..f433cb1f31 100644
--- a/packages/storybook/src/nimble/select/select.stories.ts
+++ b/packages/storybook/src/nimble/select/select.stories.ts
@@ -86,7 +86,7 @@ It is recommended that if the \`Select\` has 15 or fewer options that you use th
 `;
 
 const placeholderDescription = `
-To display placeholder text within the \`Select\` you must provide an option that has the \`disabled\`, \`selected\` and \`hidden\` attributes set. This option will not be available in the dropdown, and its contents will be used as the placeholder text.
+To display placeholder text within the \`Select\` you must provide an option that has the \`disabled\`, \`selected\` and \`hidden\` attributes set. This option will not be available in the dropdown, and its contents will be used as the placeholder text. Note that giving the placeholder an initial \`selected\` state is only necessary to display the placeholder initially. If another option is selected initially the placeholder will be displayed upon clearing the current value.
 
 Any \`Select\` without a default selected option should provide placeholder text. Placeholder text should always follow the pattern "Select [thing(s)]", for example "Select country". Use sentence casing and don't include punctuation at the end of the prompt.
 `;

From 3e682d35fe44506cd74523bf6df25abdcdf515c9 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Wed, 15 May 2024 14:50:10 -0500
Subject: [PATCH 17/18] Update
 packages/nimble-components/src/select/tests/select.spec.ts

Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
---
 packages/nimble-components/src/select/tests/select.spec.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 55e9497b25..2fdd63e197 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -1384,7 +1384,7 @@ describe('Select', () => {
                         option.disabled = false;
                     }
                 }
-            ];
+            ] as const;
             parameterizeSpec(removePlaceholderData, (spec, name, value) => {
                 spec(name, async () => {
                     value.updatePlaceholder(element.options[0]! as ListOption);

From f618689e63a92d072396e30a8b436278bee56989 Mon Sep 17 00:00:00 2001
From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com>
Date: Wed, 15 May 2024 15:22:33 -0500
Subject: [PATCH 18/18] Handle PR feedback

---
 .../src/select/tests/select.spec.ts                | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts
index 2fdd63e197..4a0fe06a69 100644
--- a/packages/nimble-components/src/select/tests/select.spec.ts
+++ b/packages/nimble-components/src/select/tests/select.spec.ts
@@ -235,35 +235,35 @@ describe('Select', () => {
             await disconnect();
         });
 
-        const defaultOptionTestCases: {
-            name: string,
-            option1State?: OptionInitialState,
-            option2State?: OptionInitialState,
-            expected: string
-        }[] = [
+        const defaultOptionTestCases = [
             {
                 name: 'first option selected, first is default',
                 option1State: 'selected',
+                option2State: undefined,
                 expected: 'one'
             },
             {
                 name: 'first option disabled, second is default',
                 option1State: 'disabled',
+                option2State: undefined,
                 expected: 'two'
             },
             {
                 name: 'first option hidden, second is default',
                 option1State: 'hidden',
+                option2State: undefined,
                 expected: 'two'
             },
             {
                 name: 'first option visually-hidden, second is default',
                 option1State: 'visually-hidden',
+                option2State: undefined,
                 expected: 'two'
             },
             {
                 name: 'first option hidden and disabled, first is default as placeholder',
                 option1State: 'disabled hidden',
+                option2State: undefined,
                 expected: 'one'
             },
             {
@@ -284,7 +284,7 @@ describe('Select', () => {
                 option2State: 'selected',
                 expected: 'two'
             }
-        ];
+        ] as const;
         parameterizeSpec(defaultOptionTestCases, (spec, name, value) => {
             spec(name, async () => {
                 const { element, connect, disconnect } = await setup(