Highlight first item in menus upon filtering (#2632)

If the user moves through the menu then resumes typing, the
`selectedIndex` can end up larger than the contents of the menu.  This
can cause errors, or at least graphical glitches, and seems odd.

browser/src/Services/ContextMenu/ContextMenu.tsx

@@ -160,10 +160,6 @@ export class ContextMenu {
         )
     }
 
-    public updateItem(item: any): void {
-        this._actions.setDetailedMenuItem(item)
-    }
-
     public hide(): void {
         this._actions.hidePopupMenu()
     }

browser/src/Services/Menu/MenuActionCreators.ts

@@ -66,13 +66,6 @@ export const setMenuItems = (id: string, items: any[]) => ({
     },
 })
 
-export const setDetailedMenuItem = (item: any) => ({
-    type: "SET_DETAILED_MENU_ITEM",
-    payload: {
-        detailedItem: item,
-    },
-})
-
 export const hidePopupMenu = () => (dispatch: any, getState: any) => {
     const state = getState()
 

browser/src/Services/Menu/MenuActions.ts

@@ -33,13 +33,6 @@ export interface IShowMenuAction {
     }
 }
 
-export interface ISetDetailedMenuItem<T> {
-    type: "SET_DETAILED_MENU_ITEM"
-    payload: {
-        detailedItem: T
-    }
-}
-
 export interface ISetMenuItems<T> {
     type: "SET_MENU_ITEMS"
     payload: {

browser/src/Services/Menu/MenuReducer.ts

@@ -63,32 +63,6 @@ export function createReducer<T, FilteredT extends T>() {
                     isLoading: false,
                 }
             }
-            case "SET_DETAILED_MENU_ITEM": {
-                if (!s || !s.options) {
-                    return s
-                }
-
-                if (!a.payload.detailedItem) {
-                    return s
-                }
-
-                const options = s.options.map(entry => {
-                    // TODO: Decide on canonical interface for menu options
-                    if ((entry as any).label === a.payload.detailedItem.label) {
-                        return a.payload.detailedItem
-                    } else {
-                        return entry
-                    }
-                })
-
-                const filterFunc = s.filterFunction
-                const filteredOptions2 = filterFunc(options, s.filter)
-                return {
-                    ...s,
-                    options,
-                    filteredOptions: filteredOptions2,
-                }
-            }
             case "SET_MENU_ITEMS": {
                 if (!s || s.id !== a.payload.id) {
                     return s
@@ -129,6 +103,8 @@ export function createReducer<T, FilteredT extends T>() {
                     return s
                 }
 
+                // Note that for language server completions, the `filterFunc` is a no-op - the
+                // items are filtered elsewhere (but this FILTER_MENU action is still dispatched).
                 const filterFunc = s.filterFunction
                 const filteredOptionsSorted = filterFunc(s.options, a.payload.filter)
 
@@ -136,6 +112,7 @@ export function createReducer<T, FilteredT extends T>() {
                     ...s,
                     filter: a.payload.filter,
                     filteredOptions: filteredOptionsSorted,
+                    selectedIndex: 0,
                 }
             }
             default:

browser/test/Services/Menu/MenuReducerTests.ts

@@ -0,0 +1,48 @@
+import * as assert from "assert"
+import * as sinon from "sinon"
+
+import { createReducer } from "../../../src/Services/Menu/MenuReducer"
+import { createDefaultState } from "../../../src/Services/Menu/MenuState"
+
+describe("MenuReducer", () => {
+    let reducer: any
+    let oldState: any
+
+    beforeEach(() => {
+        reducer = createReducer<any, any>()
+        oldState = createDefaultState<any, any>()
+        oldState.menu = {}
+    })
+
+    describe("popupMenuReducer", () => {
+        describe("FILTER_MENU", () => {
+            let filteredOptions: any
+            let filterFunction: any
+
+            beforeEach(() => {
+                filteredOptions = {}
+                filterFunction = sinon.stub().returns(filteredOptions)
+                oldState.menu.filterFunction = filterFunction
+            })
+
+            it("resets selectedIndex to zero", () => {
+                oldState.menu.selectedIndex = 123
+
+                const newState = reducer(oldState, {
+                    type: "FILTER_MENU",
+                    payload: { filter: "mock filter" },
+                })
+
+                assert.deepStrictEqual(newState, {
+                    ...oldState,
+                    menu: {
+                        filter: "mock filter",
+                        filterFunction,
+                        filteredOptions,
+                        selectedIndex: 0,
+                    },
+                })
+            })
+        })
+    })
+})

test/ci/AutoCompletionTest-TypeScript.ts

@@ -18,7 +18,7 @@ export const test = async (oni: Oni.Plugin.Api) => {
     oni.automation.sendKeys("window.a")
 
     // Wait for completion popup to show
-    await oni.automation.waitFor(() => getCompletionElement() !== null)
+    await oni.automation.waitFor(() => getCompletionElement() !== null, 120000)
 
     // Check for 'alert' as an available completion
     const completionElement = getCompletionElement()

test/ci/Configuration.TypeScriptEditor.CompletionTest.ts

@@ -44,7 +44,7 @@ export const test = async (oni: Oni.Plugin.Api) => {
     const hasCompletionElement = () =>
         getCompletionElement() && getCompletionElement().textContent.indexOf("automation") >= 0
 
-    await oni.automation.waitFor(hasCompletionElement)
+    await oni.automation.waitFor(hasCompletionElement, 120000)
 
     assert.ok(hasCompletionElement(), "Got completion element!")
 }

test/ci/Editor.NextPreviousErrorTest.ts

@@ -29,7 +29,7 @@ export const test = async (oni: Oni.Plugin.Api) => {
             errors[filePath]["language-typescript"] &&
             errors[filePath]["language-typescript"].length === 3
         )
-    })
+    }, 120000)
 
     // nextError jumps to 1st error
     oni.commands.executeCommand("oni.editor.nextError")

test/ci/TextmateHighlighting.DebugScopesTest.ts

@@ -29,7 +29,7 @@ export const test = async (oni: Oni.Plugin.Api) => {
             types.Position.create(0, 0),
         )
         return tokens && tokens.scopes && tokens.scopes.length > 0
-    }, 10000)
+    }, 120000)
 
     oni.automation.sendKeys("w")
 
@@ -37,7 +37,7 @@ export const test = async (oni: Oni.Plugin.Api) => {
     await oni.automation.waitFor(() => {
         element = getElementByClassName("quick-info-debug-scopes")
         return !!element
-    })
+    }, 120000)
 
     await oni.automation.waitFor(() => {
         const items = element.getElementsByTagName("li")