From e13ed241223a05ee4c4c1fc1044c1e00571043cd Mon Sep 17 00:00:00 2001 From: Sam <sam.saffron@gmail.com> Date: Tue, 8 Sep 2015 11:03:51 +1000 Subject: [PATCH] FEATURE: on mobile take users to full page search UX: improve styling on full page search page FEATURE: allow search context in full page search FEATURE: visited color link for full page search FIX: broken search help on fulls page search page FEATURE: allow preload store to return a null FEATURE: "mobileAction" for the header buttons --- .../components/header-dropdown.js.es6 | 6 ++ .../discourse/components/search-menu.js.es6 | 15 +--- .../controllers/full-page-search.js.es6 | 72 ++++++++++++++++--- .../discourse/controllers/header.js.es6 | 13 ++++ .../{search-for-term.js.es6 => search.js.es6} | 17 ++++- .../discourse/routes/full-page-search.js.es6 | 21 ++++-- .../discourse/templates/full-page-search.hbs | 13 +++- .../discourse/templates/header.hbs | 1 + .../discourse/views/choose-topic.js.es6 | 2 +- app/assets/javascripts/main_include.js | 2 +- app/assets/javascripts/preload_store.js | 2 +- .../stylesheets/common/base/search.scss | 14 ++++ app/assets/stylesheets/mobile.scss | 1 + app/assets/stylesheets/mobile/search.scss | 16 +++++ app/controllers/search_controller.rb | 51 ++++++++++--- 15 files changed, 203 insertions(+), 43 deletions(-) rename app/assets/javascripts/discourse/lib/{search-for-term.js.es6 => search.js.es6} (84%) create mode 100644 app/assets/stylesheets/mobile/search.scss diff --git a/app/assets/javascripts/discourse/components/header-dropdown.js.es6 b/app/assets/javascripts/discourse/components/header-dropdown.js.es6 index 07123063382..02843970acf 100644 --- a/app/assets/javascripts/discourse/components/header-dropdown.js.es6 +++ b/app/assets/javascripts/discourse/components/header-dropdown.js.es6 @@ -13,6 +13,12 @@ export default Ember.Component.extend({ actions: { toggle() { + + if (Discourse.Mobile.mobileView && this.get('mobileAction')) { + this.sendAction('mobileAction'); + return; + } + if (this.siteSettings.login_required && !this.currentUser) { this.sendAction('loginAction'); } else { diff --git a/app/assets/javascripts/discourse/components/search-menu.js.es6 b/app/assets/javascripts/discourse/components/search-menu.js.es6 index 4786747d99f..8a4f2ddecef 100644 --- a/app/assets/javascripts/discourse/components/search-menu.js.es6 +++ b/app/assets/javascripts/discourse/components/search-menu.js.es6 @@ -1,4 +1,4 @@ -import searchForTerm from 'discourse/lib/search-for-term'; +import {searchForTerm, searchContextDescription} from 'discourse/lib/search'; import DiscourseURL from 'discourse/lib/url'; import { default as computed, observes } from 'ember-addons/ember-computed-decorators'; import showModal from 'discourse/lib/show-modal'; @@ -48,18 +48,7 @@ export default Ember.Component.extend({ @computed('searchService.searchContext') searchContextDescription(ctx) { - if (ctx) { - switch(Em.get(ctx, 'type')) { - case 'topic': - return I18n.t('search.context.topic'); - case 'user': - return I18n.t('search.context.user', {username: Em.get(ctx, 'user.username')}); - case 'category': - return I18n.t('search.context.category', {category: Em.get(ctx, 'category.name')}); - case 'private_messages': - return I18n.t('search.context.private_messages'); - } - } + return searchContextDescription(Em.get(ctx, 'type'), Em.get(ctx, 'user.username') || Em.get(ctx, 'category.name')); }, @observes('searchService.searchContextEnabled') diff --git a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 index 7a5f031d2bf..8efc5c6adfe 100644 --- a/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 +++ b/app/assets/javascripts/discourse/controllers/full-page-search.js.es6 @@ -1,30 +1,65 @@ -import { translateResults } from "discourse/lib/search-for-term"; +import { translateResults, searchContextDescription } from "discourse/lib/search"; +import showModal from 'discourse/lib/show-modal'; +import { default as computed, observes } from 'ember-addons/ember-computed-decorators'; +import Category from 'discourse/models/category'; export default Ember.Controller.extend({ needs: ["application"], loading: Em.computed.not("model"), - queryParams: ["q"], + queryParams: ["q", "context_id", "context", "skip_context"], q: null, selected: [], + context_id: null, + context: null, - modelChanged: function() { + @computed('skip_context', 'context') + searchContextEnabled: { + get(skip,context){ + return (!skip && context) || skip === "false"; + }, + set(val) { + this.set('skip_context', val ? "false" : "true" ) + } + }, + + @computed('context', 'context_id') + searchContextDescription(context, id){ + var name = id; + if (context === 'category') { + var category = Category.findById(id); + if (!category) {return;} + + name = category.get('name'); + } + return searchContextDescription(context, name); + }, + + @computed('q') + searchActive(q){ + return q && q.length > 0; + }, + + @observes('model') + modelChanged() { if (this.get("searchTerm") !== this.get("q")) { this.set("searchTerm", this.get("q")); } - }.observes("model"), + }, - qChanged: function() { + @observes('q') + qChanged() { const model = this.get("model"); if (model && this.get("model.q") !== this.get("q")) { this.set("searchTerm", this.get("q")); this.send("search"); } - }.observes("q"), + }, - _showFooter: function() { + @observes('loading') + _showFooter() { this.set("controllers.application.showFooter", !this.get("loading")); - }.observes("loading"), + }, canBulkSelect: Em.computed.alias('currentUser.staff'), @@ -32,9 +67,19 @@ export default Ember.Controller.extend({ this.set("q", this.get("searchTerm")); this.set("model", null); - Discourse.ajax("/search", { data: { q: this.get("searchTerm") } }).then(results => { + var args = { q: this.get("searchTerm") }; + + const skip = this.get("skip_context"); + if ((!skip && this.get('context')) || skip==="false"){ + args.search_context = { + type: this.get('context'), + id: this.get('context_id') + }; + } + + Discourse.ajax("/search", { data: args }).then(results => { this.set("model", translateResults(results) || {}); - this.set("model.q", this.get("q")); + // this.set("model.q", this.get("q")); }); }, @@ -51,6 +96,13 @@ export default Ember.Controller.extend({ this.search(); }, + showSearchHelp() { + // TODO: dupe code should be centralized + Discourse.ajax("/static/search_help.html", { dataType: 'html' }).then((model) => { + showModal('searchHelp', { model }); + }); + }, + search() { this.search(); } diff --git a/app/assets/javascripts/discourse/controllers/header.js.es6 b/app/assets/javascripts/discourse/controllers/header.js.es6 index 813e9e717ec..4d40efbbea0 100644 --- a/app/assets/javascripts/discourse/controllers/header.js.es6 +++ b/app/assets/javascripts/discourse/controllers/header.js.es6 @@ -1,3 +1,5 @@ +import DiscourseURL from 'discourse/lib/url'; + const HeaderController = Ember.Controller.extend({ topic: null, showExtraInfo: null, @@ -18,6 +20,17 @@ const HeaderController = Ember.Controller.extend({ actions: { + fullPageSearch() { + const searchService = this.container.lookup('search-service:main'); + const context = searchService.get('searchContext'); + var params = ""; + + if (context) { + params = `?context=${context.type}&context_id=${context.id}`; + } + + DiscourseURL.routeTo('/search' + params); + }, toggleMenuPanel(visibleProp) { this.toggleProperty(visibleProp); this.appEvents.trigger('dropdowns:closeAll'); diff --git a/app/assets/javascripts/discourse/lib/search-for-term.js.es6 b/app/assets/javascripts/discourse/lib/search.js.es6 similarity index 84% rename from app/assets/javascripts/discourse/lib/search-for-term.js.es6 rename to app/assets/javascripts/discourse/lib/search.js.es6 index 2d029fdf9ea..d39ed0d7abc 100644 --- a/app/assets/javascripts/discourse/lib/search-for-term.js.es6 +++ b/app/assets/javascripts/discourse/lib/search.js.es6 @@ -86,4 +86,19 @@ function searchForTerm(term, opts) { return promise; } -export default searchForTerm; +const searchContextDescription = function(type, name){ + if (type) { + switch(type) { + case 'topic': + return I18n.t('search.context.topic'); + case 'user': + return I18n.t('search.context.user', {username: name}); + case 'category': + return I18n.t('search.context.category', {category: name}); + case 'private_messages': + return I18n.t('search.context.private_messages'); + } + } +}; + +export { searchForTerm, searchContextDescription }; diff --git a/app/assets/javascripts/discourse/routes/full-page-search.js.es6 b/app/assets/javascripts/discourse/routes/full-page-search.js.es6 index 482ebf1fc50..8aeedefdde6 100644 --- a/app/assets/javascripts/discourse/routes/full-page-search.js.es6 +++ b/app/assets/javascripts/discourse/routes/full-page-search.js.es6 @@ -1,15 +1,24 @@ -import { translateResults } from "discourse/lib/search-for-term"; +import { translateResults } from "discourse/lib/search"; export default Discourse.Route.extend({ - queryParams: { q: {} }, + queryParams: { q: {}, "context-id": {}, context: {} }, model(params) { return PreloadStore.getAndRemove("search", function() { - return Discourse.ajax("/search", { data: { q: params.q } }); + if (params.q && params.q.length > 2) { + var args = { q: params.q }; + if (params.context_id && !args.skip_context) { + args.search_context = { + type: params.context, + id: params.context_id + } + } + return Discourse.ajax("/search", { data: args }); + } else { + return null; + } }).then(results => { - const model = translateResults(results) || {}; - model.q = params.q; - return model; + return (results && translateResults(results)) || {}; }); }, diff --git a/app/assets/javascripts/discourse/templates/full-page-search.hbs b/app/assets/javascripts/discourse/templates/full-page-search.hbs index 832e68148e8..f0278b64072 100644 --- a/app/assets/javascripts/discourse/templates/full-page-search.hbs +++ b/app/assets/javascripts/discourse/templates/full-page-search.hbs @@ -9,11 +9,22 @@ {{/if}} </div> +{{#if context}} +<div class='fps-search-context'> +<label> + {{input type="checkbox" name="searchContext" checked=searchContextEnabled}} {{searchContextDescription}} +</label> +</div> +{{/if}} + {{#conditional-loading-spinner condition=loading}} {{#unless model.posts}} <h3> - {{i18n "search.no_results"}} <a href class="show-help" {{action "showSearchHelp" bubbles=false}}>{{i18n "search.search_help"}}</a> + {{#if searchActive}} + {{i18n "search.no_results"}} + {{/if}} + <a href class="show-help" {{action "showSearchHelp" bubbles=false}}>{{i18n "search.search_help"}}</a> </h3> {{/unless}} diff --git a/app/assets/javascripts/discourse/templates/header.hbs b/app/assets/javascripts/discourse/templates/header.hbs index 2c11cae568e..04c87250f98 100644 --- a/app/assets/javascripts/discourse/templates/header.hbs +++ b/app/assets/javascripts/discourse/templates/header.hbs @@ -23,6 +23,7 @@ {{#header-dropdown iconId="search-button" icon="search" toggleVisible=searchVisible + mobileAction="fullPageSearch" loginAction="showLogin" title="search.title"}} {{/header-dropdown}} diff --git a/app/assets/javascripts/discourse/views/choose-topic.js.es6 b/app/assets/javascripts/discourse/views/choose-topic.js.es6 index da4cbac2f48..cb2a346614b 100644 --- a/app/assets/javascripts/discourse/views/choose-topic.js.es6 +++ b/app/assets/javascripts/discourse/views/choose-topic.js.es6 @@ -1,5 +1,5 @@ import debounce from 'discourse/lib/debounce'; -import searchForTerm from 'discourse/lib/search-for-term'; +import { searchForTerm } from 'discourse/lib/search'; export default Ember.View.extend({ templateName: 'choose_topic', diff --git a/app/assets/javascripts/main_include.js b/app/assets/javascripts/main_include.js index 5ea3455611d..797cab86581 100644 --- a/app/assets/javascripts/main_include.js +++ b/app/assets/javascripts/main_include.js @@ -35,7 +35,7 @@ //= require_tree ./discourse/mixins //= require ./discourse/lib/ajax-error //= require ./discourse/lib/markdown -//= require ./discourse/lib/search-for-term +//= require ./discourse/lib/search //= require ./discourse/lib/user-search //= require ./discourse/lib/export-csv //= require ./discourse/lib/autocomplete diff --git a/app/assets/javascripts/preload_store.js b/app/assets/javascripts/preload_store.js index b4b0e8b0de8..4911d4e54ae 100644 --- a/app/assets/javascripts/preload_store.js +++ b/app/assets/javascripts/preload_store.js @@ -42,7 +42,7 @@ window.PreloadStore = { var result = finder(); // If the finder returns a promise, we support that too - if (result.then) { + if (result && result.then) { result.then(function(result) { return resolve(result); }, function(result) { diff --git a/app/assets/stylesheets/common/base/search.scss b/app/assets/stylesheets/common/base/search.scss index ec75223b5dd..83e82ecf23a 100644 --- a/app/assets/stylesheets/common/base/search.scss +++ b/app/assets/stylesheets/common/base/search.scss @@ -18,10 +18,20 @@ top: -3px; margin-right: 4px; } + a.search-link:visited .topic-title { + color: scale-color($tertiary, $lightness: 15%); + } .search-link { .topic-statuses, .topic-title { font-size: 1.25em; } + + .topic-statuses { + float: none; + display: inline-block; + color: $primary; + font-size: 1.0em; + } } .blurb { font-size: 1.0em; @@ -50,3 +60,7 @@ .search-footer { margin-bottom: 30px; } + +.panel-body-contents .search-context label { + float: left; +} diff --git a/app/assets/stylesheets/mobile.scss b/app/assets/stylesheets/mobile.scss index 984da0c6683..8434f8c731c 100644 --- a/app/assets/stylesheets/mobile.scss +++ b/app/assets/stylesheets/mobile.scss @@ -19,6 +19,7 @@ @import "mobile/history"; @import "mobile/directory"; @import "mobile/menu-panel"; +@import "mobile/search"; /* These files doesn't actually exist, they are injected by DiscourseSassImporter. */ diff --git a/app/assets/stylesheets/mobile/search.scss b/app/assets/stylesheets/mobile/search.scss new file mode 100644 index 00000000000..0438c41c059 --- /dev/null +++ b/app/assets/stylesheets/mobile/search.scss @@ -0,0 +1,16 @@ +.search button.btn-primary, .search button.btn { + float: none; +} + +.search.row { + margin-top: 5px; +} + +.search.row input.search { + height: 25px; + width: 69%; +} + +.fps-search-context { + margin-bottom: 15px; +} diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 4446a0af99e..0b857278f03 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -9,7 +9,20 @@ class SearchController < ApplicationController end def show - search = Search.new(params[:q], type_filter: 'topic', guardian: guardian, include_blurbs: true, blurb_length: 300) + search_args = { + type_filter: 'topic', + guardian: guardian, + include_blurbs: true, + blurb_length: 300 + } + + context, type = lookup_search_context + if context + search_args[:search_context] = context + search_args[:type_filter] = type if type + end + + search = Search.new(params[:q], search_args) result = search.execute serializer = serialize_data(result, GroupedSearchResultSerializer, result: result) @@ -34,7 +47,29 @@ class SearchController < ApplicationController search_args[:include_blurbs] = params[:include_blurbs] == "true" if params[:include_blurbs].present? search_args[:search_for_id] = true if params[:search_for_id].present? + context,type = lookup_search_context + if context + search_args[:search_context] = context + search_args[:type_filter] = type if type + end + + search = Search.new(params[:term], search_args.symbolize_keys) + result = search.execute + render_serialized(result, GroupedSearchResultSerializer, result: result) + end + + protected + + def lookup_search_context + + return if params[:skip_context] == "true" + search_context = params[:search_context] + unless search_context + if (context = params[:context]) && (id = params[:context_id]) + search_context = {type: context, id: id} + end + end if search_context.present? raise Discourse::InvalidParameters.new(:search_context) unless SearchController.valid_context_types.include?(search_context[:type]) @@ -43,23 +78,21 @@ class SearchController < ApplicationController # A user is found by username context_obj = nil if ['user','private_messages'].include? search_context[:type] - context_obj = User.find_by(username_lower: params[:search_context][:id].downcase) + context_obj = User.find_by(username_lower: search_context[:id].downcase) else klass = search_context[:type].classify.constantize - context_obj = klass.find_by(id: params[:search_context][:id]) + context_obj = klass.find_by(id: search_context[:id]) end + type_filter = nil if search_context[:type] == 'private_messages' - search_args[:type_filter] = 'private_messages' + type_filter = 'private_messages' end guardian.ensure_can_see!(context_obj) - search_args[:search_context] = context_obj - end - search = Search.new(params[:term], search_args.symbolize_keys) - result = search.execute - render_serialized(result, GroupedSearchResultSerializer, result: result) + [context_obj, type_filter] + end end end