From 8180770e7b97a440d6f6324000e7f077b556bf79 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 20 Mar 2024 09:20:06 +1000 Subject: [PATCH] FIX: Do not lose admin sidebar when opening chat drawer (#26235) This commit fixes an issue where the following happens: 1. You open /admin as a member of the admin_sidebar_enabled_groups 1. You then click the chat icon in the header when you prefer to have drawer open, or if you just minimise chat into drawer after it opens fullscreen 1. You lose the admin sidebar panel, and are reset instead to the main panel Also included is a bit of refactoring to make it so the forcing of admin sidebar state is in one place. --- .../admin/addon/routes/admin-revamp.js | 15 +++--- .../javascripts/admin/addon/routes/admin.js | 12 ++--- .../services/admin-sidebar-state-manager.js | 38 +++++++++++++- .../discourse/services/chat-state-manager.js | 50 +++++++++++++------ .../system/admin_sidebar_navigation_spec.rb | 19 +++++++ 5 files changed, 104 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/admin/addon/routes/admin-revamp.js b/app/assets/javascripts/admin/addon/routes/admin-revamp.js index fd24bbf25b9..dfbf88d0eae 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-revamp.js +++ b/app/assets/javascripts/admin/addon/routes/admin-revamp.js @@ -1,5 +1,5 @@ import { service } from "@ember/service"; -import { ADMIN_PANEL, MAIN_PANEL } from "discourse/lib/sidebar/panels"; +import { MAIN_PANEL } from "discourse/lib/sidebar/panels"; import DiscourseURL from "discourse/lib/url"; import DiscourseRoute from "discourse/routes/discourse"; import I18n from "discourse-i18n"; @@ -8,6 +8,7 @@ export default class AdminRoute extends DiscourseRoute { @service siteSettings; @service currentUser; @service sidebarState; + @service adminSidebarStateManager; titleToken() { return I18n.t("admin_title"); @@ -18,9 +19,9 @@ export default class AdminRoute extends DiscourseRoute { return DiscourseURL.redirectTo("/admin"); } - this.sidebarState.setPanel(ADMIN_PANEL); - this.sidebarState.setSeparatedMode(); - this.sidebarState.hideSwitchPanelButtons(); + this.adminSidebarStateManager.maybeForceAdminSidebar({ + onlyIfAlreadyActive: false, + }); this.controllerFor("application").setProperties({ showTop: false, @@ -29,8 +30,10 @@ export default class AdminRoute extends DiscourseRoute { deactivate(transition) { this.controllerFor("application").set("showTop", true); - if (!transition?.to.name.startsWith("admin")) { - this.sidebarState.setPanel(MAIN_PANEL); + if (this.adminSidebarStateManager.currentUserUsingAdminSidebar) { + if (!transition?.to.name.startsWith("admin")) { + this.sidebarState.setPanel(MAIN_PANEL); + } } } } diff --git a/app/assets/javascripts/admin/addon/routes/admin.js b/app/assets/javascripts/admin/addon/routes/admin.js index d395437808d..74e0dca7be8 100644 --- a/app/assets/javascripts/admin/addon/routes/admin.js +++ b/app/assets/javascripts/admin/addon/routes/admin.js @@ -1,7 +1,7 @@ import { tracked } from "@glimmer/tracking"; import { service } from "@ember/service"; import PreloadStore from "discourse/lib/preload-store"; -import { ADMIN_PANEL, MAIN_PANEL } from "discourse/lib/sidebar/panels"; +import { MAIN_PANEL } from "discourse/lib/sidebar/panels"; import DiscourseRoute from "discourse/routes/discourse"; import I18n from "discourse-i18n"; @@ -18,11 +18,9 @@ export default class AdminRoute extends DiscourseRoute { } activate() { - if (this.currentUser.use_admin_sidebar) { - this.sidebarState.setPanel(ADMIN_PANEL); - this.sidebarState.setSeparatedMode(); - this.sidebarState.hideSwitchPanelButtons(); - } + this.adminSidebarStateManager.maybeForceAdminSidebar({ + onlyIfAlreadyActive: false, + }); this.controllerFor("application").setProperties({ showTop: false, @@ -39,7 +37,7 @@ export default class AdminRoute extends DiscourseRoute { deactivate(transition) { this.controllerFor("application").set("showTop", true); - if (this.currentUser.use_admin_sidebar) { + if (this.adminSidebarStateManager.currentUserUsingAdminSidebar) { if (!transition?.to.name.startsWith("admin")) { this.sidebarState.setPanel(MAIN_PANEL); } diff --git a/app/assets/javascripts/admin/addon/services/admin-sidebar-state-manager.js b/app/assets/javascripts/admin/addon/services/admin-sidebar-state-manager.js index 43b4987fff7..1dde6816fc1 100644 --- a/app/assets/javascripts/admin/addon/services/admin-sidebar-state-manager.js +++ b/app/assets/javascripts/admin/addon/services/admin-sidebar-state-manager.js @@ -1,10 +1,14 @@ import { tracked } from "@glimmer/tracking"; -import Service from "@ember/service"; +import Service, { service } from "@ember/service"; import { TrackedObject } from "@ember-compat/tracked-built-ins"; import KeyValueStore from "discourse/lib/key-value-store"; +import { ADMIN_PANEL } from "discourse/lib/sidebar/panels"; export default class AdminSidebarStateManager extends Service { + @service sidebarState; + @service currentUser; @tracked keywords = new TrackedObject(); + STORE_NAMESPACE = "discourse_admin_sidebar_experiment_"; store = new KeyValueStore(this.STORE_NAMESPACE); @@ -16,4 +20,36 @@ export default class AdminSidebarStateManager extends Service { set navConfig(value) { this.store.setObject({ key: "navConfig", value }); } + + get currentUserUsingAdminSidebar() { + return this.currentUser?.use_admin_sidebar; + } + + maybeForceAdminSidebar(opts = {}) { + opts.onlyIfAlreadyActive ??= true; + + const isAdminSidebarActive = + this.sidebarState.currentPanel?.key === ADMIN_PANEL; + + if (!this.currentUserUsingAdminSidebar) { + return false; + } + + if (!opts.onlyIfAlreadyActive) { + return this.#forceAdminSidebar(); + } + + if (isAdminSidebarActive) { + return this.#forceAdminSidebar(); + } else { + return false; + } + } + + #forceAdminSidebar() { + this.sidebarState.setPanel(ADMIN_PANEL); + this.sidebarState.setSeparatedMode(); + this.sidebarState.hideSwitchPanelButtons(); + return true; + } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js index 15107d40a87..d21e5b89df0 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js @@ -62,12 +62,20 @@ export default class ChatStateManager extends Service { didOpenDrawer(url = null) { withPluginApi("1.8.0", (api) => { - if (getUserChatSeparateSidebarMode(this.currentUser).always) { - api.setSidebarPanel(MAIN_PANEL); - api.setSeparatedSidebarMode(); - api.hideSidebarSwitchPanelButtons(); - } else { - api.setCombinedSidebarMode(); + const adminSidebarStateManager = api.container.lookup( + "service:admin-sidebar-state-manager" + ); + + if ( + adminSidebarStateManager === undefined || + !adminSidebarStateManager.maybeForceAdminSidebar() + ) { + if (getUserChatSeparateSidebarMode(this.currentUser).always) { + api.setSeparatedSidebarMode(); + api.hideSidebarSwitchPanelButtons(); + } else { + api.setCombinedSidebarMode(); + } } }); @@ -84,20 +92,30 @@ export default class ChatStateManager extends Service { didCloseDrawer() { withPluginApi("1.8.0", (api) => { - api.setSidebarPanel(MAIN_PANEL); + const adminSidebarStateManager = api.container.lookup( + "service:admin-sidebar-state-manager" + ); const chatSeparateSidebarMode = getUserChatSeparateSidebarMode( this.currentUser ); - if (chatSeparateSidebarMode.fullscreen) { - api.setCombinedSidebarMode(); - api.showSidebarSwitchPanelButtons(); - } else if (chatSeparateSidebarMode.always) { - api.setSeparatedSidebarMode(); - api.showSidebarSwitchPanelButtons(); - } else { - api.setCombinedSidebarMode(); - api.hideSidebarSwitchPanelButtons(); + + if ( + adminSidebarStateManager === undefined || + !adminSidebarStateManager.maybeForceAdminSidebar() + ) { + api.setSidebarPanel(MAIN_PANEL); + + if (chatSeparateSidebarMode.fullscreen) { + api.setCombinedSidebarMode(); + api.showSidebarSwitchPanelButtons(); + } else if (chatSeparateSidebarMode.always) { + api.setSeparatedSidebarMode(); + api.showSidebarSwitchPanelButtons(); + } else { + api.setCombinedSidebarMode(); + api.hideSidebarSwitchPanelButtons(); + } } }); diff --git a/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb b/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb index 8505bc7cc1d..6ebb4093ffa 100644 --- a/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb +++ b/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb @@ -3,6 +3,7 @@ describe "Admin Revamp | Sidebar Navigation | Plugin Links", type: :system do fab!(:admin) let(:sidebar) { PageObjects::Components::NavigationMenu::Sidebar.new } + let(:chat_page) { PageObjects::Pages::Chat.new } before do chat_system_bootstrap @@ -56,5 +57,23 @@ describe "Admin Revamp | Sidebar Navigation | Plugin Links", type: :system do find("#site-logo").click expect(sidebar).to have_section("chat-channels") end + + it "keeps the admin sidebar open instead of switching to the main panel when toggling the drawer" do + membership = + Fabricate( + :user_chat_channel_membership, + user: admin, + chat_channel: Fabricate(:chat_channel), + ) + admin.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => membership.chat_channel.id) + chat_page.prefers_full_page + visit("/admin") + expect(sidebar).to have_section("admin-nav-section-root") + chat_page.open_from_header + expect(sidebar).to have_no_section("admin-nav-section-root") + chat_page.minimize_full_page + expect(chat_page).to have_drawer + expect(sidebar).to have_section("admin-nav-section-root") + end end end