From 4013aed07882fc17e3429dfbd1cf86ce293ff115 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Thu, 18 Jun 2020 18:47:01 -0400 Subject: [PATCH] Remove app.search instance, cache app.cache.searched (#2151) * Moved search state logic into search state --- .../core/js/src/forum/ForumApplication.js | 17 ++-- .../src/forum/components/HeaderSecondary.js | 3 +- .../core/js/src/forum/components/IndexPage.js | 79 ++------------- .../core/js/src/forum/components/Search.js | 67 ++++--------- .../js/src/forum/components/SearchSource.js | 4 +- .../js/src/forum/states/GlobalSearchState.js | 95 +++++++++++++++++++ .../core/js/src/forum/states/SearchState.js | 35 +++++++ 7 files changed, 169 insertions(+), 131 deletions(-) create mode 100644 framework/core/js/src/forum/states/GlobalSearchState.js create mode 100644 framework/core/js/src/forum/states/SearchState.js diff --git a/framework/core/js/src/forum/ForumApplication.js b/framework/core/js/src/forum/ForumApplication.js index 8173dd73d..c1ab8a5ee 100644 --- a/framework/core/js/src/forum/ForumApplication.js +++ b/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(); diff --git a/framework/core/js/src/forum/components/HeaderSecondary.js b/framework/core/js/src/forum/components/HeaderSecondary.js index cc6db8bb2..932408fde 100644 --- a/framework/core/js/src/forum/components/HeaderSecondary.js +++ b/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 = []; diff --git a/framework/core/js/src/forum/components/IndexPage.js b/framework/core/js/src/forum/components/IndexPage.js index 9e399a8c8..76a2e83c8 100644 --- a/framework/core/js/src/forum/components/IndexPage.js +++ b/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. * diff --git a/framework/core/js/src/forum/components/Search.js b/framework/core/js/src/forum/components/Search.js index b744abdcc..5302d6c6f 100644 --- a/framework/core/js/src/forum/components/Search.js +++ b/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 - * a `searching` method that returns 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 - * `clearSearch` method on the controller. + * The search box will be 'activated' if the app's seach state's + * 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 clear the search. + * + * PROPS: + * + * - state: AlertState instance. */ export default class Search extends Component { init() { - /** - * The value of the search input. - * - * @type {Function} - */ - this.value = m.prop(''); + this.state = this.props.state; /** * 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
  • 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(); - - // 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 || ''); - } + const currentSearch = this.state.getInitialSearch(); // 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()} - oninput={m.withAttr('value', this.value)} + value={this.state.getValue()} + 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 { )} ); @@ -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(''); - - if (this.getCurrentSearch()) { - app.current.clearSearch(); - } else { - m.redraw(); - } + this.state.clear(); } /** diff --git a/framework/core/js/src/forum/components/SearchSource.js b/framework/core/js/src/forum/components/SearchSource.js index 38947a180..8ae6bf1c3 100644 --- a/framework/core/js/src/forum/components/SearchSource.js +++ b/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 - * (app.search) by extending the `sourceItems` method. When the user types a + * 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. diff --git a/framework/core/js/src/forum/states/GlobalSearchState.js b/framework/core/js/src/forum/states/GlobalSearchState.js new file mode 100644 index 000000000..a068008e3 --- /dev/null +++ b/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)); + } +} diff --git a/framework/core/js/src/forum/states/SearchState.js b/framework/core/js/src/forum/states/SearchState.js new file mode 100644 index 000000000..5644085df --- /dev/null +++ b/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; + } +}