From c7b2369bfa12a5e2fbeeecda75f4e29fec1b9ab8 Mon Sep 17 00:00:00 2001 From: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com> Date: Thu, 7 Mar 2024 11:14:43 -0700 Subject: [PATCH] FIX: In topic search for glimmer header (#26040) - Fix cmd + f keyboard shortcut that opens up the search menu or browser search --- .../app/components/glimmer-header.gjs | 11 ++-- spec/system/header_spec.rb | 66 +++++++++---------- spec/system/page_objects/pages/header.rb | 29 ++++++++ spec/system/page_objects/pages/search.rb | 8 +++ 4 files changed, 76 insertions(+), 38 deletions(-) create mode 100644 spec/system/page_objects/pages/header.rb diff --git a/app/assets/javascripts/discourse/app/components/glimmer-header.gjs b/app/assets/javascripts/discourse/app/components/glimmer-header.gjs index b001fded75d..ef716bbf7da 100644 --- a/app/assets/javascripts/discourse/app/components/glimmer-header.gjs +++ b/app/assets/javascripts/discourse/app/components/glimmer-header.gjs @@ -1,5 +1,6 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; +import { getOwner } from "@ember/application"; import { action } from "@ember/object"; import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import { service } from "@ember/service"; @@ -24,7 +25,6 @@ export default class GlimmerHeader extends Component { @service currentUser; @service site; @service appEvents; - @service register; @service header; @tracked skipSearchContext = this.site.mobileView; @@ -99,12 +99,13 @@ export default class GlimmerHeader extends Component { let showSearch = this.router.currentRouteName.startsWith("topic."); // If we're viewing a topic, only intercept search if there are cloaked posts if (showSearch) { - const controller = this.register.lookup("controller:topic"); - const total = controller.get("model.postStream.stream.length") || 0; - const chunkSize = controller.get("model.chunk_size") || 0; + const container = getOwner(this); + const topic = container.lookup("controller:topic"); + const total = topic.get("model.postStream.stream.length") || 0; + const chunkSize = topic.get("model.chunk_size") || 0; showSearch = total > chunkSize && - document.querySelector( + document.querySelectorAll( ".topic-post .cooked, .small-action:not(.time-gap)" )?.length < total; } diff --git a/spec/system/header_spec.rb b/spec/system/header_spec.rb index 48d3e2a7e85..e448308929c 100644 --- a/spec/system/header_spec.rb +++ b/spec/system/header_spec.rb @@ -1,7 +1,10 @@ # frozen_string_literal: true RSpec.describe "Glimmer Header", type: :system do + let(:header) { PageObjects::Pages::Header.new } + let(:search) { PageObjects::Pages::Search.new } fab!(:current_user) { Fabricate(:user) } + fab!(:topic) before { SiteSetting.experimental_glimmer_header_groups = Group::AUTO_GROUPS[:everyone] } it "renders basics" do @@ -18,7 +21,7 @@ RSpec.describe "Glimmer Header", type: :system do find("button.sign-up-button").click expect(page).to have_css(".d-modal.create-account") - click_outside + header.click_outside find("button.login-button").click expect(page).to have_css(".d-modal.login-modal") @@ -73,32 +76,36 @@ RSpec.describe "Glimmer Header", type: :system do it "sets header's height css property" do sign_in(current_user) visit "/" - resize_element(".d-header", 90) - wait_for(timeout: 100) { get_computed_style_value(".d-header", "--header-offset") == "90px" } - expect(get_computed_style_value(".d-header", "--header-offset")).to eq("90px") + header.resize_element(".d-header", 90) + wait_for(timeout: 100) do + header.get_computed_style_value(".d-header", "--header-offset") == "90px" + end + expect(header.get_computed_style_value(".d-header", "--header-offset")).to eq("90px") - resize_element(".d-header", 60) - wait_for(timeout: 100) { get_computed_style_value(".d-header", "--header-offset") == "60px" } - expect(get_computed_style_value(".d-header", "--header-offset")).to eq("60px") + header.resize_element(".d-header", 60) + wait_for(timeout: 100) do + header.get_computed_style_value(".d-header", "--header-offset") == "60px" + end + expect(header.get_computed_style_value(".d-header", "--header-offset")).to eq("60px") end it "moves focus between tabs using arrow keys" do sign_in(current_user) visit "/" find(".header-dropdown-toggle.current-user").click - expect(active_element_id).to eq("user-menu-button-all-notifications") + expect(header.active_element_id).to eq("user-menu-button-all-notifications") - find("##{active_element_id}").send_keys(:arrow_down) - expect(active_element_id).to eq("user-menu-button-replies") + find("##{header.active_element_id}").send_keys(:arrow_down) + expect(header.active_element_id).to eq("user-menu-button-replies") - 4.times { find("##{active_element_id}").send_keys(:arrow_down) } - expect(active_element_id).to eq("user-menu-button-profile") + 4.times { find("##{header.active_element_id}").send_keys(:arrow_down) } + expect(header.active_element_id).to eq("user-menu-button-profile") - find("##{active_element_id}").send_keys(:arrow_down) - expect(active_element_id).to eq("user-menu-button-all-notifications") + find("##{header.active_element_id}").send_keys(:arrow_down) + expect(header.active_element_id).to eq("user-menu-button-all-notifications") - find("##{active_element_id}").send_keys(:arrow_up) - expect(active_element_id).to eq("user-menu-button-profile") + find("##{header.active_element_id}").send_keys(:arrow_up) + expect(header.active_element_id).to eq("user-menu-button-profile") end it "prioritizes new personal messages bubble over unseen reviewables and regular notifications bubbles" do @@ -189,23 +196,16 @@ RSpec.describe "Glimmer Header", type: :system do end end - private + context "when cmd + f keyboard shortcut pressed - when within a topic with 20+ posts" do + before { sign_in(current_user) } + fab!(:posts) { Fabricate.times(21, :post, topic: topic) } - def get_computed_style_value(selector, property) - page.evaluate_script( - "window.getComputedStyle(document.querySelector('#{selector}')).getPropertyValue('#{property}')", - ).strip - end - - def resize_element(selector, size) - page.evaluate_script("document.querySelector('#{selector}').style.height = '#{size}px'") - end - - def active_element_id - page.evaluate_script("document.activeElement.id") - end - - def click_outside - find(".d-modal").click(x: 0, y: 0) + it "opens search on first press, and closes on the second" do + visit "/t/#{topic.slug}/#{topic.id}" + header.search_in_topic_keyboard_shortcut + expect(search).to have_search_menu_visible + header.search_in_topic_keyboard_shortcut + expect(search).to have_no_search_menu_visible + end end end diff --git a/spec/system/page_objects/pages/header.rb b/spec/system/page_objects/pages/header.rb new file mode 100644 index 00000000000..3e4252f3ed3 --- /dev/null +++ b/spec/system/page_objects/pages/header.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class Header < PageObjects::Pages::Base + def get_computed_style_value(selector, property) + page.evaluate_script( + "window.getComputedStyle(document.querySelector('#{selector}')).getPropertyValue('#{property}')", + ).strip + end + + def resize_element(selector, size) + page.evaluate_script("document.querySelector('#{selector}').style.height = '#{size}px'") + end + + def active_element_id + page.evaluate_script("document.activeElement.id") + end + + def click_outside + find(".d-modal").click(x: 0, y: 0) + end + + def search_in_topic_keyboard_shortcut + page.send_keys([PLATFORM_KEY_MODIFIER, "f"]) + end + end + end +end diff --git a/spec/system/page_objects/pages/search.rb b/spec/system/page_objects/pages/search.rb index 14d7eb38ef1..caba892aa23 100644 --- a/spec/system/page_objects/pages/search.rb +++ b/spec/system/page_objects/pages/search.rb @@ -42,6 +42,14 @@ module PageObjects find(".topic-list-body tr:first-of-type").click end + def has_search_menu_visible? + page.has_selector?(".search-menu .search-menu-panel", visible: true) + end + + def has_no_search_menu_visible? + page.has_no_selector?(".search-menu .search-menu-panel") + end + SEARCH_RESULT_SELECTOR = ".search-results .fps-result" def has_topic_title_for_first_search_result?(title)