Remove app.search instance, cache app.cache.searched (#2151)

* Moved search state logic into search state

framework/core/js/src/forum/ForumApplication.js

@@ -1,6 +1,5 @@
 import History from './utils/History';
 import Pane from './utils/Pane';
-import Search from './components/Search';
 import ReplyComposer from './components/ReplyComposer';
 import DiscussionPage from './components/DiscussionPage';
 import SignUpModal from './components/SignUpModal';
@@ -15,6 +14,7 @@ import alertEmailConfirmation from './utils/alertEmailConfirmation';
 import Application from '../common/Application';
 import Navigation from '../common/components/Navigation';
 import NotificationListState from './states/NotificationListState';
+import GlobalSearchState from './states/GlobalSearchState';
 
 export default class ForumApplication extends Application {
   /**
@@ -35,13 +35,6 @@ export default class ForumApplication extends Application {
     discussionRenamed: DiscussionRenamedPost,
   };
 
-  /**
-   * The page's search component instance.
-   *
-   * @type {Search}
-   */
-  search = new Search();
-
   /**
    * An object which controls the state of the page's side pane.
    *
@@ -71,6 +64,14 @@ export default class ForumApplication extends Application {
    */
   notifications = new NotificationListState(this);
 
+  /*
+   * An object which stores previously searched queries and provides convenient
+   * tools for retrieving and managing search values.
+   *
+   * @type {GlobalSearchState}
+   */
+  search = new GlobalSearchState();
+
   constructor() {
     super();
 

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

@@ -7,6 +7,7 @@ import SelectDropdown from '../../common/components/SelectDropdown';
 import NotificationsDropdown from './NotificationsDropdown';
 import ItemList from '../../common/utils/ItemList';
 import listItems from '../../common/helpers/listItems';
+import Search from '../components/Search';
 
 /**
  * The `HeaderSecondary` component displays secondary header controls, such as
@@ -33,7 +34,7 @@ export default class HeaderSecondary extends Component {
   items() {
     const items = new ItemList();
 
-    items.add('search', app.search.render(), 30);
+    items.add('search', Search.component({ state: app.search }), 30);
 
     if (app.forum.attribute('showLanguageSelector') && Object.keys(app.data.locales).length > 1) {
       const locales = [];

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

@@ -2,7 +2,6 @@ import { extend } from '../../common/extend';
 import Page from './Page';
 import ItemList from '../../common/utils/ItemList';
 import listItems from '../../common/helpers/listItems';
-import icon from '../../common/helpers/icon';
 import DiscussionList from './DiscussionList';
 import WelcomeHero from './WelcomeHero';
 import DiscussionComposer from './DiscussionComposer';
@@ -18,6 +17,8 @@ import SelectDropdown from '../../common/components/SelectDropdown';
  * hero, the sidebar, and the discussion list.
  */
 export default class IndexPage extends Page {
+  static providesInitialSearch = true;
+
   init() {
     super.init();
 
@@ -36,7 +37,7 @@ export default class IndexPage extends Page {
       app.cache.discussionList = null;
     }
 
-    const params = this.params();
+    const params = app.search.params();
 
     if (app.cache.discussionList) {
       // Compare the requested parameters (sort, search query) to the ones that
@@ -187,7 +188,7 @@ export default class IndexPage extends Page {
    */
   navItems() {
     const items = new ItemList();
-    const params = this.stickyParams();
+    const params = app.search.stickyParams();
 
     items.add(
       'allDiscussions',
@@ -222,15 +223,15 @@ export default class IndexPage extends Page {
       'sort',
       Dropdown.component({
         buttonClassName: 'Button',
-        label: sortOptions[this.params().sort] || Object.keys(sortMap).map((key) => sortOptions[key])[0],
+        label: sortOptions[app.search.params().sort] || Object.keys(sortMap).map((key) => sortOptions[key])[0],
         children: Object.keys(sortOptions).map((value) => {
           const label = sortOptions[value];
-          const active = (this.params().sort || Object.keys(sortMap)[0]) === value;
+          const active = (app.search.params().sort || Object.keys(sortMap)[0]) === value;
 
           return Button.component({
             children: label,
             icon: active ? 'fas fa-check' : true,
-            onclick: this.changeSort.bind(this, value),
+            onclick: app.search.changeSort.bind(app.search, value),
             active: active,
           });
         }),
@@ -280,72 +281,6 @@ export default class IndexPage extends Page {
     return items;
   }
 
-  /**
-   * Return the current search query, if any. This is implemented to activate
-   * the search box in the header.
-   *
-   * @see Search
-   * @return {String}
-   */
-  searching() {
-    return 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
-   */
-  clearSearch() {
-    const params = this.params();
-    delete params.q;
-
-    m.route(app.route(this.props.routeName, params));
-  }
-
-  /**
-   * Redirect to the index page using the given sort parameter.
-   *
-   * @param {String} sort
-   */
-  changeSort(sort) {
-    const params = this.params();
-
-    if (sort === Object.keys(app.cache.discussionList.sortMap())[0]) {
-      delete params.sort;
-    } else {
-      params.sort = sort;
-    }
-
-    m.route(app.route(this.props.routeName, params));
-  }
-
-  /**
-   * Get URL parameters that stick between filter changes.
-   *
-   * @return {Object}
-   */
-  stickyParams() {
-    return {
-      sort: m.route.param('sort'),
-      q: m.route.param('q'),
-    };
-  }
-
-  /**
-   * Get parameters to pass to the DiscussionList component.
-   *
-   * @return {Object}
-   */
-  params() {
-    const params = this.stickyParams();
-
-    params.filter = m.route.param('filter');
-
-    return params;
-  }
-
   /**
    * Open the composer for a new discussion or prompt the user to login.
    *

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

@@ -12,19 +12,17 @@ import UsersSearchSource from './UsersSearchSource';
  * The `Search` component displays a menu of as-you-type results from a variety
  * of sources.
  *
- * The search box will be 'activated' if the app's current controller implements
+ * The search box will be 'activated' if the app's seach state's
- * a `searching` method that returns a truthy value. If this is the case, an 'x'
+ * getInitialSearch() value is a truthy value. If this is the case, an 'x'
- * button will be shown next to the search field, and clicking it will call the
+ * button will be shown next to the search field, and clicking it will clear the search.
- * `clearSearch` method on the controller.
+ *
+ * PROPS:
+ *
+ * - state: AlertState instance.
  */
 export default class Search extends Component {
   init() {
-    /**
+    this.state = this.props.state;
-     * The value of the search input.
-     *
-     * @type {Function}
-     */
-    this.value = m.prop('');
 
     /**
      * Whether or not the search input has focus.
@@ -47,13 +45,6 @@ export default class Search extends Component {
      */
     this.loadingSources = 0;
 
-    /**
-     * A list of queries that have been searched for.
-     *
-     * @type {Array}
-     */
-    this.searched = [];
-
     /**
      * 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
@@ -66,13 +57,7 @@ export default class Search extends Component {
   }
 
   view() {
-    const currentSearch = this.getCurrentSearch();
+    const currentSearch = this.state.getInitialSearch();
-
-    // Initialize search input value in the view rather than the constructor so
-    // that we have access to app.current.
-    if (typeof this.value() === 'undefined') {
-      this.value(currentSearch || '');
-    }
 
     // Initialize search sources in the view rather than the constructor so
     // that we have access to app.forum.
@@ -88,7 +73,7 @@ export default class Search extends Component {
         className={
           'Search ' +
           classList({
-            open: this.value() && this.hasFocus,
+            open: this.state.getValue() && this.hasFocus,
             focused: this.hasFocus,
             active: !!currentSearch,
             loading: !!this.loadingSources,
@@ -100,8 +85,8 @@ export default class Search extends Component {
             className="FormControl"
             type="search"
             placeholder={extractText(app.translator.trans('core.forum.header.search_placeholder'))}
-            value={this.value()}
+            value={this.state.getValue()}
-            oninput={m.withAttr('value', this.value)}
+            oninput={m.withAttr('value', this.state.setValue.bind(this.state))}
             onfocus={() => (this.hasFocus = true)}
             onblur={() => (this.hasFocus = false)}
           />
@@ -116,7 +101,7 @@ export default class Search extends Component {
           )}
         </div>
         <ul className="Dropdown-menu Search-results">
-          {this.value() && this.hasFocus ? this.sources.map((source) => source.view(this.value())) : ''}
+          {this.state.getValue() && this.hasFocus ? this.sources.map((source) => source.view(this.state.getValue())) : ''}
         </ul>
       </div>
     );
@@ -129,6 +114,7 @@ export default class Search extends Component {
     if (isInitialized) return;
 
     const search = this;
+    const state = this.state;
 
     this.$('.Search-results')
       .on('mousedown', (e) => e.preventDefault())
@@ -158,7 +144,7 @@ export default class Search extends Component {
 
         clearTimeout(search.searchTimeout);
         search.searchTimeout = setTimeout(() => {
-          if (search.searched.indexOf(query) !== -1) return;
+          if (state.isCached(query)) return;
 
           if (query.length >= 3) {
             search.sources.map((source) => {
@@ -173,7 +159,7 @@ export default class Search extends Component {
             });
           }
 
-          search.searched.push(query);
+          state.cache(query);
           m.redraw();
         }, 250);
       })
@@ -185,15 +171,6 @@ export default class Search extends Component {
       });
   }
 
-  /**
-   * Get the active search in the app's current controller.
-   *
-   * @return {String}
-   */
-  getCurrentSearch() {
-    return app.current && typeof app.current.searching === 'function' && app.current.searching();
-  }
-
   /**
    * Navigate to the currently selected search result and close the list.
    */
@@ -201,7 +178,7 @@ export default class Search extends Component {
     clearTimeout(this.searchTimeout);
     this.loadingSources = 0;
 
-    if (this.value()) {
+    if (this.state.getValue()) {
       m.route(this.getItem(this.index).find('a').attr('href'));
     } else {
       this.clear();
@@ -211,16 +188,10 @@ export default class Search extends Component {
   }
 
   /**
-   * Clear the search input and the current controller's active search.
+   * Clear the search
    */
   clear() {
-    this.value('');
+    this.state.clear();
-
-    if (this.getCurrentSearch()) {
-      app.current.clearSearch();
-    } else {
-      m.redraw();
-    }
   }
 
   /**

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

@@ -2,8 +2,8 @@
  * The `SearchSource` interface defines a section of search results in the
  * search dropdown.
  *
- * Search sources should be registered with the `Search` component instance
+ * Search sources should be registered with the `Search` component class
- * (app.search) by extending the `sourceItems` method. When the user types a
+ * 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.

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

@@ -0,0 +1,95 @@
+import SearchState from './SearchState';
+
+export default class GlobalSearchState extends SearchState {
+  constructor(cachedSearches = [], searchRoute = 'index') {
+    super(cachedSearches);
+    this.searchRoute = searchRoute;
+  }
+
+  getValue() {
+    if (this.value === undefined) {
+      this.value = this.getInitialSearch() || '';
+    }
+
+    return super.getValue();
+  }
+
+  /**
+   * Clear the search input and the current controller's active search.
+   */
+  clear() {
+    super.clear();
+
+    if (this.getInitialSearch()) {
+      this.clearInitialSearch();
+    } else {
+      m.redraw();
+    }
+  }
+
+  /**
+   * Get URL parameters that stick between filter changes.
+   *
+   * @return {Object}
+   */
+  stickyParams() {
+    return {
+      sort: m.route.param('sort'),
+      q: m.route.param('q'),
+    };
+  }
+
+  /**
+   * Get parameters to pass to the DiscussionList component.
+   *
+   * @return {Object}
+   */
+  params() {
+    const params = this.stickyParams();
+
+    params.filter = m.route.param('filter');
+
+    return params;
+  }
+
+  /**
+   * Redirect to the index page using the given sort parameter.
+   *
+   * @param {String} sort
+   */
+  changeSort(sort) {
+    const params = this.params();
+
+    if (sort === Object.keys(app.cache.discussionList.sortMap())[0]) {
+      delete params.sort;
+    } else {
+      params.sort = sort;
+    }
+
+    m.route(app.route(this.searchRoute, 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.constructor.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;
+
+    m.route(app.route(this.searchRoute, params));
+  }
+}

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

@@ -0,0 +1,35 @@
+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;
+  }
+}