diff --git a/src/ui/AppShell.tsx b/src/ui/AppShell.tsx index 97db683dd..2bf556fc6 100644 --- a/src/ui/AppShell.tsx +++ b/src/ui/AppShell.tsx @@ -1686,6 +1686,10 @@ export function AppShell({ tabLabel, ); } + // Asking for a page again still goes there after the user moved away + if (type === "settings" && data?.section && data.revealAt === undefined) { + data = { ...data, revealAt: Date.now() }; + } const id = type; const singletonLabels: Partial> = { "host-manager": t("nav.manage"), diff --git a/src/ui/plugins/PluginDetail.tsx b/src/ui/plugins/PluginDetail.tsx index 5c88f3d07..dcc5606ba 100644 --- a/src/ui/plugins/PluginDetail.tsx +++ b/src/ui/plugins/PluginDetail.tsx @@ -37,6 +37,7 @@ import { type PluginEntry, } from "./plugin-model"; import { openPluginSettings } from "./open-plugins"; +import { hasSettingsPage } from "@/settings/settings-fields-util"; import { PluginReleaseNotes, PluginVideo } from "./PluginReleaseNotes"; import type { PluginsManager } from "./use-plugins-manager"; @@ -67,11 +68,10 @@ export function PluginDetail({ }, [plugin.version, plugin.latestVersion]); const contributions = describeContributions(plugin.contributes); - const settings = plugin.contributes?.settings as - { admin?: unknown[]; user?: unknown[] } | undefined; + // Only people who can manage plugins get here, and they see admin settings const hasSettings = plugin.installed && - ((settings?.admin?.length ?? 0) > 0 || (settings?.user?.length ?? 0) > 0); + hasSettingsPage(plugin as Parameters[0], true); const issueUrl = reportIssueUrl( plugin, import.meta.env.VITE_APP_VERSION || undefined, diff --git a/src/ui/settings/SettingsTab.tsx b/src/ui/settings/SettingsTab.tsx index bc74318ba..1109b2ce0 100644 --- a/src/ui/settings/SettingsTab.tsx +++ b/src/ui/settings/SettingsTab.tsx @@ -43,6 +43,8 @@ import { export interface SettingsTabProps { /** The page to open, from the tab's data. */ section?: string; + /** Changes on every open request, so asking for the same page again still goes there. */ + sectionAt?: string | number; /** An element id on that page to scroll to, as "id@nonce". */ reveal?: string; username: string; @@ -86,6 +88,7 @@ function revealText(container: HTMLElement, needle: string): boolean { /** Everything that configures the app, one page at a time. */ export function SettingsTab({ section, + sectionAt, reveal, username, isAdmin, @@ -141,7 +144,7 @@ export function SettingsTab({ const [current, setCurrent] = useState(section ?? "account"); useEffect(() => { if (section) setCurrent(section); - }, [section]); + }, [section, sectionAt]); // The page renders lazily, so the target may take a moment to appear. useEffect(() => { diff --git a/src/ui/settings/settings-fields-util.ts b/src/ui/settings/settings-fields-util.ts index 78fe9fce4..9f2a2ff89 100644 --- a/src/ui/settings/settings-fields-util.ts +++ b/src/ui/settings/settings-fields-util.ts @@ -19,3 +19,24 @@ export function isFieldShown( export function hasVisibleFields(fields: PluginSettingsField[]): boolean { return fields.some((field) => !field.hidden); } + +/** Whether Settings lists a page for this plugin, by the same rule it uses. */ +export function hasSettingsPage( + plugin: { + enabled: boolean; + contributes?: { + settings?: { + user?: PluginSettingsField[]; + admin?: PluginSettingsField[]; + }; + } | null; + }, + isAdmin: boolean, +): boolean { + const settings = plugin.contributes?.settings; + return ( + plugin.enabled && + (hasVisibleFields(settings?.user ?? []) || + (isAdmin && hasVisibleFields(settings?.admin ?? []))) + ); +} diff --git a/src/ui/shell/tabUtils.tsx b/src/ui/shell/tabUtils.tsx index 8e685f054..0ba1d3982 100644 --- a/src/ui/shell/tabUtils.tsx +++ b/src/ui/shell/tabUtils.tsx @@ -227,7 +227,7 @@ function noop() {} export interface TabRenderContext { shell: TabShellCallbacks; /** What the Settings tab needs from the shell. */ - settings?: Omit; + settings?: Omit; /** The terminal a panel shown as a tab sends commands to. */ panelTargetTab?: Tab; isVisible?: boolean; @@ -266,6 +266,12 @@ export function renderTabContent(tab: Tab, context: TabRenderContext) { ? tab.data.section : undefined } + sectionAt={ + typeof tab.data?.revealAt === "number" || + typeof tab.data?.revealAt === "string" + ? tab.data.revealAt + : undefined + } reveal={ typeof tab.data?.reveal === "string" ? `${tab.data.reveal}@${String(tab.data.revealAt ?? "")}` diff --git a/src/ui/tests/settings/settings-fields-util.test.ts b/src/ui/tests/settings/settings-fields-util.test.ts new file mode 100644 index 000000000..b4e4cfd8b --- /dev/null +++ b/src/ui/tests/settings/settings-fields-util.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; +import { + hasSettingsPage, + hasVisibleFields, +} from "@/settings/settings-fields-util"; +import type { PluginSettingsField } from "@/api/plugins-api"; + +const field = (hidden = false) => + ({ key: "a", type: "string", hidden }) as unknown as PluginSettingsField; + +describe("hasVisibleFields", () => { + it("ignores hidden fields", () => { + expect(hasVisibleFields([field(true)])).toBe(false); + expect(hasVisibleFields([field(true), field()])).toBe(true); + }); +}); + +describe("hasSettingsPage", () => { + it("needs an enabled plugin", () => { + expect( + hasSettingsPage( + { enabled: false, contributes: { settings: { user: [field()] } } }, + true, + ), + ).toBe(false); + }); + + it("is false with no settings or only hidden ones", () => { + expect(hasSettingsPage({ enabled: true, contributes: null }, true)).toBe( + false, + ); + expect( + hasSettingsPage( + { enabled: true, contributes: { settings: { user: [field(true)] } } }, + true, + ), + ).toBe(false); + }); + + it("counts admin settings only for admins", () => { + const plugin = { + enabled: true, + contributes: { settings: { admin: [field()] } }, + }; + expect(hasSettingsPage(plugin, true)).toBe(true); + expect(hasSettingsPage(plugin, false)).toBe(false); + }); + + it("counts user settings for anyone", () => { + expect( + hasSettingsPage( + { enabled: true, contributes: { settings: { user: [field()] } } }, + false, + ), + ).toBe(true); + }); +});