Handle PR feedback.

ni · Apr 29, 2024 · 7d4ba9a · 7d4ba9a
1 parent 042d2b0
commit 7d4ba9a
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 35 deletions.
diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts
@@ -26,7 +26,6 @@ import {
     keyEscape,
     keyHome,
     keySpace,
-    keyTab,
     uniqueId
 } from '@microsoft/fast-web-utilities';
 import { arrowExpanderDown16X16 } from '@ni/nimble-tokens/dist/icons/js';
@@ -474,22 +473,19 @@ export class Select
         this.filterOptions();
 
         const enabledOptions = this.filteredOptions.filter(o => !o.disabled);
-        let selectedOptionIndex = this.filter !== ''
+        let activeOptionIndex = this.filter !== ''
             ? this.openActiveIndex ?? this.selectedIndex
             : this.selectedIndex;
 
         if (
             enabledOptions.length > 0
-            && (this.selectedIndex < 0
-                || !enabledOptions.find(
-                    o => o === this.options[selectedOptionIndex]
-                ))
+            && !enabledOptions.find(o => o === this.options[activeOptionIndex])
         ) {
-            selectedOptionIndex = this.options.indexOf(enabledOptions[0]!);
+            activeOptionIndex = this.options.indexOf(enabledOptions[0]!);
         } else if (enabledOptions.length === 0) {
-            selectedOptionIndex = -1;
+            activeOptionIndex = -1;
         }
-        this.setActiveOption(selectedOptionIndex);
+        this.setActiveOption(activeOptionIndex);
 
         if (e.inputType.includes('deleteContent') || !this.filter.length) {
             return true;
@@ -519,11 +515,6 @@ export class Select
             this.open = false;
             if (currentActiveIndex === -1) {
                 currentActiveIndex = this.selectedIndex;
-                if (this.selectedIndex >= 0) {
-                    (
-                        this.options[this.selectedIndex]! as ListOption
-                    ).activeOption = true;
-                }
             }
 
             if (this.selectedIndex !== currentActiveIndex) {
@@ -597,20 +588,6 @@ export class Select
                 this.focus();
                 break;
             }
-            case keyTab: {
-                if (this.collapsible && this.open) {
-                    if (
-                        this.filteredOptions.length === 0
-                        || this.filteredOptions.every(o => o.disabled)
-                    ) {
-                        this.open = false;
-                        return true;
-                    }
-                }
-
-                this.open = false;
-                break;
-            }
 
             default: {
                 break;
@@ -661,13 +638,13 @@ export class Select
             const typeaheadMatches = this.getTypeaheadMatches();
 
             if (typeaheadMatches.length) {
-                const selectedIndex = this.options.indexOf(
+                const activeOption = this.options.indexOf(
                     typeaheadMatches[0] as ListOption
                 );
-                if (selectedIndex > -1 && !this.open) {
-                    this.selectedIndex = selectedIndex;
+                if (activeOption > -1 && !this.open) {
+                    this.selectedIndex = activeOption;
                 } else if (this.filterMode === FilterMode.none) {
-                    this.setActiveOption(selectedIndex);
+                    this.setActiveOption(activeOption);
                 }
             }
 
@@ -1080,7 +1057,7 @@ export class Select
     private initializeOpenState(): void {
         this.openActiveIndex = this.selectedIndex;
         this.committedSelectedOption = this.options[this.selectedIndex];
-        this.setActiveOption(this.openActiveIndex ?? 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
@@ -123,7 +123,7 @@ export const styles = css`
     }
 
     ::slotted([role='option']:not([active-option])) {
-        background-color: none;
+        background: none;
     }
 
     ::slotted([role='option'][active-option]:hover) {

diff --git a/packages/nimble-components/src/select/testing/select.pageobject.ts b/packages/nimble-components/src/select/testing/select.pageobject.ts
@@ -149,6 +149,7 @@ export class SelectPageObject {
         this.selectElement.dispatchEvent(
             new KeyboardEvent('keydown', { key: keyTab })
         );
+        this.selectElement.dispatchEvent(new FocusEvent('focusout'));
     }
 
     public pressArrowDownKey(): void {

diff --git a/packages/nimble-components/src/select/tests/select-matrix.stories.ts b/packages/nimble-components/src/select/tests/select-matrix.stories.ts
@@ -1,5 +1,6 @@
 import type { StoryFn, Meta } from '@storybook/html';
 import { html, ViewTemplate } from '@microsoft/fast-element';
+import { keyArrowDown } from '@microsoft/fast-web-utilities';
 import { createStory } from '../../utilities/tests/storybook';
 import {
     createMatrixThemeStory,
@@ -21,9 +22,10 @@ import {
     menuMinWidth,
     standardPadding
 } from '../../theme-provider/design-tokens';
-import { selectTag } from '..';
+import { Select, selectTag } from '..';
 import { listOptionTag } from '../../list-option';
 import { loremIpsum } from '../../utilities/tests/lorem-ipsum';
+import { waitForUpdatesAsync } from '../../testing/async-helpers';
 
 const appearanceStates = [
     ['Underline', DropdownAppearance.underline],
@@ -101,6 +103,29 @@ export const blankListOption: StoryFn = createStory(
     </${selectTag}>`
 );
 
+const playFunction = async (): Promise<void> => {
+    await Promise.all(
+        Array.from(document.querySelectorAll<Select>('nimble-select')).map(
+            async select => {
+                const arrowDownEvent = new KeyboardEvent('keydown', {
+                    key: keyArrowDown
+                });
+                select.dispatchEvent(arrowDownEvent);
+                await waitForUpdatesAsync();
+            }
+        )
+    );
+};
+
+export const navigateToDifferentOption: StoryFn = createStory(
+    html`<${selectTag} open>
+        <${listOptionTag} value="1" selected>Option 1</${listOptionTag}>
+        <${listOptionTag}>Option 2</${listOptionTag}>
+    </${selectTag}>`
+);
+
+navigateToDifferentOption.play = playFunction;
+
 export const textCustomized: StoryFn = createMatrixThemeStory(
     textCustomizationWrapper(
         html`