FEATURE: More flexible admin plugin config nav definition (#26254)

This commit changes the API for registering the plugin config
page nav configuration from a server-side to a JS one;
there is no need for it to be server-side.

It also makes some changes to allow for 2 different ways of displaying
navigation for plugin pages, depending on complexity:

* TOP - This is the best mode for simple plugins without a lot of different
  custom configuration pages, and it reuses the grey horizontal nav bar
  already used for admins.
* SIDEBAR - This is better for more complex plugins; likely this won't
  be used in the near future, but it's readily available if needed

There is a new AdminPluginConfigNavManager service too to manage which
plugin the admin is actively viewing, otherwise we would have trouble
hiding the main plugin nav for admins when viewing a single plugin.
This commit is contained in:
Martin Brennan 2024-03-21 13:42:06 +10:00 committed by GitHub
parent e5566b8519
commit 70f7c0ee6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 268 additions and 120 deletions

View File

@ -1,9 +1,12 @@
import Component from "@glimmer/component";
import { LinkTo } from "@ember/routing";
import { inject as service } from "@ember/service";
import concatClass from "discourse/helpers/concat-class";
import I18n from "discourse-i18n";
export default class AdminPluginConfigArea extends Component {
@service adminPluginNavManager;
linkText(navLink) {
if (navLink.label) {
return I18n.t(navLink.label);
@ -13,10 +16,13 @@ export default class AdminPluginConfigArea extends Component {
}
<template>
{{#if @innerSidebarNavLinks}}
{{#if this.adminPluginNavManager.isSidebarMode}}
<nav class="admin-nav admin-plugin-inner-sidebar-nav pull-left">
<ul class="nav nav-stacked">
{{#each @innerSidebarNavLinks as |navLink|}}
{{#each
this.adminPluginNavManager.currentConfigNav.links
as |navLink|
}}
<li
class={{concatClass
"admin-plugin-inner-sidebar-nav__item"

View File

@ -1,19 +1,18 @@
import Component from "@glimmer/component";
import { inject as service } from "@ember/service";
import HorizontalOverflowNav from "discourse/components/horizontal-overflow-nav";
import NavItem from "discourse/components/nav-item";
import i18n from "discourse-common/helpers/i18n";
import AdminPluginConfigArea from "./admin-plugin-config-area";
export default class extends Component {
@service currentUser;
get configNavRoutes() {
return this.args.plugin.configNavRoutes || [];
}
@service adminPluginNavManager;
get mainAreaClasses() {
let classes = ["admin-plugin-config-page__main-area"];
if (this.configNavRoutes.length) {
if (this.adminPluginNavManager.isSidebarMode) {
classes.push("-with-inner-sidebar");
} else {
classes.push("-without-inner-sidebar");
@ -22,32 +21,61 @@ export default class extends Component {
return classes.join(" ");
}
linkText(navLink) {
if (navLink.label) {
return i18n(navLink.label);
} else {
return navLink.text;
}
}
<template>
<div class="admin-plugin-config-page">
<div class="admin-plugin-config-page__metadata">
<h2>
{{@plugin.nameTitleized}}
</h2>
<p>
{{@plugin.about}}
{{#if @plugin.linkUrl}}
|
<a
href={{@plugin.linkUrl}}
rel="noopener noreferrer"
target="_blank"
>
{{i18n "admin.plugins.learn_more"}}
</a>
{{/if}}
{{#if this.adminPluginNavManager.isTopMode}}
<div class="admin-controls">
<HorizontalOverflowNav
class="nav-pills action-list main-nav nav plugin-nav"
>
{{#each
this.adminPluginNavManager.currentConfigNav.links
as |navLink|
}}
<NavItem
@route={{navLink.route}}
@i18nLabel={{this.linkText navLink}}
title={{this.linkText navLink}}
class="admin-plugin-config-page__top-nav-item"
>
{{this.linkText navLink}}
</NavItem>
{{/each}}
</HorizontalOverflowNav>
</div>
{{/if}}
</p>
<div class="admin-plugin-config-page__metadata">
<div class="admin-plugin-config-area__metadata-title">
<h2>
{{@plugin.nameTitleized}}
</h2>
<p>
{{@plugin.about}}
{{#if @plugin.linkUrl}}
|
<a
href={{@plugin.linkUrl}}
rel="noopener noreferrer"
target="_blank"
>
{{i18n "admin.plugins.learn_more"}}
</a>
{{/if}}
</p>
</div>
</div>
<div class="admin-plugin-config-page__content">
<div class={{this.mainAreaClasses}}>
<AdminPluginConfigArea
@innerSidebarNavLinks={{@plugin.configNavRoutes}}
>
<AdminPluginConfigArea>
{{yield}}
</AdminPluginConfigArea>
</div>

View File

@ -2,6 +2,7 @@ import Controller from "@ember/controller";
import { service } from "@ember/service";
export default class AdminPluginsController extends Controller {
@service adminPluginNavManager;
@service router;
get adminRoutes() {
@ -21,6 +22,13 @@ export default class AdminPluginsController extends Controller {
.filter(Boolean);
}
get showTopNav() {
return (
!this.adminPluginNavManager.currentPlugin ||
this.adminPluginNavManager.isSidebarMode
);
}
routeExists(route) {
try {
if (route.use_new_show_route) {

View File

@ -26,7 +26,6 @@ export default class AdminPlugin {
this.version = args.version;
this.metaUrl = args.meta_url;
this.authors = args.authors;
this.configNavRoutes = args.admin_config_nav_routes;
}
get snakeCaseName() {

View File

@ -6,6 +6,7 @@ import AdminPlugin from "admin/models/admin-plugin";
export default class AdminPluginsShowRoute extends Route {
@service router;
@service adminPluginNavManager;
model(params) {
const pluginId = sanitize(params.plugin_id).substring(0, 100);
@ -13,4 +14,12 @@ export default class AdminPluginsShowRoute extends Route {
return AdminPlugin.create(plugin);
});
}
afterModel(model) {
this.adminPluginNavManager.currentPlugin = model;
}
deactivate() {
this.adminPluginNavManager.currentPlugin = null;
}
}

View File

@ -0,0 +1,28 @@
import { tracked } from "@glimmer/tracking";
import Service, { service } from "@ember/service";
import {
configNavForPlugin,
PLUGIN_NAV_MODE_SIDEBAR,
PLUGIN_NAV_MODE_TOP,
} from "discourse/lib/admin-plugin-config-nav";
export default class AdminPluginNavManager extends Service {
@service currentUser;
@tracked currentPlugin;
get currentUserUsingAdminSidebar() {
return this.currentUser?.use_admin_sidebar;
}
get currentConfigNav() {
return configNavForPlugin(this.currentPlugin.id);
}
get isSidebarMode() {
return this.currentConfigNav.mode === PLUGIN_NAV_MODE_SIDEBAR;
}
get isTopMode() {
return this.currentConfigNav.mode === PLUGIN_NAV_MODE_TOP;
}
}

View File

@ -1,20 +1,22 @@
{{#if this.model.length}}
<h3>{{i18n "admin.plugins.installed"}}</h3>
<AdminPluginsList @plugins={{this.model}} />
{{else}}
<p>{{i18n "admin.plugins.none_installed"}}</p>
{{/if}}
<div class="admin-plugins-list-container">
{{#if this.model.length}}
<h2>{{i18n "admin.plugins.installed"}}</h2>
<AdminPluginsList @plugins={{this.model}} />
{{else}}
<p>{{i18n "admin.plugins.none_installed"}}</p>
{{/if}}
<p class="admin-plugins-howto">
<a href="https://meta.discourse.org/t/install-a-plugin/19157">
{{i18n "admin.plugins.howto"}}
</a>
</p>
<p class="admin-plugins-howto">
<a href="https://meta.discourse.org/t/install-a-plugin/19157">
{{i18n "admin.plugins.howto"}}
</a>
</p>
<span>
<PluginOutlet
@name="admin-below-plugins-index"
@connectorTagName="div"
@outletArgs={{hash model=this.model}}
/>
</span>
<span>
<PluginOutlet
@name="admin-below-plugins-index"
@connectorTagName="div"
@outletArgs={{hash model=this.model}}
/>
</span>
</div>

View File

@ -1,19 +1,21 @@
<div class="admin-controls">
<HorizontalOverflowNav class="main-nav nav plugin-nav">
<NavItem @route="adminPlugins.index" @label="admin.plugins.title" />
{{#each this.adminRoutes as |route|}}
{{#if route.use_new_show_route}}
<NavItem
@route={{route.full_location}}
@label={{route.label}}
@routeParam={{route.location}}
/>
{{else}}
<NavItem @route={{route.full_location}} @label={{route.label}} />
{{/if}}
{{/each}}
</HorizontalOverflowNav>
</div>
{{#if this.showTopNav}}
<div class="admin-controls">
<HorizontalOverflowNav class="main-nav nav plugin-nav">
<NavItem @route="adminPlugins.index" @label="admin.plugins.title" />
{{#each this.adminRoutes as |route|}}
{{#if route.use_new_show_route}}
<NavItem
@route={{route.full_location}}
@label={{route.label}}
@routeParam={{route.location}}
/>
{{else}}
<NavItem @route={{route.full_location}} @label={{route.label}} />
{{/if}}
{{/each}}
</HorizontalOverflowNav>
</div>
{{/if}}
<div class="admin-container">
{{#each this.brokenAdminRoutes as |route|}}

View File

@ -0,0 +1,12 @@
export const PLUGIN_NAV_MODE_SIDEBAR = "sidebar";
export const PLUGIN_NAV_MODE_TOP = "top";
let pluginConfigNav = {};
export function registerAdminPluginConfigNav(pluginId, mode, links) {
pluginConfigNav[pluginId] = { mode, links };
}
export function resetAdminPluginConfigNav() {
pluginConfigNav = {};
}
export function configNavForPlugin(pluginId) {
return pluginConfigNav[pluginId];
}

View File

@ -49,6 +49,11 @@ import {
import { addUsernameSelectorDecorator } from "discourse/helpers/decorate-username-selector";
import { registerCustomAvatarHelper } from "discourse/helpers/user-avatar";
import { addBeforeAuthCompleteCallback } from "discourse/instance-initializers/auth-complete";
import {
PLUGIN_NAV_MODE_SIDEBAR,
PLUGIN_NAV_MODE_TOP,
registerAdminPluginConfigNav,
} from "discourse/lib/admin-plugin-config-nav";
import { addPopupMenuOption } from "discourse/lib/composer/custom-popup-menu-options";
import { registerDesktopNotificationHandler } from "discourse/lib/desktop-notifications";
import { downloadCalendar } from "discourse/lib/download-calendar";
@ -2914,6 +2919,36 @@ class PluginApi {
addImageWrapperButton(label, btnClass, icon);
addApiImageWrapperButtonClickEvent(fn);
}
/**
* Defines a list of links used in the adminPlugins.show page for
* a specific plugin. Each link must have:
*
* * route
* * label OR text
*
* And the mode must be one of "sidebar" or "top", which controls
* where in the admin plugin show UI the links will be displayed.
*/
addAdminPluginConfigurationNav(pluginId, mode, links) {
if (!pluginId) {
// eslint-disable-next-line no-console
console.warn(consolePrefix(), "A pluginId must be provided!");
return;
}
const validModes = [PLUGIN_NAV_MODE_SIDEBAR, PLUGIN_NAV_MODE_TOP];
if (!validModes.includes(mode)) {
// eslint-disable-next-line no-console
console.warn(
consolePrefix(),
`${mode} is an invalid mode for admin plugin config pages, only ${validModes} are usable.`
);
return;
}
registerAdminPluginConfigNav(pluginId, mode, links);
}
}
// from http://stackoverflow.com/questions/6832596/how-to-compare-software-version-number-using-js-only-number

View File

@ -32,6 +32,7 @@ import { resetCustomPostMessageCallbacks } from "discourse/controllers/topic";
import { clearHTMLCache } from "discourse/helpers/custom-html";
import { resetUsernameDecorators } from "discourse/helpers/decorate-username-selector";
import { resetBeforeAuthCompleteCallbacks } from "discourse/instance-initializers/auth-complete";
import { resetAdminPluginConfigNav } from "discourse/lib/admin-plugin-config-nav";
import { clearPopupMenuOptions } from "discourse/lib/composer/custom-popup-menu-options";
import { clearDesktopNotificationHandlers } from "discourse/lib/desktop-notifications";
import { cleanUpHashtagTypeClasses } from "discourse/lib/hashtag-type-registry";
@ -244,6 +245,7 @@ export function testCleanup(container, app) {
resetBeforeAuthCompleteCallbacks();
clearPopupMenuOptions();
clearAdditionalAdminSidebarSectionLinks();
resetAdminPluginConfigNav();
}
function cleanupCssGeneratorTags() {

View File

@ -1,25 +1,38 @@
import { getOwner } from "@ember/application";
import { render } from "@ember/test-helpers";
import { hbs } from "ember-cli-htmlbars";
import { module, test } from "qunit";
import {
PLUGIN_NAV_MODE_SIDEBAR,
PLUGIN_NAV_MODE_TOP,
registerAdminPluginConfigNav,
} from "discourse/lib/admin-plugin-config-nav";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import AdminPlugin from "admin/models/admin-plugin";
module("Integration | Component | admin-plugin-config-area", function (hooks) {
setupRenderingTest(hooks);
test("it renders the plugin config nav and content", async function (assert) {
this.set("innerSidebarNavLinks", [
{
route: "adminPlugins.show.discourse-test-plugin.one",
label: "admin.title",
},
{
route: "adminPlugins.show.discourse-test-plugin.two",
label: "admin.back_to_forum",
},
]);
test("it renders the plugin config nav and content in the sidebar mode", async function (assert) {
registerAdminPluginConfigNav(
"discourse-test-plugin",
PLUGIN_NAV_MODE_SIDEBAR,
[
{
route: "adminPlugins.show.discourse-test-plugin.one",
label: "admin.title",
},
{
route: "adminPlugins.show.discourse-test-plugin.two",
label: "admin.back_to_forum",
},
]
);
getOwner(this).lookup("service:admin-plugin-nav-manager").currentPlugin =
new AdminPlugin({ id: "discourse-test-plugin" });
await render(hbs`
<AdminPluginConfigArea @innerSidebarNavLinks={{this.innerSidebarNavLinks}}>
<AdminPluginConfigArea>
Test content
</AdminPluginConfigArea>
`);
@ -36,4 +49,37 @@ module("Integration | Component | admin-plugin-config-area", function (hooks) {
"it renders the yielded content"
);
});
test("it does not render the nav items in the sidebar when using top mode", async function (assert) {
registerAdminPluginConfigNav("discourse-test-plugin", PLUGIN_NAV_MODE_TOP, [
{
route: "adminPlugins.show.discourse-test-plugin.one",
label: "admin.title",
},
{
route: "adminPlugins.show.discourse-test-plugin.two",
label: "admin.back_to_forum",
},
]);
getOwner(this).lookup("service:admin-plugin-nav-manager").currentPlugin =
new AdminPlugin({ id: "discourse-test-plugin" });
await render(hbs`
<AdminPluginConfigArea>
Test content
</AdminPluginConfigArea>
`);
assert.strictEqual(
document.querySelectorAll(".admin-plugin-inner-sidebar-nav__item").length,
0,
"it renders the correct number of nav items"
);
assert.strictEqual(
document.querySelector(".admin-plugin-config-area").textContent.trim(),
"Test content",
"it renders the yielded content"
);
});
});

View File

@ -84,6 +84,10 @@
}
.admin-plugin-config-page {
.admin-controls {
margin-bottom: 1em;
}
&__main-area {
.admin-detail {
padding-top: 15px;
@ -99,6 +103,14 @@
}
}
.admin-plugins .admin-container {
margin-top: 0;
}
.admin-plugins-list-container {
margin-top: 1em;
}
.admin-plugin-filtered-site-settings {
&__filter {
width: 100%;

View File

@ -16,12 +16,7 @@ class AdminPluginSerializer < ApplicationSerializer
:commit_hash,
:commit_url,
:meta_url,
:authors,
:admin_config_nav_routes
def admin_config_nav_routes
object.admin_config_nav_routes
end
:authors
def id
object.directory_name

View File

@ -7,6 +7,10 @@ in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [1.30.0] - 2024-03-20
- Added `addAdminPluginConfigurationNav`, which defines a list of links used in the adminPlugins.show page for a specific plugin, and displays them either in an inner sidebar or in a top horizontal nav.
## [1.29.0] - 2024-03-05
- added `headerButtons` which allows for manipulation of the header butttons. This includes, adding, removing, or modifying the order of buttons.

View File

@ -45,7 +45,6 @@ end
class Plugin::Instance
attr_accessor :path, :metadata
attr_reader :admin_route
attr_reader :admin_config_nav_routes
# Memoized array readers
%i[
@ -114,19 +113,6 @@ class Plugin::Instance
}
end
def register_admin_config_nav_routes(plugin_id, nav)
@admin_config_nav_routes =
nav.each do |n|
if !n.key?(:label) || !n.key?(:route)
raise ArgumentError.new(
"Admin config nav routes must have a `route` value that matches an Ember route and a `label` value that matches a client I18n key",
)
end
n[:model] = plugin_id
end
end
def configurable?
true
end

View File

@ -999,30 +999,4 @@ TEXT
expect(sum).to eq(3)
end
end
describe "#register_admin_config_nav_routes" do
let(:plugin) { Plugin::Instance.new }
it "adds the specified plugin id as the 'model' for the route" do
plugin.register_admin_config_nav_routes(
"discourse-awesome",
[{ route: "adminPlugins.show", label: "some.i18n.label" }],
)
expect(plugin.admin_config_nav_routes).to eq(
[{ route: "adminPlugins.show", label: "some.i18n.label", model: "discourse-awesome" }],
)
end
it "errors if the route or label is not provided" do
expect {
plugin.register_admin_config_nav_routes("discourse-awesome", [{ label: "some.i18n.label" }])
}.to raise_error(ArgumentError)
expect {
plugin.register_admin_config_nav_routes(
"discourse-awesome",
[{ route: "adminPlugins.show" }],
)
}.to raise_error(ArgumentError)
end
end
end