From 748940e11fab27915a5280d74da145bd5bbb591f Mon Sep 17 00:00:00 2001 From: Isaac Janzen Date: Mon, 27 Nov 2023 11:25:17 -0700 Subject: [PATCH] DEV: Update to glimmer search menu --- common/common.scss | 47 +++--- .../api-initializers/init-search-banner.js | 154 ------------------ .../discourse/components/search-banner.hbs | 4 +- .../search-icon.gjs | 25 +++ mobile/mobile.scss | 2 +- spec/system/viewing_search_banner_spec.rb | 30 ++++ .../acceptance/search-banner-test.js | 73 +++++++++ 7 files changed, 158 insertions(+), 177 deletions(-) create mode 100644 javascripts/discourse/connectors/search-menu-before-term-input/search-icon.gjs create mode 100644 test/javascripts/acceptance/search-banner-test.js diff --git a/common/common.scss b/common/common.scss index 57f1e9b..2d1a997 100644 --- a/common/common.scss +++ b/common/common.scss @@ -34,21 +34,21 @@ $max-width: 600px; max-width: $max-width; } - .search-menu { - position: relative; - display: flex; - flex-wrap: wrap; - .d-icon-search { - margin: 0; - } - .browser-search-tip { - display: none; - } - .search-input input#search-term[type="text"] { - margin: 0; - width: 100%; - padding-left: 2.25em; - } + .menu-panel-results .menu-panel { + position: unset; + padding: 0; + } + + .d-icon-search { + margin: 0; + } + .browser-search-tip { + display: none; + } + .search-input input#search-term[type="text"] { + margin: 0; + width: 100%; + // padding: 0; } .search-input { @@ -75,12 +75,12 @@ $max-width: 600px; } .btn.search-icon:not(.has-search-button-text) { - position: absolute; z-index: 2; background: transparent; line-height: 1; color: var(--primary-medium); height: 100%; + padding-right: 0.25em; .discourse-no-touch & { &:hover { background: transparent; @@ -97,19 +97,24 @@ $max-width: 600px; column-gap: 0.5em; background-color: var(--tertiary); color: var(--secondary); + flex: none; &:hover { background-color: var(--tertiary-hover); } .d-icon { color: var(--secondary); } - + .search-input #search-term[type="text"] { - padding-left: 0.5em; - } } - .search-input .btn.search-context { - margin: 0; + .search-input .search-context { + // margin-right: 1em !important; + } + + // hide the search icon when a search context is selected + // eg when searching in a topic + .search-input .search-context + .search-icon:not(.has-search-button-text), + .search-input .search-context ~ .search-icon:not(.has-search-button-text) { + display: none; } .results { diff --git a/javascripts/discourse/api-initializers/init-search-banner.js b/javascripts/discourse/api-initializers/init-search-banner.js index c0be64a..c3b0218 100644 --- a/javascripts/discourse/api-initializers/init-search-banner.js +++ b/javascripts/discourse/api-initializers/init-search-banner.js @@ -12,158 +12,4 @@ export default apiInitializer("1.14.0", (api) => { : "below-site-header", SearchBanner ); - - // Simplified version of header search theme component - const searchMenuWidget = api.container.factoryFor("widget:search-menu"); - const corePanelContents = searchMenuWidget.class.prototype["panelContents"]; - - api.reopenWidget("search-menu", { - buildKey(attrs) { - let type = attrs.formFactor || "menu"; - return `search-${type}`; - }, - - defaultState(attrs) { - return { - formFactor: attrs.formFactor || "menu", - showHeaderResults: false, - inTopicContext: attrs.inTopicContext, - }; - }, - - html(attrs, state) { - if (this.state.formFactor === "widget") { - return this.panelContents(); - } else { - return this._super(attrs, state); - } - }, - - clickOutside() { - const formFactor = this.state.formFactor; - if (formFactor === "menu") { - return this.sendWidgetAction("toggleSearchMenu"); - } else { - this.state.showHeaderResults = false; - this.scheduleRerender(); - } - }, - - click() { - const formFactor = this.state.formFactor; - if (formFactor === "widget") { - this.showResults(); - } - }, - - showResults() { - this.state.showHeaderResults = true; - this.scheduleRerender(); - }, - - linkClickedEvent(attrs) { - if (attrs) { - const { searchLogId, searchResultId, searchResultType } = attrs; - if (searchLogId && searchResultId && searchResultType) { - logSearchLinkClick({ - searchLogId, - searchResultId, - searchResultType, - }); - } - } - - const formFactor = this.state.formFactor; - - if (formFactor === "widget") { - this.state.showHeaderResults = false; - this.scheduleRerender(); - } - }, - - focusSearchInput() { - const searchInput = - this.state.formFactor === "widget" - ? document.querySelector(".search-widget #search-term") - : document.querySelector(".search-menu #search-term"); - - searchInput.focus(); - searchInput.select(); - }, - - clearContext() { - this.sendWidgetAction("clearSearch"); - this.sendWidgetAction("clearSearchWidgetContext"); - this.state.inTopicContext = false; - }, - - clearSearchWidgetContext() { - this.state.inTopicContext = false; - }, - - panelContents() { - const formFactor = this.state.formFactor; - let showHeaderResults = - this.state.showHeaderResults == null || - this.state.showHeaderResults === true; - let contents = []; - - let buttonClass = - I18n.t(themePrefix("search_banner.search_button_text")) === "" - ? "btn search-icon" - : "btn search-icon has-search-button-text"; - - if (formFactor === "widget") { - contents.push( - this.attach("link", { - href: this.fullSearchUrl({ expanded: true }), - contents: () => [ - iconNode("search"), - h( - "span", - I18n.t(themePrefix("search_banner.search_button_text")) - ), - ], - className: buttonClass, - title: "search.open_advanced", - }) - ); - } - - contents = contents.concat(...corePanelContents.call(this)); - if (formFactor === "menu" || showHeaderResults) { - return contents; - } else { - return contents.filter((widget) => { - return ( - widget.name !== "search-menu-results" && - widget.name !== "search-context" - ); - }); - } - }, - }); - - api.createWidget("search-widget", { - tagName: "div.search-widget", - - html() { - const searchMenuVisible = this.state.searchVisible; - - if (!searchMenuVisible && !this.attrs.topic) { - return this.attach("search-menu", { - contextEnabled: this.state.contextEnabled, - formFactor: "widget", - }); - } - }, - - clearSearchWidgetContext() { - this.state.inTopicContext = false; - }, - - setTopicContext() { - this.state.inTopicContext = true; - }, - }); }); diff --git a/javascripts/discourse/components/search-banner.hbs b/javascripts/discourse/components/search-banner.hbs index 8dbd788..3a54380 100644 --- a/javascripts/discourse/components/search-banner.hbs +++ b/javascripts/discourse/components/search-banner.hbs @@ -11,7 +11,9 @@

{{html-safe (theme-i18n "search_banner.headline")}}

{{html-safe (theme-i18n "search_banner.subhead")}}

- +
+ +
diff --git a/javascripts/discourse/connectors/search-menu-before-term-input/search-icon.gjs b/javascripts/discourse/connectors/search-menu-before-term-input/search-icon.gjs new file mode 100644 index 0000000..6fac1ce --- /dev/null +++ b/javascripts/discourse/connectors/search-menu-before-term-input/search-icon.gjs @@ -0,0 +1,25 @@ +import Component from "@glimmer/component"; +import DButton from "discourse/components/d-button"; +import I18n from "discourse-i18n"; + +export default class SearchIcon extends Component { + get buttonText() { + return I18n.t(themePrefix("search_banner.search_button_text")); + } + + get buttonClass() { + return this.buttonText + ? "btn search-icon has-search-button-text" + : "btn search-icon"; + } + + +} diff --git a/mobile/mobile.scss b/mobile/mobile.scss index eb65058..e981552 100644 --- a/mobile/mobile.scss +++ b/mobile/mobile.scss @@ -1,4 +1,4 @@ -.search-widget { +.search-menu { .search-link { .badge-category { display: inline-block; diff --git a/spec/system/viewing_search_banner_spec.rb b/spec/system/viewing_search_banner_spec.rb index 8106555..b32e7d2 100644 --- a/spec/system/viewing_search_banner_spec.rb +++ b/spec/system/viewing_search_banner_spec.rb @@ -2,6 +2,8 @@ RSpec.describe "Viewing the search banner", type: :system do fab!(:theme) { upload_theme_component } + fab!(:topic) + let(:topic_page) { PageObjects::Pages::Topic.new } it "should display the search banner below the site header when `plugin_outlet` theme setting is set to `below-site-header`" do theme.update_setting(:plugin_outlet, "below-site-header") @@ -21,4 +23,32 @@ RSpec.describe "Viewing the search banner", type: :system do expect(page).to have_css("#main-outlet .custom-search-banner") end + + it "should hide the search icon when searching within a topic" do + theme.update_setting(:show_on, "all") + theme.save! + + topic_page.visit_topic(topic) + expect(topic_page).to have_css(".custom-search-banner") + topic_page.find(".custom-search-banner input#search-term").fill_in(with: "test") + topic_page.find(".custom-search-banner .results li:nth-child(2) a").click + + expect(topic_page).to have_css(".custom-search-banner .search-context") + expect(topic_page).to_not have_css(".custom-search-banner .search-icon") + end + + it "should display the search icon when searching within a topic when search button text is present" do + theme.update_setting(:show_on, "all") + theme.update_translation("search_banner.search_button_text", "Foo") + theme.save! + + topic_page.visit_topic(topic) + expect(topic_page).to have_css(".custom-search-banner") + + topic_page.find(".custom-search-banner input#search-term").fill_in(with: "test") + topic_page.find(".custom-search-banner .results li:nth-child(2) a").click + + expect(topic_page).to have_css(".custom-search-banner .search-context") + expect(topic_page).to have_css(".custom-search-banner .search-icon") + end end \ No newline at end of file diff --git a/test/javascripts/acceptance/search-banner-test.js b/test/javascripts/acceptance/search-banner-test.js new file mode 100644 index 0000000..2aecfef --- /dev/null +++ b/test/javascripts/acceptance/search-banner-test.js @@ -0,0 +1,73 @@ +import { acceptance, query } from "discourse/tests/helpers/qunit-helpers"; +import { fillIn, triggerKeyEvent, visit } from "@ember/test-helpers"; +import { test } from "qunit"; + +acceptance("Discourse Search Banner", function (needs) { + needs.user({ admin: true, experimental_search_menu_groups_enabled: true }); + + test("Search Banner is present", async function (assert) { + await visit("/"); + assert.dom(".custom-search-banner input#search-term").exists(); + }); + + test("is only present on intended routes", async function (assert) { + await visit("/admin"); + assert.dom(".custom-search-banner").doesNotExist(); + }); + + test("it closes the search menu when clicking outside", async function (assert) { + await visit("/"); + await click(".custom-search-banner input#search-term"); + await fillIn(".custom-search-banner input#search-term", "test"); + assert.dom(".custom-search-banner .results").exists(); + + // select a element to simulate clicking outside the search banner + await click(".custom-search-banner h1"); + assert.dom(".custom-search-banner .results").doesNotExist(); + }); + + test("pressing escape closes the search menu", async function (assert) { + await visit("/"); + await click(".custom-search-banner input#search-term"); + await fillIn(".custom-search-banner input#search-term", "test"); + assert.dom(".custom-search-banner .results").exists(); + + await triggerKeyEvent( + ".custom-search-banner #search-term", + "keyup", + "Escape" + ); + assert.dom(".custom-search-banner .results").doesNotExist(); + }); + + test("searching for a term in the search menu fills in the search banner search input", async function (assert) { + await visit("/"); + await click("#search-button"); + await fillIn("#search-term", "test"); + assert + .dom(".custom-search-banner input#search-term") + .hasValue("test", "search inputs have matching terms"); + }); + + test("you can navigate search results with the keyboard", async function (assert) { + const container = ".custom-search-banner .results"; + + await visit("/"); + await click(".custom-search-banner input#search-term"); + await fillIn(".custom-search-banner input#search-term", "test"); + + await triggerKeyEvent( + ".custom-search-banner #search-term", + "keyup", + "Enter" + ); + assert.dom(`${container} .search-result-topic`).exists("has topic results"); + + await triggerKeyEvent(document.activeElement, "keyup", "ArrowDown"); + assert.strictEqual( + document.activeElement.getAttribute("href"), + query(`${container} li:first-child a`).getAttribute("href"), + "arrow down selects first element" + ); + }); +});