FIX: Do not duplicate admin sidebar plugin links (#25780)

When hiding/showing the sidebar, as is the case on mobile
and using the toggle in the top left on desktop, we delete
and recreate the ember component on the page. This causes
the `sections` for each sidebar panel to get re-evaluated
every time.

For the admin sidebar, this means that we were constantly
re-adding the plugin links to the sidebar, causing duplication.
This can be fixed by just adding @cached to the getter for
sections.
This commit is contained in:
Martin Brennan 2024-02-21 12:58:31 +10:00 committed by GitHub
parent 533b91dec6
commit 95014e9ab8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 21 additions and 3 deletions

View File

@ -1,3 +1,4 @@
import { cached } from "@glimmer/tracking";
import PreloadStore from "discourse/lib/preload-store"; import PreloadStore from "discourse/lib/preload-store";
import { ADMIN_NAV_MAP } from "discourse/lib/sidebar/admin-nav-map"; import { ADMIN_NAV_MAP } from "discourse/lib/sidebar/admin-nav-map";
import BaseCustomSidebarPanel from "discourse/lib/sidebar/base-custom-sidebar-panel"; import BaseCustomSidebarPanel from "discourse/lib/sidebar/base-custom-sidebar-panel";
@ -213,6 +214,7 @@ export default class AdminSidebarPanel extends BaseCustomSidebarPanel {
key = ADMIN_PANEL; key = ADMIN_PANEL;
hidden = true; hidden = true;
@cached
get sections() { get sections() {
const currentUser = getOwnerWithFallback().lookup("service:current-user"); const currentUser = getOwnerWithFallback().lookup("service:current-user");
const siteSettings = getOwnerWithFallback().lookup("service:site-settings"); const siteSettings = getOwnerWithFallback().lookup("service:site-settings");

View File

@ -14,4 +14,12 @@ describe "Admin Revamp | Sidebar Navigation | Plugin Links", type: :system do
visit("/admin") visit("/admin")
expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat") expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat")
end end
it "does not duplicate links to enabled plugin admin routes when showing and hiding sidebar" do
visit("/admin")
expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat", count: 1)
find(".header-sidebar-toggle").click
find(".header-sidebar-toggle").click
expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat", count: 1)
end
end end

View File

@ -36,8 +36,15 @@ module PageObjects
has_css?(".#{SIDEBAR_SECTION_LINK_SELECTOR}--active", count: 1) has_css?(".#{SIDEBAR_SECTION_LINK_SELECTOR}--active", count: 1)
end end
def has_section_link?(name, href: nil, active: false, target: nil) def has_section_link?(name, href: nil, active: false, target: nil, count: 1)
section_link_present?(name, href: href, active: active, target: target, present: true) section_link_present?(
name,
href: href,
active: active,
target: target,
present: true,
count: count,
)
end end
def has_no_section_link?(name, href: nil, active: false) def has_no_section_link?(name, href: nil, active: false)
@ -163,12 +170,13 @@ module PageObjects
private private
def section_link_present?(name, href: nil, active: false, target: nil, present:) def section_link_present?(name, href: nil, active: false, target: nil, count: 1, present:)
attributes = { exact_text: name } attributes = { exact_text: name }
attributes[:href] = href if href attributes[:href] = href if href
attributes[:class] = SIDEBAR_SECTION_LINK_SELECTOR attributes[:class] = SIDEBAR_SECTION_LINK_SELECTOR
attributes[:class] += "--active" if active attributes[:class] += "--active" if active
attributes[:target] = target if target attributes[:target] = target if target
attributes[:count] = count
page.public_send(present ? :has_link? : :has_no_link?, **attributes) page.public_send(present ? :has_link? : :has_no_link?, **attributes)
end end