From b88232c4e82147ac1bd07db84fb21fc10d866e67 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:52:52 +0200 Subject: [PATCH] fix: list a plugin with both a settings page and a settings section once, with the section on its page --- .../__tests__/settings-plugin-page.test.tsx | 23 ++++++++++ components/settings/settings-app.tsx | 43 ++++++++++++++----- lib/plugin-slot-catalog.ts | 2 +- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/components/settings/__tests__/settings-plugin-page.test.tsx b/components/settings/__tests__/settings-plugin-page.test.tsx index 40887a75e..622628fe3 100644 --- a/components/settings/__tests__/settings-plugin-page.test.tsx +++ b/components/settings/__tests__/settings-plugin-page.test.tsx @@ -94,6 +94,29 @@ describe('Settings plugin pages', () => { expect(opts.plugin.id).toBe('kanban'); }); + it('gives a plugin with both a page and a section one entry, with the section on its page', async () => { + activate('showcase', 'Showcase', ['settings-section', 'settings-page']); + const { container } = render(); + + const entries = screen.getAllByRole('button', { name: /Showcase/ }); + expect(entries).toHaveLength(1); + + act(() => { fireEvent.click(entries[0]); }); + await waitFor(() => expect(createSlotInstance).toHaveBeenCalledTimes(2)); + const slots = createSlotInstance.mock.calls.map((call) => (call[0] as { slot: string }).slot); + expect(slots.sort()).toEqual(['settings-page', 'settings-section']); + const section = container.querySelector('[data-plugin-slot="settings-section"]')!; + const page = container.querySelector('[data-plugin-slot="settings-page"]')!; + expect(section.compareDocumentPosition(page) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + }); + + it('opens the page of such a plugin from an old link to its section', async () => { + activate('showcase', 'Showcase', ['settings-section', 'settings-page']); + const { container } = render(); + + await waitFor(() => expect(container.querySelector('[data-plugin-slot="settings-page"]')).not.toBeNull()); + }); + it('opens the page from a /settings/plugin-page: link', async () => { activate('kanban', 'Kanban', ['settings-page']); render(); diff --git a/components/settings/settings-app.tsx b/components/settings/settings-app.tsx index 42c26226e..be232a5e9 100644 --- a/components/settings/settings-app.tsx +++ b/components/settings/settings-app.tsx @@ -254,6 +254,8 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps = () => pluginOffersForSlot('settings-page'), ); const tPluginPages = useTranslations('plugin_pages'); + const pluginPageIds = useMemo(() => new Set(pluginPageOffers.map((offer) => offer.pluginId)), [pluginPageOffers]); + const pluginSectionIds = useMemo(() => new Set(pluginSettingsOffers.map((offer) => offer.pluginId)), [pluginSettingsOffers]); // Consume the one-shot deep-link key so a section gear only steers this one // open, never the persisted default for future Settings-button clicks. useEffect(() => { @@ -584,7 +586,9 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps = ...(isFeatureEnabled('sidebarAppsEnabled') ? [{ id: 'sidebar_apps' as Tab, label: t('tabs.sidebar_apps'), icon: tabIcons.sidebar_apps, group: 'apps' as TabGroup }] : []), // Plugin-contributed settings pages: one entry per active plugin that // offers a `settings-section` slot (e.g. S/MIME key & certificate manager). - ...pluginSettingsOffers.map((offer): TabDef => ({ + // A plugin that also has a page of its own gets that entry only, with the + // section on its page, instead of two entries with the same name. + ...pluginSettingsOffers.filter((offer) => !pluginPageIds.has(offer.pluginId)).map((offer): TabDef => ({ id: `plugin:${offer.pluginId}` as PluginTabId, label: getActivePlugin(offer.pluginId)?.plugin.name ?? offer.pluginId, icon: Puzzle, @@ -657,9 +661,15 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps = // If active tab is not in the visible list (e.g., feature disabled, or scoped // mode hides it), fall back. In scoped mode fall back to the first scoped tab; // otherwise the usual 'appearance' default. - const isActiveVisible = visibleTabs.some((tab) => tab.id === activeTab); + // An old link or remembered tab to the section of a plugin that now has a + // page of its own opens that page, where the section is. + const sectionPluginId = activeTab.startsWith('plugin:') ? activeTab.slice('plugin:'.length) : null; + const requestedTab: SettingsTabId = sectionPluginId && pluginPageIds.has(sectionPluginId) + ? (`${PLUGIN_PAGE_PREFIX}${sectionPluginId}` as PluginPageTabId) + : activeTab; + const isActiveVisible = visibleTabs.some((tab) => tab.id === requestedTab); const effectiveActiveTab: SettingsTabId = isActiveVisible - ? activeTab + ? requestedTab : (managedAccountId ? (visibleTabs[0]?.id ?? 'appearance') : 'appearance'); const handleTabSelect = (tabId: SettingsTabId) => { @@ -736,13 +746,26 @@ export function SettingsApp({ linkSegments: routeSegments }: SettingsAppProps = /> )} {effectiveActiveTab.startsWith(PLUGIN_PAGE_PREFIX) && ( -
- -
+ <> + {/* The plugin's settings-section, on its page rather than as a + second entry with the same name. */} + {pluginSectionIds.has(effectiveActiveTab.slice(PLUGIN_PAGE_PREFIX.length)) && ( +
+ +
+ )} +
+ +
+ )} ); diff --git a/lib/plugin-slot-catalog.ts b/lib/plugin-slot-catalog.ts index f6bece96c..f6e307bdf 100644 --- a/lib/plugin-slot-catalog.ts +++ b/lib/plugin-slot-catalog.ts @@ -106,7 +106,7 @@ export const SLOT_CATALOG = { 'file-preview-section': S('files', 'frame', ['ui:files'], { file: 'files:read' }, 'a section in the file preview'), // ─── Settings and admin ──────────────────────────────────── - 'settings-section': S('settings', 'frame', ['ui:settings-section'], {}, 'a section on the plugin\'s own page in Settings'), + 'settings-section': S('settings', 'frame', ['ui:settings-section'], {}, 'a section on the plugin\'s own page in Settings (above its settings-page, when it has one: one entry per plugin)'), 'settings-page': S('settings', 'frame', ['ui:settings-section'], {}, 'a page of its own in Settings, listed under the plugin\'s name'), 'admin-plugin-page': S('admin', 'frame', ['ui:admin-page', 'admin:config'], {}, 'the plugin\'s page in the admin dashboard'), } as const satisfies Record;