From 9bcbfbba43b876ae92cb90d6d619be7b1154c119 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 13 May 2024 14:40:23 +1000 Subject: [PATCH] FEATURE: Force admin sidebar for all admins in `admin_sidebar_enabled_groups` and handle legacy "hamburger dropdown" in this mode (#26899) Some sites are still on the legacy "hamburger dropdown" navigation_menu setting. In this case to avoid confusion, we want to show both the sidebar icon and the header dropdown hamburger when visiting the admin portal. Otherwise, the hamburger switches sides from right to left for admins and takes on different behaviour. The hamburger in this case _only_ shows the main panel, not other sidebar panels like the admin one. --- .../discourse/app/components/header.gjs | 44 +++++-- .../app/components/header/contents.gjs | 12 +- .../header/hamburger-dropdown-wrapper.gjs | 43 ++++++- .../discourse/app/components/header/icons.gjs | 32 ++++- .../app/components/header/sidebar-toggle.gjs | 10 +- .../components/sidebar/hamburger-dropdown.gjs | 6 +- .../app/components/sidebar/sections.gjs | 1 + .../app/components/sidebar/user/sections.gjs | 4 +- .../discourse/app/controllers/application.js | 8 ++ .../discourse/app/services/sidebar-state.js | 10 ++ .../discourse/app/widgets/header.js | 17 ++- .../editing_sidebar_community_section_spec.rb | 4 +- .../navigation_menu_state_integration_spec.rb | 119 ++++++++++++++++++ .../navigation_menu/header_dropdown.rb | 43 ++++++- .../components/navigation_menu/sidebar.rb | 8 ++ 15 files changed, 332 insertions(+), 29 deletions(-) create mode 100644 spec/system/navigation_menu_state_integration_spec.rb diff --git a/app/assets/javascripts/discourse/app/components/header.gjs b/app/assets/javascripts/discourse/app/components/header.gjs index 0b459b57074..54b33121332 100644 --- a/app/assets/javascripts/discourse/app/components/header.gjs +++ b/app/assets/javascripts/discourse/app/components/header.gjs @@ -40,6 +40,7 @@ export default class GlimmerHeader extends Component { @service router; @service search; @service currentUser; + @service siteSettings; @service site; @service appEvents; @service header; @@ -71,7 +72,7 @@ export default class GlimmerHeader extends Component { this.toggleUserMenu(); break; case "hamburger": - this.toggleHamburger(); + this.toggleNavigationMenu(); break; case "page-search": if (!this.togglePageSearch()) { @@ -148,15 +149,33 @@ export default class GlimmerHeader extends Component { } @action - toggleHamburger() { - if (this.args.sidebarEnabled && !this.site.narrowDesktopView) { - this.args.toggleSidebar(); - this.args.animateMenu(); - } else { - this.header.hamburgerVisible = !this.header.hamburgerVisible; - this.toggleBodyScrolling(this.header.hamburgerVisible); - this.args.animateMenu(); + toggleNavigationMenu(override = null) { + if (override === "sidebar") { + return this.toggleSidebar(); } + + if (override === "hamburger") { + return this.toggleHamburger(); + } + + if (this.args.sidebarEnabled && !this.site.narrowDesktopView) { + this.toggleSidebar(); + } else { + this.toggleHamburger(); + } + } + + @action + toggleHamburger() { + this.header.hamburgerVisible = !this.header.hamburgerVisible; + this.toggleBodyScrolling(this.header.hamburgerVisible); + this.args.animateMenu(); + } + + @action + toggleSidebar() { + this.args.toggleSidebar(); + this.args.animateMenu(); } @action @@ -171,7 +190,7 @@ export default class GlimmerHeader extends Component {
@@ -194,7 +213,7 @@ export default class GlimmerHeader extends Component { @@ -204,7 +223,8 @@ export default class GlimmerHeader extends Component { {{else if this.header.hamburgerVisible}} {{else if this.header.userVisible}} diff --git a/app/assets/javascripts/discourse/app/components/header/contents.gjs b/app/assets/javascripts/discourse/app/components/header/contents.gjs index 25d6fe628e2..b4104daa926 100644 --- a/app/assets/javascripts/discourse/app/components/header/contents.gjs +++ b/app/assets/javascripts/discourse/app/components/header/contents.gjs @@ -13,18 +13,28 @@ export default class Contents extends Component { @service currentUser; @service siteSettings; @service header; + @service sidebarState; get topicPresent() { return !!this.header.topic; } + get sidebarIcon() { + if (this.sidebarState.adminSidebarAllowedWithLegacyNavigationMenu) { + return "discourse-sidebar"; + } + + return "bars"; + } + } diff --git a/app/assets/javascripts/discourse/app/controllers/application.js b/app/assets/javascripts/discourse/app/controllers/application.js index 88f60f2e295..40202a78a97 100644 --- a/app/assets/javascripts/discourse/app/controllers/application.js +++ b/app/assets/javascripts/discourse/app/controllers/application.js @@ -97,6 +97,14 @@ export default Controller.extend({ return false; } + // Always show sidebar for admin if user can see the admin sidbar + if ( + this.router.currentRouteName.startsWith("admin.") && + this.currentUser?.use_admin_sidebar + ) { + return true; + } + return this.siteSettings.navigation_menu === "sidebar"; }, diff --git a/app/assets/javascripts/discourse/app/services/sidebar-state.js b/app/assets/javascripts/discourse/app/services/sidebar-state.js index 0157bd4677d..f71d29f1105 100644 --- a/app/assets/javascripts/discourse/app/services/sidebar-state.js +++ b/app/assets/javascripts/discourse/app/services/sidebar-state.js @@ -16,11 +16,14 @@ import { @disableImplicitInjections export default class SidebarState extends Service { @service keyValueStore; + @service currentUser; + @service siteSettings; @tracked currentPanelKey = currentPanelKey; @tracked mode = COMBINED_MODE; @tracked displaySwitchPanelButtons = false; @tracked filter = ""; + panels = panels; collapsedSections = new TrackedSet(); previousState = {}; @@ -119,6 +122,13 @@ export default class SidebarState extends Service { return this.currentPanelKey === MAIN_PANEL; } + get adminSidebarAllowedWithLegacyNavigationMenu() { + return ( + this.currentUser?.use_admin_sidebar && + this.siteSettings.navigation_menu === "header dropdown" + ); + } + clearFilter() { this.filter = ""; } diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index 2bde3e8d883..61f4174c105 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -289,6 +289,16 @@ createWidget("header-icons", { } }); + if (attrs.user) { + icons.push( + this.attach("user-dropdown", { + active: attrs.userVisible, + action: "toggleUserMenu", + user: attrs.user, + }) + ); + } + return icons; }, }); @@ -644,7 +654,12 @@ export default createWidget("header", { toggleHamburger() { if (this.attrs.sidebarEnabled && !this.site.narrowDesktopView) { - this.sendWidgetAction("toggleSidebar"); + if (!this.attrs.showSidebar) { + this.sendWidgetAction("toggleSidebar"); + this.closeAll(); + } else { + this.state.hamburgerVisible = !this.state.hamburgerVisible; + } } else { this.state.hamburgerVisible = !this.state.hamburgerVisible; this.toggleBodyScrolling(this.state.hamburgerVisible); diff --git a/spec/system/editing_sidebar_community_section_spec.rb b/spec/system/editing_sidebar_community_section_spec.rb index 997ce6d12c2..7cb08ecbbb9 100644 --- a/spec/system/editing_sidebar_community_section_spec.rb +++ b/spec/system/editing_sidebar_community_section_spec.rb @@ -78,7 +78,9 @@ RSpec.describe "Editing Sidebar Community Section", type: :system do visit("/latest") - modal = sidebar_header_dropdown.open.click_customize_community_section_button + sidebar_header_dropdown.open + expect(sidebar_header_dropdown).to have_dropdown_visible + modal = sidebar_header_dropdown.click_customize_community_section_button expect(modal).to be_visible end diff --git a/spec/system/navigation_menu_state_integration_spec.rb b/spec/system/navigation_menu_state_integration_spec.rb new file mode 100644 index 00000000000..0a879bc83e6 --- /dev/null +++ b/spec/system/navigation_menu_state_integration_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +describe "Navigation menu states", type: :system, js: true do + fab!(:current_user) { Fabricate(:user) } + let!(:sidebar_navigation) { PageObjects::Components::NavigationMenu::Sidebar.new } + let!(:header_dropdown) { PageObjects::Components::NavigationMenu::HeaderDropdown.new } + + before { sign_in(current_user) } + + context "when navigation_menu is 'header dropdown'" do + before { SiteSetting.navigation_menu = "header dropdown" } + + it "does not show the sidebar" do + visit "/" + expect(sidebar_navigation).to be_not_visible + end + + it "opens and closes the hamburger menu from the toggle" do + visit "/" + expect(header_dropdown).to be_visible + header_dropdown.open + expect(header_dropdown).to have_dropdown_visible + expect(header_dropdown).to have_sidebar_panel("main") + expect(header_dropdown).to have_no_sidebar_panel("admin") + header_dropdown.close + expect(header_dropdown).to have_no_dropdown_visible + end + + context "for admins" do + fab!(:current_user) { Fabricate(:admin, refresh_auto_groups: true) } + + it "shows the sidebar and allows toggling it" do + visit "/admin" + expect(sidebar_navigation).to be_visible + sidebar_navigation.click_header_toggle + expect(sidebar_navigation).to be_not_visible + sidebar_navigation.click_header_toggle + expect(sidebar_navigation).to be_visible + expect(find(sidebar_navigation.header_toggle_css)).to have_css(".d-icon-discourse-sidebar") + end + + it "shows the hamburger menu and allows toggling it, which shows the MAIN_PANEL onls" do + visit "/admin" + expect(header_dropdown).to be_visible + header_dropdown.open + expect(header_dropdown).to have_dropdown_visible + expect(header_dropdown).to have_sidebar_panel("main") + expect(header_dropdown).to have_no_sidebar_panel("admin") + header_dropdown.close + expect(header_dropdown).to have_no_dropdown_visible + end + + context "when the user is not in admin_sidebar_enabled_groups" do + before { SiteSetting.admin_sidebar_enabled_groups = "" } + + it "does not show the sidebar" do + visit "/admin" + expect(sidebar_navigation).to be_not_visible + end + end + end + end + + context "when navigation_menu is 'sidebar'" do + before { SiteSetting.navigation_menu = "sidebar" } + + it "shows the sidebar" do + visit "/" + expect(sidebar_navigation).to be_visible + end + + it "does not show the hamburger menu" do + visit "/" + expect(header_dropdown).to be_not_visible + end + + it "opens and closes the sidebar from the toggle" do + visit "/" + sidebar_navigation.click_header_toggle + expect(sidebar_navigation).to be_not_visible + sidebar_navigation.click_header_toggle + expect(sidebar_navigation).to be_visible + end + + context "for admins" do + fab!(:current_user) { Fabricate(:admin, refresh_auto_groups: true) } + + it "shows the sidebar and allows toggling it" do + visit "/admin" + expect(sidebar_navigation).to be_visible + sidebar_navigation.click_header_toggle + expect(sidebar_navigation).to be_not_visible + sidebar_navigation.click_header_toggle + expect(sidebar_navigation).to be_visible + expect(find(sidebar_navigation.header_toggle_css)).to have_css(".d-icon-bars") + end + + it "does not show the hamburger menu" do + visit "/admin" + expect(header_dropdown).to be_not_visible + end + + context "when the user is not in admin_sidebar_enabled_groups" do + before { SiteSetting.admin_sidebar_enabled_groups = "" } + + it "shows the MAIN_PANEL of the sidebar" do + visit "/admin" + expect(sidebar_navigation).to have_no_section("admin-root") + expect(sidebar_navigation).to have_section("community") + end + + it "does show the sidebar toggle" do + visit "/admin" + expect(page).to have_css(sidebar_navigation.header_toggle_css) + end + end + end + end +end diff --git a/spec/system/page_objects/components/navigation_menu/header_dropdown.rb b/spec/system/page_objects/components/navigation_menu/header_dropdown.rb index 54bd9e79208..c2370b04c4e 100644 --- a/spec/system/page_objects/components/navigation_menu/header_dropdown.rb +++ b/spec/system/page_objects/components/navigation_menu/header_dropdown.rb @@ -6,10 +6,41 @@ module PageObjects class HeaderDropdown < Base def open find(".header-dropdown-toggle.hamburger-dropdown").click - expect(page).to have_css(".sidebar-hamburger-dropdown") self end + def close + open + end + + def has_sidebar_panel?(panel) + has_css?( + ".sidebar-hamburger-dropdown .sidebar-section-wrapper[data-section-name=\"#{panel_id(panel)}\"]", + ) + end + + def has_no_sidebar_panel?(panel) + has_no_css?( + ".sidebar-hamburger-dropdown .sidebar-section-wrapper[data-section-name=\"#{panel_id(panel)}\"]", + ) + end + + def has_dropdown_visible? + page.has_css?(".sidebar-hamburger-dropdown") + end + + def has_no_dropdown_visible? + page.has_no_css?(".sidebar-hamburger-dropdown") + end + + def visible? + page.has_css?(".hamburger-dropdown.header-dropdown-toggle") + end + + def not_visible? + page.has_no_css?(".hamburger-dropdown.header-dropdown-toggle") + end + def click_customize_community_section_button community_section.click_button( I18n.t("js.sidebar.sections.community.edit_section.header_dropdown"), @@ -19,6 +50,16 @@ module PageObjects PageObjects::Modals::SidebarSectionForm.new end + + private + + def panel_id(panel) + if panel == "admin" + "admin-root" + elsif panel == "main" + "community" + end + end end end end diff --git a/spec/system/page_objects/components/navigation_menu/sidebar.rb b/spec/system/page_objects/components/navigation_menu/sidebar.rb index 5e74c1843da..b1c5c3db7c9 100644 --- a/spec/system/page_objects/components/navigation_menu/sidebar.rb +++ b/spec/system/page_objects/components/navigation_menu/sidebar.rb @@ -9,6 +9,14 @@ module PageObjects wait_for_animation(find("div.menu-panel")) end + def click_header_toggle + find(header_toggle_css).click + end + + def header_toggle_css + ".header-sidebar-toggle" + end + def visible? page.has_css?("#d-sidebar") end