From eae2370424be89054db963770f53af28b60b55a8 Mon Sep 17 00:00:00 2001 From: Yuriy Kurant Date: Wed, 1 Oct 2025 17:34:15 +0300 Subject: [PATCH] FIX: hides redundant chat icon on mobile chat routes (#35015) ### Chat in Mobile view In order to reduce visual noise in mobile view, there is no need to show chat icon in header, once user has already chat open. |Before|After| |---|---| |Screenshot 2025-10-01 at 17 16 29|Screenshot 2025-10-01 at 17 16 06| --- .../discourse/components/chat/header/icon.gjs | 41 +++++++++---------- .../message_notifications_mobile_spec.rb | 32 ++++----------- plugins/chat/spec/system/navigation_spec.rb | 4 +- .../spec/system/page_objects/chat/chat.rb | 9 ++++ .../page_objects/chat/components/footer.rb | 17 ++++++++ .../components/chat-header-icon-test.gjs | 21 ++++++++-- 6 files changed, 73 insertions(+), 51 deletions(-) create mode 100644 plugins/chat/spec/system/page_objects/chat/components/footer.rb diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/header/icon.gjs b/plugins/chat/assets/javascripts/discourse/components/chat/header/icon.gjs index 28e15e81ed4..9ef2c07b0e3 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/header/icon.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/header/icon.gjs @@ -1,5 +1,6 @@ import Component from "@glimmer/component"; import { service } from "@ember/service"; +import { and } from "truth-helpers"; import DButton from "discourse/components/d-button"; import concatClass from "discourse/helpers/concat-class"; import icon from "discourse/helpers/d-icon"; @@ -61,10 +62,6 @@ export default class ChatHeaderIcon extends Component { } get href() { - if (this.site.mobileView && this.chatStateManager.isFullPageActive) { - return getURL("/chat"); - } - if ( this.chatStateManager.isFullPageActive && !this.chatSeparateSidebarMode.never @@ -80,22 +77,24 @@ export default class ChatHeaderIcon extends Component { } } diff --git a/plugins/chat/spec/system/message_notifications_mobile_spec.rb b/plugins/chat/spec/system/message_notifications_mobile_spec.rb index bbd2e1c6558..619940e2c50 100644 --- a/plugins/chat/spec/system/message_notifications_mobile_spec.rb +++ b/plugins/chat/spec/system/message_notifications_mobile_spec.rb @@ -84,8 +84,8 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do create_message(channel_1, user: user_1) - expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "") expect(channels_index_page).to have_unread_channel(channel_1) + expect(chat_page.footer).to have_unread_channels end end @@ -101,8 +101,8 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do message: "hello @#{current_user.username} what's up?", ) - expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator") expect(channels_index_page).to have_unread_channel(channel_1, count: 1) + expect(chat_page.footer).to have_unread_channels end it "shows correct count when there are multiple messages but only 1 is urgent" do @@ -118,11 +118,8 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do 3.times { create_message(channel_1, user: user_1) } - expect(page).to have_css( - ".chat-header-icon .chat-channel-unread-indicator", - text: "1", - ) expect(channels_index_page).to have_unread_channel(channel_1, count: 1) + expect(chat_page.footer).to have_unread_channels end end end @@ -141,21 +138,12 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do visit("/chat/direct-messages") create_message(dm_channel_1, user: user_1) - - expect(page).to have_css( - ".chat-header-icon .chat-channel-unread-indicator", - text: "1", - wait: 25, - ) - expect(channels_index_page).to have_unread_channel(dm_channel_1, wait: 25) + expect(channels_index_page).to have_unread_channel(dm_channel_1, count: 1, wait: 25) + expect(chat_page.footer).to have_unread_dms("1") create_message(dm_channel_1, user: user_1) - - expect(page).to have_css( - ".chat-header-icon .chat-channel-unread-indicator", - text: "2", - wait: 25, - ) + expect(channels_index_page).to have_unread_channel(dm_channel_1, count: 2, wait: 25) + expect(chat_page.footer).to have_unread_dms("2") end it "reorders channels" do @@ -223,18 +211,12 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do context "when messages are created" do it "correctly renders notifications" do visit("/chat/channels") - create_message(channel_1, user: user_1) - - expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "") expect(channels_index_page).to have_unread_channel(channel_1) visit("/chat/direct-messages") - create_message(dm_channel_1, user: user_1) - expect(channels_index_page).to have_unread_channel(dm_channel_1) - expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "1") end end end diff --git a/plugins/chat/spec/system/navigation_spec.rb b/plugins/chat/spec/system/navigation_spec.rb index fa3a00f98ef..0de48253cb7 100644 --- a/plugins/chat/spec/system/navigation_spec.rb +++ b/plugins/chat/spec/system/navigation_spec.rb @@ -38,10 +38,10 @@ RSpec.describe "Navigation", type: :system do end end - context "when clicking chat icon on mobile and is viewing channel" do + context "when clicking back button on mobile and is viewing channel" do it "navigates to channels tab", mobile: true do chat_page.visit_channel(category_channel_2) - chat_page.open_from_header + chat_page.back_to_channels_list expect(page).to have_current_path("/chat/channels") end diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index bf249ccdaf3..04269ef23dd 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -11,6 +11,10 @@ module PageObjects @sidebar ||= PageObjects::Components::Chat::Sidebar.new end + def footer + @footer ||= PageObjects::Components::Chat::Footer.new + end + def prefers_full_page page.execute_script( "window.localStorage.setItem('discourse_chat_preferred_mode', '\"FULL_PAGE_CHAT\"');", @@ -33,6 +37,11 @@ module PageObjects has_no_css?("html.has-chat") end + def back_to_channels_list + find(".d-icon.d-icon-chevron-left").click + has_css?("html.has-chat") + end + def has_header_href?(href) find(".chat-header-icon").has_link?(href: href) end diff --git a/plugins/chat/spec/system/page_objects/chat/components/footer.rb b/plugins/chat/spec/system/page_objects/chat/components/footer.rb new file mode 100644 index 00000000000..fb4a6b92883 --- /dev/null +++ b/plugins/chat/spec/system/page_objects/chat/components/footer.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module PageObjects + module Components + module Chat + class Footer < PageObjects::Components::Base + def has_unread_channels? + has_css?(".c-footer #c-footer-channels .c-unread-indicator") + end + + def has_unread_dms?(text) + has_css?(".c-footer #c-footer-direct-messages .c-unread-indicator", text: text) + end + end + end + end +end diff --git a/plugins/chat/test/javascripts/components/chat-header-icon-test.gjs b/plugins/chat/test/javascripts/components/chat-header-icon-test.gjs index 599a7a23e97..747749770c7 100644 --- a/plugins/chat/test/javascripts/components/chat-header-icon-test.gjs +++ b/plugins/chat/test/javascripts/components/chat-header-icon-test.gjs @@ -1,4 +1,5 @@ -import { render } from "@ember/test-helpers"; +import { tracked } from "@glimmer/tracking"; +import { render, settled } from "@ember/test-helpers"; import { module, test } from "qunit"; import sinon from "sinon"; import { forceMobile } from "discourse/lib/mobile"; @@ -43,16 +44,30 @@ module("Discourse Chat | Component | chat-header-icon", function (hooks) { }); test("mobile", async function (assert) { + const testState = new (class { + @tracked isActive = false; + })(); + forceMobile(); - await render(); + await render( + + ); assert .dom(".icon.btn-flat") .hasAttribute("title", i18n("chat.title_capitalized")) .hasAttribute("href", "/chat"); - assert.dom(".d-icon-d-chat").exists(); + assert + .dom(".d-icon-d-chat") + .exists("chat icon is rendered if chat is inactive"); + + testState.isActive = true; + await settled(); + assert + .dom(".d-icon-d-chat") + .doesNotExist("chat icon is not rendered if chat is active"); }); test("full page - with unread", async function (assert) {