From 073e5ccd839b675e555182686a52fb51661efe1f Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 18 Oct 2021 13:17:27 -0400 Subject: [PATCH] UX: Better topic search experience (#14625) --- .../javascripts/discourse/app/lib/search.js | 10 ++- .../discourse/app/widgets/header.js | 68 +++++++---------- .../app/widgets/search-menu-results.js | 32 ++++---- .../discourse/app/widgets/search-menu.js | 75 +++++++++++++++++-- .../discourse/tests/acceptance/search-test.js | 33 ++++++-- .../stylesheets/common/base/search-menu.scss | 24 +++++- .../common/components/buttons.scss | 4 + config/locales/client.en.yml | 1 + 8 files changed, 163 insertions(+), 84 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/search.js b/app/assets/javascripts/discourse/app/lib/search.js index 5df67d4f302..9f2b3f39bc1 100644 --- a/app/assets/javascripts/discourse/app/lib/search.js +++ b/app/assets/javascripts/discourse/app/lib/search.js @@ -117,7 +117,7 @@ function translateGroupedSearchResults(results, opts) { const name = pair[1]; if (results[name].length > 0) { const componentName = - opts.showPosts && type === "topic" ? "post" : type; + opts.searchContext && type === "topic" ? "post" : type; const result = { results: results[name], @@ -154,8 +154,12 @@ export function searchForTerm(term, opts) { data.restrict_to_archetype = opts.restrictToArchetype; } - if (term.includes("topic:")) { - opts.showPosts = true; + if (opts.searchContext) { + data.search_context = { + type: opts.searchContext.type, + id: opts.searchContext.id, + name: opts.searchContext.name, + }; } let ajaxPromise = ajax("/search/query", { data }); diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index afe4821b3f1..db23ceb8f82 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -4,10 +4,10 @@ import { addExtraUserClasses } from "discourse/helpers/user-avatar"; import { ajax } from "discourse/lib/ajax"; import { avatarImg } from "discourse/widgets/post"; import { createWidget } from "discourse/widgets/widget"; -import { get } from "@ember/object"; import getURL from "discourse-common/lib/get-url"; import { h } from "virtual-dom"; import { iconNode } from "discourse-common/lib/icon-library"; +import putCursorAtEnd from "discourse/lib/put-cursor-at-end"; import { schedule } from "@ember/runloop"; import { scrollTop } from "discourse/mixins/scroll-top"; import { wantsNewWindow } from "discourse/lib/intercept-click"; @@ -321,8 +321,6 @@ createWidget("header-cloak", { scheduleRerender() {}, }); -const forceContextEnabled = ["category", "user", "private_messages", "tag"]; - let additionalPanels = []; export function attachAdditionalPanel(name, toggle, transformAttrs) { additionalPanels.push({ name, toggle, transformAttrs }); @@ -339,6 +337,7 @@ export default createWidget("header", { hamburgerVisible: false, userVisible: false, ringBackdrop: true, + inTopicContext: false, }; if (this.site.mobileView) { @@ -366,21 +365,10 @@ export default createWidget("header", { const panels = [this.attach("header-buttons", attrs), headerIcons]; if (state.searchVisible) { - const contextType = this.searchContextType(); - - if (state.searchContextType !== contextType) { - state.contextEnabled = undefined; - state.searchContextType = contextType; - } - - if (state.contextEnabled === undefined) { - if (forceContextEnabled.includes(contextType)) { - state.contextEnabled = true; - } - } - panels.push( - this.attach("search-menu", { contextEnabled: state.contextEnabled }) + this.attach("search-menu", { + inTopicContext: state.inTopicContext, + }) ); } else if (state.hamburgerVisible) { panels.push(this.attach("hamburger-menu")); @@ -477,11 +465,9 @@ export default createWidget("header", { this.updateHighlight(); if (this.state.searchVisible) { - schedule("afterRender", () => { - const searchInput = document.querySelector("#search-term"); - searchInput.focus(); - searchInput.select(); - }); + this.focusSearchInput(); + } else { + this.state.inTopicContext = false; } }, @@ -537,8 +523,7 @@ export default createWidget("header", { togglePageSearch() { const { state } = this; - - state.contextEnabled = false; + state.inTopicContext = false; const currentPath = this.router.get("_router.currentPath"); const blocklist = [/^discovery\.categories/]; @@ -565,7 +550,7 @@ export default createWidget("header", { } if (showSearch) { - state.contextEnabled = true; + state.inTopicContext = true; this.toggleSearchMenu(); return false; } @@ -573,13 +558,6 @@ export default createWidget("header", { return true; }, - searchMenuContextChanged(value) { - this.state.contextType = this.register - .lookup("search-service:main") - .get("contextType"); - this.state.contextEnabled = value; - }, - domClean() { const { state } = this; @@ -637,10 +615,6 @@ export default createWidget("header", { this.toggleHamburger(); break; case "page-search": - let contextType = this.searchContextType(); - if (contextType === "topic") { - this.state.searchContextType = contextType; - } if (!this.togglePageSearch()) { msg.event.preventDefault(); msg.event.stopPropagation(); @@ -649,13 +623,21 @@ export default createWidget("header", { } }, - searchContextType() { - const service = this.register.lookup("search-service:main"); - if (service) { - const ctx = service.get("searchContext"); - if (ctx) { - return get(ctx, "type"); - } + focusSearchInput() { + if (this.state.searchVisible) { + schedule("afterRender", () => { + putCursorAtEnd(document.querySelector("#search-term")); + }); } }, + + setTopicContext() { + this.state.inTopicContext = true; + this.focusSearchInput(); + }, + + clearContext() { + this.state.inTopicContext = false; + this.focusSearchInput(); + }, }); diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js index 364fee711b7..3aaf47520a6 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js @@ -11,10 +11,7 @@ import { h } from "virtual-dom"; import highlightSearch from "discourse/lib/highlight-search"; import { iconNode } from "discourse-common/lib/icon-library"; import renderTag from "discourse/lib/render-tag"; -import { - MODIFIER_REGEXP, - TOPIC_REPLACE_REGEXP, -} from "discourse/widgets/search-menu"; +import { MODIFIER_REGEXP } from "discourse/widgets/search-menu"; const suggestionShortcuts = [ "in:title", @@ -79,9 +76,7 @@ resetQuickSearchRandomTips(); class Highlighted extends RawHtml { constructor(html, term) { super({ html: `${html}` }); - if (term) { - this.term = term.replace(TOPIC_REPLACE_REGEXP, ""); - } + this.term = term; } decorate($html) { @@ -296,9 +291,7 @@ createWidget("search-menu-results", { } if (!term) { - return this.attach("search-menu-initial-options", { - term, - }); + return this.attach("search-menu-initial-options", { term }); } const resultTypes = results.resultTypes || []; @@ -483,15 +476,15 @@ createWidget("search-menu-initial-options", { if (attrs.term || ctx) { if (ctx) { - const term = attrs.term ? `${attrs.term} ` : ""; - + const term = attrs.term || ""; switch (ctx.type) { case "topic": content.push( this.attach("search-menu-assistant-item", { - slug: `${term}topic:${ctx.id}`, + slug: term, + setTopicContext: true, label: [ - h("span", term), + h("span", `${term} `), h("span.label-suffix", I18n.t("search.in_this_topic")), ], }) @@ -501,7 +494,7 @@ createWidget("search-menu-initial-options", { case "private_messages": content.push( this.attach("search-menu-assistant-item", { - slug: `${term}in:personal`, + slug: `${term} in:personal`, }) ); break; @@ -513,7 +506,7 @@ createWidget("search-menu-initial-options", { content.push( this.attach("search-menu-assistant", { - term: `${term}${fullSlug}`, + term: `${term} ${fullSlug}`, suggestionKeyword: "#", results: [{ model: ctx.category }], withInLabel: true, @@ -524,7 +517,7 @@ createWidget("search-menu-initial-options", { case "tag": content.push( this.attach("search-menu-assistant", { - term: `${term}#${ctx.name}`, + term: `${term} #${ctx.name}`, suggestionKeyword: "#", results: [{ name: ctx.name }], withInLabel: true, @@ -534,9 +527,9 @@ createWidget("search-menu-initial-options", { case "user": content.push( this.attach("search-menu-assistant-item", { - slug: `${term}@${ctx.user.username}`, + slug: `${term} @${ctx.user.username}`, label: [ - h("span", term), + h("span", `${term} `), h( "span.label-suffix", I18n.t("search.in_posts_by", { @@ -636,6 +629,7 @@ createWidget("search-menu-assistant-item", { this.sendWidgetAction("triggerAutocomplete", { value: this.attrs.slug, searchTopics: true, + setTopicContext: this.attrs.setTopicContext, }); e.preventDefault(); return false; diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu.js b/app/assets/javascripts/discourse/app/widgets/search-menu.js index dbaf83564d4..ef0426f3a7c 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu.js @@ -15,7 +15,6 @@ import { CANCELLED_STATUS } from "discourse/lib/autocomplete"; const CATEGORY_SLUG_REGEXP = /(\#[a-zA-Z0-9\-:]*)$/gi; const USERNAME_REGEXP = /(\@[a-zA-Z0-9\-\_]*)$/gi; const SUGGESTIONS_REGEXP = /(in:|status:|order:|:)([a-zA-Z]*)$/gi; -export const TOPIC_REPLACE_REGEXP = /\btopic:\d+\s?/i; export const MODIFIER_REGEXP = /.*(\#|\@|:).*$/gi; export const DEFAULT_TYPE_FILTER = "exclude_topics"; @@ -50,6 +49,8 @@ const SearchHelper = { this.cancel(); const { term, typeFilter } = searchData; + const searchContext = widget.searchContext(); + const fullSearchUrl = widget.fullSearchUrl(); const matchSuggestions = this.matchesSuggestions(); @@ -127,13 +128,14 @@ const SearchHelper = { this._activeSearch = searchForTerm(term, { typeFilter, fullSearchUrl, + searchContext, }); this._activeSearch .then((results) => { // we ensure the current search term is the one used // when starting the query if (results && term === searchData.term) { - if (term.includes("topic:")) { + if (searchContext) { widget.appEvents.trigger("post-stream:refresh", { force: true }); } @@ -183,6 +185,14 @@ export default createWidget("search-menu", { tagName: "div.search-menu", searchData, + buildKey: () => "search-menu", + + defaultState(attrs) { + return { + inTopicContext: attrs.inTopicContext, + }; + }, + fullSearchUrl(opts) { let url = "/search"; const params = []; @@ -209,7 +219,23 @@ export default createWidget("search-menu", { }, panelContents() { - let searchInput = [this.attach("search-term", { value: searchData.term })]; + let searchInput = []; + + if (this.state.inTopicContext) { + searchInput.push( + this.attach("button", { + icon: "times", + label: "search.in_this_topic", + title: "search.in_this_topic_tooltip", + className: "btn btn-small search-context", + action: "clearTopicContext", + iconRight: true, + }) + ); + } + + searchInput.push(this.attach("search-term", { value: searchData.term })); + if (searchData.loading) { searchInput.push(h("div.searching", h("div.spinner"))); } else { @@ -238,6 +264,13 @@ export default createWidget("search-menu", { const results = [h("div.search-input", searchInput)]; + if ( + this.state.inTopicContext && + (!SearchHelper.includesTopics() || !searchData.term) + ) { + return results; + } + if (!searchData.loading) { results.push( this.attach("search-menu-results", { @@ -270,7 +303,11 @@ export default createWidget("search-menu", { return this._searchService; }, - html() { + html(attrs, state) { + if (attrs.inTopicContext === false) { + state.inTopicContext = false; + } + return this.attach("menu-panel", { maxWidth: 500, contents: () => this.panelContents(), @@ -281,6 +318,10 @@ export default createWidget("search-menu", { this.sendWidgetAction("toggleSearchMenu"); }, + clearTopicContext() { + this.sendWidgetAction("clearContext"); + }, + keyDown(e) { if (e.which === 27 /* escape */) { this.sendWidgetAction("toggleSearchMenu"); @@ -377,16 +418,22 @@ export default createWidget("search-menu", { this.triggerSearch(); } } + + if (e.target === searchInput && e.which === 8 /* backspace */) { + if (!searchInput.value) { + this.clearTopicContext(); + } + } }, triggerSearch() { searchData.noResults = false; - if (searchData.term.includes("topic:")) { - const highlightTerm = searchData.term.replace(TOPIC_REPLACE_REGEXP, ""); - this.searchService().set("highlightTerm", highlightTerm); - } if (SearchHelper.includesTopics()) { + if (this.state.inTopicContext) { + this.searchService().set("highlightTerm", searchData.term); + } + searchData.loading = true; SearchHelper.perform(this); } else { @@ -407,6 +454,10 @@ export default createWidget("search-menu", { }, triggerAutocomplete(opts = {}) { + if (opts.setTopicContext) { + this.sendWidgetAction("setTopicContext"); + this.state.inTopicContext = true; + } this.searchTermChanged(opts.value, { searchTopics: opts.searchTopics }); }, @@ -420,4 +471,12 @@ export default createWidget("search-menu", { DiscourseURL.routeTo(url); } }, + + searchContext() { + if (this.state.inTopicContext) { + return this.searchService().get("searchContext"); + } + + return false; + }, }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/search-test.js b/app/assets/javascripts/discourse/tests/acceptance/search-test.js index f384289ed2b..93787b1a761 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/search-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/search-test.js @@ -197,19 +197,38 @@ acceptance("Search - Anonymous", function (needs) { "highlights the post correctly" ); - await fillIn("#search-term", "topic:280 interface"); + assert.ok( + exists(".search-menu .search-context"), + "search context indicator is visible" + ); + await click(".clear-search"); + assert.equal(query("#search-term").value, "", "clear button works"); + + await click(".search-context"); + + assert.ok( + !exists(".search-menu .search-context"), + "search context indicator is no longer visible" + ); + + await fillIn("#search-term", "dev"); await focus("input#search-term"); await triggerKeyEvent(".search-menu", "keydown", 40); await click(document.activeElement); - assert.equal( - query("#post_7 span.highlighted").textContent.trim(), - "interface", - "highlights the post when term is after modifier" + assert.ok( + exists(".search-menu .search-context"), + "search context indicator is visible" ); - await click(".clear-search"); - assert.equal(query("#search-term").value, "", "clear button works"); + await fillIn("#search-term", ""); + await focus("input#search-term"); + await triggerKeyEvent("input#search-term", "keydown", 8); // backspace + + assert.ok( + !exists(".search-menu .search-context"), + "backspace resets search context" + ); }); test("Right filters are shown in full page search", async function (assert) { diff --git a/app/assets/stylesheets/common/base/search-menu.scss b/app/assets/stylesheets/common/base/search-menu.scss index ad03465dd01..71fdb80d7da 100644 --- a/app/assets/stylesheets/common/base/search-menu.scss +++ b/app/assets/stylesheets/common/base/search-menu.scss @@ -21,7 +21,24 @@ $search-pad-horizontal: 0.5em; .search-input { position: relative; - padding: $search-pad-vertical 0.1em; + margin: 1px; + display: flex; + align-items: center; + border: 1px solid var(--primary-medium); + input#search-term { + border-width: 0; + margin-bottom: 0; + &:focus { + outline: none; + } + } + + .btn { + margin-left: 1px; + } + &:focus-within { + @include default-focus; + } } .heading { @@ -32,8 +49,7 @@ $search-pad-horizontal: 0.5em; } input[type="text"] { - width: 100%; - margin-bottom: 0; + margin-right: 0px; } .results { @@ -242,7 +258,7 @@ $search-pad-horizontal: 0.5em; .searching { position: absolute; - top: $search-pad-vertical + 0.4em; + top: $search-pad-vertical + 0.2em; right: $search-pad-horizontal; min-height: 20px; diff --git a/app/assets/stylesheets/common/components/buttons.scss b/app/assets/stylesheets/common/components/buttons.scss index 689cc1aeedb..3e141beeec8 100644 --- a/app/assets/stylesheets/common/components/buttons.scss +++ b/app/assets/stylesheets/common/components/buttons.scss @@ -28,6 +28,10 @@ margin-right: 0.45em; transition: color 0.25s; } + .d-button-label + .d-icon { + margin-left: 0.45em; + margin-right: 0; + } &.no-text { .d-icon { margin-right: 0; diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 3e7c73e52c4..6e919bd4acf 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2365,6 +2365,7 @@ en: tags: "Tags" in: "in" in_this_topic: "in this topic" + in_this_topic_tooltip: "switch to searching all topics" in_topics_posts: "in all topics and posts" enter_hint: "or press Enter" in_posts_by: "in posts by %{username}"