Store reference to the dropdown's active item

Avoids the array `Array#find` call, fixing IE11 support. Fixes #124
yuku · Dec 2, 2017 · 6d716c4 · 6d716c4
1 parent 0f66ea0
commit 6d716c4
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 21 deletions.
diff --git a/src/dropdown.js b/src/dropdown.js
@@ -28,6 +28,7 @@ export type DropdownOptions = {
 export default class Dropdown extends EventEmitter {
   shown: boolean
   items: DropdownItem[]
+  activeItem: DropdownItem | null
   footer: $PropertyType<DropdownOptions, "footer">
   header: $PropertyType<DropdownOptions, "header">
   maxCount: $PropertyType<DropdownOptions, "maxCount">
@@ -52,6 +53,7 @@ export default class Dropdown extends EventEmitter {
     super()
     this.shown = false
     this.items = []
+    this.activeItem = null
     this.footer = options.footer
     this.header = options.header
     this.maxCount = options.maxCount || 10
@@ -155,8 +157,8 @@ export default class Dropdown extends EventEmitter {
   /**
    * Retrieve the active item.
    */
-  getActiveItem(): ?DropdownItem {
-    return this.items.find(item => item.active)
+  getActiveItem(): DropdownItem | null {
+    return this.activeItem
   }
 
   /**
@@ -246,15 +248,13 @@ export default class Dropdown extends EventEmitter {
   }
 
   /** @private */
-  moveActiveItem(name: "next" | "prev", e: CustomEvent) {
-    const activeItem: any = this.getActiveItem()
-    let nextActiveItem
-    if (activeItem) {
-      nextActiveItem = activeItem[name]
-    } else {
-      nextActiveItem =
-        name === "next" ? this.items[0] : this.items[this.items.length - 1]
-    }
+  moveActiveItem(direction: "next" | "prev", e: CustomEvent) {
+    const nextActiveItem =
+      direction === "next"
+        ? this.activeItem ? this.activeItem.next : this.items[0]
+        : this.activeItem
+          ? this.activeItem.prev
+          : this.items[this.items.length - 1]
     if (nextActiveItem) {
       nextActiveItem.activate()
       e.preventDefault()

diff --git a/src/dropdown_item.js b/src/dropdown_item.js
@@ -50,6 +50,9 @@ export default class DropdownItem {
     this.el.removeEventListener("mousedown", this.onClick, false)
     this.el.removeEventListener("mouseover", this.onMouseover, false)
     this.el.removeEventListener("touchstart", this.onClick, false)
+    if (this.active) {
+      this.dropdown.activeItem = null
+    }
     // This element has already been removed by {@link Dropdown#clear}.
     this._el = null
   }
@@ -76,6 +79,7 @@ export default class DropdownItem {
       if (activeItem) {
         activeItem.deactivate()
       }
+      this.dropdown.activeItem = this
       this.active = true
       this.el.className = ACTIVE_CLASS_NAME
     }
@@ -119,6 +123,7 @@ export default class DropdownItem {
     if (this.active) {
       this.active = false
       this.el.className = CLASS_NAME
+      this.dropdown.activeItem = null
     }
     return this
   }

diff --git a/test/unit/dropdown_spec.js b/test/unit/dropdown_spec.js
@@ -3,7 +3,6 @@ require("../test_helper")
 import Dropdown from "../../src/dropdown"
 import DropdownItem from "../../src/dropdown_item"
 import { createSearchResult } from "../test_utils"
-import isUndefined from "lodash.isundefined"
 
 const assert = require("power-assert")
 
@@ -65,7 +64,6 @@ describe("Dropdown", function() {
       dropdown = new Dropdown({})
       assert.strictEqual(subject(), dropdown)
     })
-
     ;["render", "rendered"].forEach(name => {
       it(`should emit ${name} event`, function() {
         var spy = this.sinon.spy()
@@ -106,7 +104,6 @@ describe("Dropdown", function() {
           subject()
           assert(dropdown.shown)
         })
-
         ;["show", "shown"].forEach(eventName => {
           it(`should emit ${eventName} event`, function() {
             var spy = this.sinon.spy()
@@ -132,7 +129,6 @@ describe("Dropdown", function() {
           dropdown = new Dropdown({})
           dropdown.shown = true
         })
-
         ;["show", "shown"].forEach(eventName => {
           it(`should not emit ${eventName} event`, function() {
             var spy = this.sinon.spy()
@@ -156,7 +152,6 @@ describe("Dropdown", function() {
           dropdown = new Dropdown({})
           dropdown.shown = true
         })
-
         ;["show", "shown"].forEach(eventName => {
           it(`should not emit ${eventName} event`, function() {
             var spy = this.sinon.spy()
@@ -172,7 +167,6 @@ describe("Dropdown", function() {
           dropdown = new Dropdown({})
           dropdown.shown = false
         })
-
         ;["show", "shown"].forEach(eventName => {
           it(`should emit ${eventName} event`, function() {
             var spy = this.sinon.spy()
@@ -304,7 +298,6 @@ describe("Dropdown", function() {
         subject()
         assert(!dropdown.shown)
       })
-
       ;["hide", "hidden"].forEach(eventName => {
         it(`should emit ${eventName} event`, function() {
           var spy = this.sinon.spy()
@@ -357,7 +350,6 @@ describe("Dropdown", function() {
       subject()
       assert(stub.calledOnce)
     })
-
     ;["select", "selected"].forEach(name => {
       it(`should emit a ${name} event`, function() {
         var spy = this.sinon.spy()
@@ -527,8 +519,8 @@ describe("Dropdown", function() {
     }
 
     context("without active item", function() {
-      it("should return undefined", function() {
-        assert(isUndefined(subject()))
+      it("should return null", function() {
+        assert(subject() === null)
       })
     })