Search frontend cleanup (#2849)

- Convert KeyboardNavigatable to TypeScript, as that is used internally here.
- Convert search sources to TypeScript
- Convert Search component to TypeScript
- Convert Search States to Typescript
- Add `getInitialSearch` to `SearchState`
- Fix search disappearing on page reload / direct link

framework/core/js/src/forum/compat.js

@@ -51,7 +51,6 @@ import PostPreview from './components/PostPreview';
 import EventPost from './components/EventPost';
 import DiscussionHero from './components/DiscussionHero';
 import PostMeta from './components/PostMeta';
-import SearchSource from './components/SearchSource';
 import DiscussionRenamedPost from './components/DiscussionRenamedPost';
 import DiscussionComposer from './components/DiscussionComposer';
 import LogInButtons from './components/LogInButtons';
@@ -127,7 +126,6 @@ export default Object.assign(compat, {
   'components/EventPost': EventPost,
   'components/DiscussionHero': DiscussionHero,
   'components/PostMeta': PostMeta,
-  'components/SearchSource': SearchSource,
   'components/DiscussionRenamedPost': DiscussionRenamedPost,
   'components/DiscussionComposer': DiscussionComposer,
   'components/LogInButtons': LogInButtons,

framework/core/js/src/forum/components/DiscussionsSearchSource.js

@@ -1,60 +0,0 @@
-import highlight from '../../common/helpers/highlight';
-import LinkButton from '../../common/components/LinkButton';
-import Link from '../../common/components/Link';
-
-/**
- * The `DiscussionsSearchSource` finds and displays discussion search results in
- * the search dropdown.
- *
- * @implements SearchSource
- */
-export default class DiscussionsSearchSource {
-  constructor() {
-    this.results = {};
-  }
-
-  search(query) {
-    query = query.toLowerCase();
-
-    this.results[query] = [];
-
-    const params = {
-      filter: { q: query },
-      page: { limit: 3 },
-      include: 'mostRelevantPost',
-    };
-
-    return app.store.find('discussions', params).then((results) => (this.results[query] = results));
-  }
-
-  view(query) {
-    query = query.toLowerCase();
-
-    const results = this.results[query] || [];
-
-    return [
-      <li className="Dropdown-header">{app.translator.trans('core.forum.search.discussions_heading')}</li>,
-      <li>
-        {LinkButton.component(
-          {
-            icon: 'fas fa-search',
-            href: app.route('index', { q: query }),
-          },
-          app.translator.trans('core.forum.search.all_discussions_button', { query })
-        )}
-      </li>,
-      results.map((discussion) => {
-        const mostRelevantPost = discussion.mostRelevantPost();
-
-        return (
-          <li className="DiscussionSearchResult" data-index={'discussions' + discussion.id()}>
-            <Link href={app.route.discussion(discussion, mostRelevantPost && mostRelevantPost.number())}>
-              <div className="DiscussionSearchResult-title">{highlight(discussion.title(), query)}</div>
-              {mostRelevantPost ? <div className="DiscussionSearchResult-excerpt">{highlight(mostRelevantPost.contentPlain(), query, 100)}</div> : ''}
-            </Link>
-          </li>
-        );
-      }),
-    ];
-  }
-}

framework/core/js/src/forum/components/DiscussionsSearchSource.tsx

@@ -0,0 +1,54 @@
+import highlight from '../../common/helpers/highlight';
+import LinkButton from '../../common/components/LinkButton';
+import Link from '../../common/components/Link';
+import { SearchSource } from './Search';
+import Mithril from 'mithril';
+
+/**
+ * The `DiscussionsSearchSource` finds and displays discussion search results in
+ * the search dropdown.
+ */
+export default class DiscussionsSearchSource implements SearchSource {
+  protected results = new Map<string, unknown[]>();
+
+  search(query: string) {
+    query = query.toLowerCase();
+
+    this.results.set(query, []);
+
+    const params = {
+      filter: { q: query },
+      page: { limit: 3 },
+      include: 'mostRelevantPost',
+    };
+
+    return app.store.find('discussions', params).then((results) => this.results.set(query, results));
+  }
+
+  view(query: string): Array<Mithril.Vnode> {
+    query = query.toLowerCase();
+
+    const results = (this.results.get(query) || []).map((discussion: unknown) => {
+      const mostRelevantPost = discussion.mostRelevantPost();
+
+      return (
+        <li className="DiscussionSearchResult" data-index={'discussions' + discussion.id()}>
+          <Link href={app.route.discussion(discussion, mostRelevantPost && mostRelevantPost.number())}>
+            <div className="DiscussionSearchResult-title">{highlight(discussion.title(), query)}</div>
+            {mostRelevantPost ? <div className="DiscussionSearchResult-excerpt">{highlight(mostRelevantPost.contentPlain(), query, 100)}</div> : ''}
+          </Link>
+        </li>
+      );
+    }) as Array<Mithril.Vnode>;
+
+    return [
+      <li className="Dropdown-header">{app.translator.trans('core.forum.search.discussions_heading')}</li>,
+      <li>
+        <LinkButton icon="fas fa-search" href={app.route('index', { q: query })}>
+          {app.translator.trans('core.forum.search.all_discussions_button', { query })}
+        </LinkButton>
+      </li>,
+      ...results,
+    ];
+  }
+}

framework/core/js/src/forum/components/Search.tsx

@@ -1,12 +1,42 @@
-import Component from '../../common/Component';
+import Component, { ComponentAttrs } from '../../common/Component';
 import LoadingIndicator from '../../common/components/LoadingIndicator';
 import ItemList from '../../common/utils/ItemList';
 import classList from '../../common/utils/classList';
 import extractText from '../../common/utils/extractText';
 import KeyboardNavigatable from '../utils/KeyboardNavigatable';
 import icon from '../../common/helpers/icon';
+import SearchState from '../states/SearchState';
 import DiscussionsSearchSource from './DiscussionsSearchSource';
 import UsersSearchSource from './UsersSearchSource';
+import Mithril from 'mithril';
+
+/**
+ * The `SearchSource` interface defines a section of search results in the
+ * search dropdown.
+ *
+ * Search sources should be registered with the `Search` component class
+ * by extending the `sourceItems` method. When the user types a
+ * query, each search source will be prompted to load search results via the
+ * `search` method. When the dropdown is redrawn, it will be constructed by
+ * putting together the output from the `view` method of each source.
+ */
+export interface SearchSource {
+  /**
+   * Make a request to get results for the given query.
+   */
+  search(query: string);
+
+  /**
+   * Get an array of virtual <li>s that list the search results for the given
+   * query.
+   */
+  view(query: string): Array<Mithril.Vnode>;
+}
+
+export interface SearchAttrs extends ComponentAttrs {
+  /** The type of alert this is. Will be used to give the alert a class name of `Alert--{type}`. */
+  state: SearchState;
+}
 
 /**
  * The `Search` component displays a menu of as-you-type results from a variety
@@ -20,43 +50,44 @@ import UsersSearchSource from './UsersSearchSource';
  *
  * - state: SearchState instance.
  */
-export default class Search extends Component {
+export default class Search<T extends SearchAttrs = SearchAttrs> extends Component<T> {
   static MIN_SEARCH_LEN = 3;
 
-  oninit(vnode) {
-    super.oninit(vnode);
-    this.state = this.attrs.state;
+  protected state!: SearchState;
 
   /**
    * Whether or not the search input has focus.
-     *
-     * @type {Boolean}
    */
-    this.hasFocus = false;
+  protected hasFocus = false;
 
   /**
    * An array of SearchSources.
-     *
-     * @type {SearchSource[]}
    */
-    this.sources = null;
+  protected sources!: SearchSource[];
 
   /**
    * The number of sources that are still loading results.
-     *
-     * @type {Integer}
    */
-    this.loadingSources = 0;
+  protected loadingSources = 0;
 
   /**
    * The index of the currently-selected <li> in the results list. This can be
    * a unique string (to account for the fact that an item's position may jump
    * around as new results load), but otherwise it will be numeric (the
    * sequential position within the list).
-     *
-     * @type {String|Integer}
    */
-    this.index = 0;
+  protected index: number = 0;
+
+  protected navigator!: KeyboardNavigatable;
+
+  protected searchTimeout?: number;
+
+  private updateMaxHeightHandler?: () => void;
+
+  oninit(vnode: Mithril.Vnode<T, this>) {
+    super.oninit(vnode);
+
+    this.state = this.attrs.state;
   }
 
   view() {
@@ -64,9 +95,7 @@ export default class Search extends Component {
 
     // Initialize search sources in the view rather than the constructor so
     // that we have access to app.forum.
-    if (!this.sources) {
-      this.sources = this.sourceItems().toArray();
-    }
+    if (!this.sources) this.sources = this.sourceItems().toArray();
 
     // Hide the search view if no sources were loaded
     if (!this.sources.length) return <div></div>;
@@ -76,15 +105,13 @@ export default class Search extends Component {
     return (
       <div
         role="search"
-        className={
-          'Search ' +
-          classList({
+        className={classList({
+          Search: true,
           open: this.state.getValue() && this.hasFocus,
           focused: this.hasFocus,
           active: !!currentSearch,
           loading: !!this.loadingSources,
-          })
-        }
+        })}
       >
         <div className="Search-input">
           <input
@@ -153,7 +180,7 @@ export default class Search extends Component {
         search.setIndex(search.selectableItems().index(this));
       });
 
-    const $input = this.$('input');
+    const $input = this.$('input') as JQuery<HTMLInputElement>;
 
     this.navigator = new KeyboardNavigatable();
     this.navigator
@@ -233,10 +260,8 @@ export default class Search extends Component {
 
   /**
    * Build an item list of SearchSources.
-   *
-   * @return {ItemList}
    */
-  sourceItems() {
+  sourceItems(): ItemList {
     const items = new ItemList();
 
     if (app.forum.attribute('canViewDiscussions')) items.add('discussions', new DiscussionsSearchSource());
@@ -247,29 +272,22 @@ export default class Search extends Component {
 
   /**
    * Get all of the search result items that are selectable.
-   *
-   * @return {jQuery}
    */
-  selectableItems() {
+  selectableItems(): JQuery {
     return this.$('.Search-results > li:not(.Dropdown-header)');
   }
 
   /**
    * Get the position of the currently selected search result item.
-   *
-   * @return {Integer}
    */
-  getCurrentNumericIndex() {
+  getCurrentNumericIndex(): number {
     return this.selectableItems().index(this.getItem(this.index));
   }
 
   /**
    * Get the <li> in the search results with the given index (numeric or named).
-   *
-   * @param {String} index
-   * @return {DOMElement}
    */
-  getItem(index) {
+  getItem(index: number): JQuery {
     const $items = this.selectableItems();
     let $item = $items.filter(`[data-index="${index}"]`);
 
@@ -283,12 +301,8 @@ export default class Search extends Component {
   /**
    * Set the currently-selected search result item to the one with the given
    * index.
-   *
-   * @param {Integer} index
-   * @param {Boolean} scrollToItem Whether or not to scroll the dropdown so that
-   *     the item is in view.
    */
-  setIndex(index, scrollToItem) {
+  setIndex(index: number, scrollToItem: boolean = false) {
     const $items = this.selectableItems();
     const $dropdown = $items.parent();
 
@@ -301,7 +315,7 @@ export default class Search extends Component {
 
     const $item = $items.removeClass('active').eq(fixedIndex).addClass('active');
 
-    this.index = $item.attr('data-index') || fixedIndex;
+    this.index = parseInt($item.attr('data-index') as string) || fixedIndex;
 
     if (scrollToItem) {
       const dropdownScroll = $dropdown.scrollTop();

framework/core/js/src/forum/components/SearchSource.js

@@ -1,30 +0,0 @@
-/**
- * The `SearchSource` interface defines a section of search results in the
- * search dropdown.
- *
- * Search sources should be registered with the `Search` component class
- * by extending the `sourceItems` method. When the user types a
- * query, each search source will be prompted to load search results via the
- * `search` method. When the dropdown is redrawn, it will be constructed by
- * putting together the output from the `view` method of each source.
- *
- * @interface
- */
-export default class SearchSource {
-  /**
-   * Make a request to get results for the given query.
-   *
-   * @param {String} query
-   * @return {Promise}
-   */
-  search() {}
-
-  /**
-   * Get an array of virtual <li>s that list the search results for the given
-   * query.
-   *
-   * @param {String} query
-   * @return {Object}
-   */
-  view() {}
-}

framework/core/js/src/forum/components/UsersSearchSource.tsx

@@ -2,34 +2,32 @@ import highlight from '../../common/helpers/highlight';
 import avatar from '../../common/helpers/avatar';
 import username from '../../common/helpers/username';
 import Link from '../../common/components/Link';
+import { SearchSource } from './Search';
+import Mithril from 'mithril';
 
 /**
  * The `UsersSearchSource` finds and displays user search results in the search
  * dropdown.
- *
- * @implements SearchSource
  */
-export default class UsersSearchResults {
-  constructor() {
-    this.results = {};
-  }
+export default class UsersSearchResults implements SearchSource {
+  protected results = new Map<string, unknown[]>();
 
-  search(query) {
+  search(query: string) {
     return app.store
       .find('users', {
         filter: { q: query },
         page: { limit: 5 },
       })
       .then((results) => {
-        this.results[query] = results;
+        this.results.set(query, results);
         m.redraw();
       });
   }
 
-  view(query) {
+  view(query: string): Array<Mithril.Vnode> {
     query = query.toLowerCase();
 
-    const results = (this.results[query] || [])
+    const results = (this.results.get(query) || [])
       .concat(
         app.store
           .all('users')
@@ -38,14 +36,14 @@ export default class UsersSearchResults {
       .filter((e, i, arr) => arr.lastIndexOf(e) === i)
       .sort((a, b) => a.displayName().localeCompare(b.displayName()));
 
-    if (!results.length) return '';
+    if (!results.length) return [];
 
     return [
       <li className="Dropdown-header">{app.translator.trans('core.forum.search.users_heading')}</li>,
-      results.map((user) => {
+      ...results.map((user) => {
         const name = username(user);
 
-        const children = [highlight(name.text, query)];
+        const children = [highlight(name.text as string, query)];
 
         return (
           <li className="UserSearchResult" data-index={'users' + user.id()}>

framework/core/js/src/forum/states/GlobalSearchState.ts

@@ -1,19 +1,43 @@
 import setRouteWithForcedRefresh from '../../common/utils/setRouteWithForcedRefresh';
 import SearchState from './SearchState';
 
+type SearchParams = Record<string, string>;
+
 export default class GlobalSearchState extends SearchState {
+  private initialValueSet = false;
+
   constructor(cachedSearches = []) {
     super(cachedSearches);
   }
 
-  getValue() {
-    if (this.value === undefined) {
-      this.value = this.getInitialSearch() || '';
+  getValue(): string {
+    // If we are on a search results page, we should initialize the value
+    // from the current search, if one is present.
+    // We can't do this in the constructor, as this class is instantiated
+    // before pages are rendered, and we need app.current.
+    if (!this.initialValueSet && this.currPageProvidesSearch()) {
+      this.intializeValue();
     }
 
     return super.getValue();
   }
 
+  protected intializeValue() {
+    this.setValue(this.getInitialSearch());
+    this.initialValueSet = true;
+  }
+
+  protected currPageProvidesSearch(): boolean {
+    return app.current.type && app.current.type.providesInitialSearch;
+  }
+
+  /**
+   * @inheritdoc
+   */
+  getInitialSearch(): string {
+    return this.currPageProvidesSearch() ? this.params().q : '';
+  }
+
   /**
    * Clear the search input and the current controller's active search.
    */
@@ -27,12 +51,22 @@ export default class GlobalSearchState extends SearchState {
     }
   }
 
+  /**
+   * Redirect to the index page without a search filter. This is called when the
+   * 'x' is clicked in the search box in the header.
+   */
+  protected clearInitialSearch() {
+    const { q, ...params } = this.params();
+
+    setRouteWithForcedRefresh(app.route(app.current.get('routeName'), params));
+  }
+
   /**
    * Get URL parameters that stick between filter changes.
    *
-   * @return {Object}
+   * This can be used to generate a link that clears filters.
    */
-  stickyParams() {
+  stickyParams(): SearchParams {
     return {
       sort: m.route.param('sort'),
       q: m.route.param('q'),
@@ -40,11 +74,9 @@ export default class GlobalSearchState extends SearchState {
   }
 
   /**
-   * Get parameters to pass to the DiscussionList component.
-   *
-   * @return {Object}
+   * Get parameters to be used in the current page.
    */
-  params() {
+  params(): SearchParams {
     const params = this.stickyParams();
 
     params.filter = m.route.param('filter');
@@ -54,10 +86,8 @@ export default class GlobalSearchState extends SearchState {
 
   /**
    * Redirect to the index page using the given sort parameter.
-   *
-   * @param {String} sort
    */
-  changeSort(sort) {
+  changeSort(sort: string) {
     const params = this.params();
 
     if (sort === Object.keys(app.discussions.sortMap())[0]) {
@@ -68,28 +98,4 @@ export default class GlobalSearchState extends SearchState {
 
     setRouteWithForcedRefresh(app.route(app.current.get('routeName'), params));
   }
-
-  /**
-   * Return the current search query, if any. This is implemented to activate
-   * the search box in the header.
-   *
-   * @see Search
-   * @return {String}
-   */
-  getInitialSearch() {
-    return app.current.type && app.current.type.providesInitialSearch && this.params().q;
-  }
-
-  /**
-   * Redirect to the index page without a search filter. This is called when the
-   * 'x' is clicked in the search box in the header.
-   *
-   * @see Search
-   */
-  clearInitialSearch() {
-    const params = this.params();
-    delete params.q;
-
-    setRouteWithForcedRefresh(app.route(app.current.get('routeName'), params));
-  }
 }

framework/core/js/src/forum/states/SearchState.js

@@ -1,35 +0,0 @@
-export default class SearchState {
-  constructor(cachedSearches = []) {
-    this.cachedSearches = cachedSearches;
-  }
-
-  getValue() {
-    return this.value;
-  }
-
-  setValue(value) {
-    this.value = value;
-  }
-
-  /**
-   * Clear the search value.
-   */
-  clear() {
-    this.setValue('');
-  }
-
-  /**
-   * Mark that we have already searched for this query so that we don't
-   * have to ping the endpoint again.
-   */
-  cache(query) {
-    this.cachedSearches.push(query);
-  }
-
-  /**
-   * Check if this query has been searched before.
-   */
-  isCached(query) {
-    return this.cachedSearches.indexOf(query) !== -1;
-  }
-}

framework/core/js/src/forum/states/SearchState.ts

@@ -0,0 +1,51 @@
+export default class SearchState {
+  protected cachedSearches: Set<string>;
+  protected value: string = '';
+
+  constructor(cachedSearches: string[] = []) {
+    this.cachedSearches = new Set(cachedSearches);
+  }
+
+  /**
+   * If we are displaying the full results of a search (not just a preview),
+   * this value should return the query that prompted that search.
+   *
+   * In this generic class, full page searching is not supported.
+   * This method should be implemented by subclasses that do support it.
+   *
+   * @see Search
+   */
+  getInitialSearch(): string {
+    return '';
+  }
+
+  getValue(): string {
+    return this.value;
+  }
+
+  setValue(value: string) {
+    this.value = value;
+  }
+
+  /**
+   * Clear the search value.
+   */
+  clear() {
+    this.setValue('');
+  }
+
+  /**
+   * Mark that we have already searched for this query so that we don't
+   * have to ping the endpoint again.
+   */
+  cache(query: string) {
+    this.cachedSearches.add(query);
+  }
+
+  /**
+   * Check if this query has been searched before.
+   */
+  isCached(query: string): boolean {
+    return this.cachedSearches.has(query);
+  }
+}

framework/core/js/src/forum/utils/KeyboardNavigatable.ts

@@ -1,3 +1,6 @@
+type KeyboardEventHandler = (event: KeyboardEvent) => void;
+type ShouldHandle = (event: KeyboardEvent) => boolean;
+
 /**
  * The `KeyboardNavigatable` class manages lists that can be navigated with the
  * keyboard, calling callbacks for each actions.
@@ -6,41 +9,27 @@
  * API for use.
  */
 export default class KeyboardNavigatable {
-  constructor() {
   /**
    * Callback to be executed for a specified input.
-     *
-     * @callback KeyboardNavigatable~keyCallback
-     * @param {KeyboardEvent} event
-     * @returns {boolean}
    */
-    this.callbacks = {};
+  protected callbacks = new Map<number, KeyboardEventHandler>();
 
   /**
    * Callback that determines whether keyboard input should be handled.
    * By default, always handle keyboard navigation.
-     *
-     * @callback whenCallback
-     * @param {KeyboardEvent} event
-     * @returns {boolean}
    */
-    this.whenCallback = (event) => true;
-  }
+  protected whenCallback: ShouldHandle = (event: KeyboardEvent) => true;
 
   /**
    * Provide a callback to be executed when navigating upwards.
    *
    * This will be triggered by the Up key.
-   *
-   * @public
-   * @param {KeyboardNavigatable~keyCallback} callback
-   * @return {KeyboardNavigatable}
    */
-  onUp(callback) {
-    this.callbacks[38] = (e) => {
+  onUp(callback: KeyboardEventHandler): KeyboardNavigatable {
+    this.callbacks.set(38, (e) => {
       e.preventDefault();
       callback(e);
-    };
+    });
 
     return this;
   }
@@ -49,16 +38,12 @@ export default class KeyboardNavigatable {
    * Provide a callback to be executed when navigating downwards.
    *
    * This will be triggered by the Down key.
-   *
-   * @public
-   * @param {KeyboardNavigatable~keyCallback} callback
-   * @return {KeyboardNavigatable}
    */
-  onDown(callback) {
-    this.callbacks[40] = (e) => {
+  onDown(callback: KeyboardEventHandler): KeyboardNavigatable {
+    this.callbacks.set(40, (e) => {
       e.preventDefault();
       callback(e);
-    };
+    });
 
     return this;
   }
@@ -67,17 +52,16 @@ export default class KeyboardNavigatable {
    * Provide a callback to be executed when the current item is selected..
    *
    * This will be triggered by the Return and Tab keys..
-   *
-   * @public
-   * @param {KeyboardNavigatable~keyCallback} callback
-   * @return {KeyboardNavigatable}
    */
-  onSelect(callback) {
-    this.callbacks[9] = this.callbacks[13] = (e) => {
+  onSelect(callback: KeyboardEventHandler): KeyboardNavigatable {
+    const handler: KeyboardEventHandler = (e) => {
       e.preventDefault();
       callback(e);
     };
 
+    this.callbacks.set(9, handler);
+    this.callbacks.set(13, handler);
+
     return this;
   }
 
@@ -85,17 +69,13 @@ export default class KeyboardNavigatable {
    * Provide a callback to be executed when the navigation is canceled.
    *
    * This will be triggered by the Escape key.
-   *
-   * @public
-   * @param {KeyboardNavigatable~keyCallback} callback
-   * @return {KeyboardNavigatable}
    */
-  onCancel(callback) {
-    this.callbacks[27] = (e) => {
+  onCancel(callback: KeyboardEventHandler): KeyboardNavigatable {
+    this.callbacks.set(27, (e) => {
       e.stopPropagation();
       e.preventDefault();
       callback(e);
-    };
+    });
 
     return this;
   }
@@ -109,52 +89,40 @@ export default class KeyboardNavigatable {
    * @param {KeyboardNavigatable~keyCallback} callback
    * @return {KeyboardNavigatable}
    */
-  onRemove(callback) {
-    this.callbacks[8] = (e) => {
+  onRemove(callback: KeyboardEventHandler): KeyboardNavigatable {
+    this.callbacks.set(8, (e) => {
       if (e.target.selectionStart === 0 && e.target.selectionEnd === 0) {
         callback(e);
         e.preventDefault();
       }
-    };
+    });
 
     return this;
   }
 
   /**
    * Provide a callback that determines whether keyboard input should be handled.
-   *
-   * @public
-   * @param {KeyboardNavigatable~whenCallback} callback
-   * @return {KeyboardNavigatable}
    */
-  when(callback) {
-    this.whenCallback = callback;
-
-    return this;
+  when(callback: ShouldHandle): KeyboardNavigatable {
+    return { ...this, whenCallback: callback };
   }
 
   /**
    * Set up the navigation key bindings on the given jQuery element.
-   *
-   * @public
-   * @param {jQuery} $element
    */
-  bindTo($element) {
+  bindTo($element: JQuery) {
     // Handle navigation key events on the navigatable element.
     $element.on('keydown', this.navigate.bind(this));
   }
 
   /**
    * Interpret the given keyboard event as navigation commands.
-   *
-   * @public
-   * @param {KeyboardEvent} event
    */
-  navigate(event) {
+  navigate(event: KeyboardEvent) {
     // This callback determines whether keyboard should be handled or ignored.
     if (!this.whenCallback(event)) return;
 
-    const keyCallback = this.callbacks[event.which];
+    const keyCallback = this.callbacks.get(event.which);
     if (keyCallback) {
       keyCallback(event);
     }