From e70d7397dddff520d535a1b910f8902412935e96 Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 30 Aug 2026 19:27:01 +0200 Subject: [PATCH] fix(admin): let the plugin consent, signature and dependency dialogs scroll --- .../Admin/AdminPluginsPanel.test.tsx | 15 +- .../components/Admin/AdminPluginsPanel.tsx | 24 +-- .../screens/admin/MAdminPluginsPanel.tsx | 160 +++++++++--------- 3 files changed, 106 insertions(+), 93 deletions(-) diff --git a/client/src/components/Admin/AdminPluginsPanel.test.tsx b/client/src/components/Admin/AdminPluginsPanel.test.tsx index 693925b0c..a414c8db7 100644 --- a/client/src/components/Admin/AdminPluginsPanel.test.tsx +++ b/client/src/components/Admin/AdminPluginsPanel.test.tsx @@ -281,6 +281,10 @@ describe('AdminPluginsPanel — a refused update', () => { expect(await screen.findByText(/key it was installed with/i)).toBeInTheDocument() expect(screen.getByText(/key it is offering now/i)).toBeInTheDocument() expect(screen.getByRole('button', { name: /trust the new key/i })).toBeInTheDocument() + // The dialog body scrolls; the decision buttons sit outside it, so they never leave the viewport (#2159). + const body = screen.getByText(/key it is offering now/i).closest('.overflow-y-auto') + expect(body).not.toBeNull() + expect(body).not.toContainElement(screen.getByRole('button', { name: /trust the new key/i })) }) // D2, at the UI. An invalid signature means the bytes are not what the author signed. @@ -947,7 +951,11 @@ describe('AdminPluginsPanel — enabling a plugin', () => { await screen.findByText('Gotify') fireEvent.click(rowToggle('Gotify')) - fireEvent.click(await screen.findByRole('button', { name: /^cancel$/i })) + // The dependency list scrolls; Cancel sits outside it, so it never leaves the viewport (#2159). + const body = (await screen.findByText('trek-core')).closest('.overflow-y-auto') + expect(body).not.toBeNull() + expect(body).not.toContainElement(screen.getByRole('button', { name: /^cancel$/i })) + fireEvent.click(screen.getByRole('button', { name: /^cancel$/i })) await waitFor(() => expect(screen.queryByText('Missing dependencies')).not.toBeInTheDocument()) expect(installs).toBe(0) @@ -967,6 +975,11 @@ describe('AdminPluginsPanel — enabling a plugin', () => { // An unknown permission is still shown — as its raw code expect(screen.getByText('weird:perm')).toBeInTheDocument() expect(screen.getByText('api.acme.io')).toBeInTheDocument() + // A long permission list scrolls inside the dialog; the decision buttons sit outside + // the scroll container, so they never leave the viewport (#2159). + const body = screen.getByText('Read trips the acting user can access').closest('.overflow-y-auto') + expect(body).not.toBeNull() + expect(body).not.toContainElement(screen.getByRole('button', { name: /keep off for now/i })) fireEvent.click(screen.getByRole('button', { name: /keep off for now/i })) expect(await screen.findByText(/left off until you approve/i)).toBeInTheDocument() diff --git a/client/src/components/Admin/AdminPluginsPanel.tsx b/client/src/components/Admin/AdminPluginsPanel.tsx index d7a195d0e..d96122894 100644 --- a/client/src/components/Admin/AdminPluginsPanel.tsx +++ b/client/src/components/Admin/AdminPluginsPanel.tsx @@ -1793,15 +1793,15 @@ function SignatureBlockDialog({ data, entry, busy, t, onRetrust, onClose }: { return (
-
e.stopPropagation()}> -
+
e.stopPropagation()}> +

{t('admin.plugins.sig.title', { name: data.subject.name })}

{t(bodyKey as never)}

-
+
{/* Fingerprints, not full keys: these exist to be COMPARED by a human — read the new one back to the author over the phone. The full key travels in the request. */} {canRetrust && ( @@ -1815,7 +1815,7 @@ function SignatureBlockDialog({ data, entry, busy, t, onRetrust, onClose }: {

{data.detail}

)}
-
+
@@ -1917,15 +1917,15 @@ function DependencyResolveDialog({ data, t, busy, installedIds, onDownload, onCl ] return (
-
e.stopPropagation()}> -
+
e.stopPropagation()}> +

{t('admin.plugins.dep.resolveTitle')}

{t('admin.plugins.dep.resolveBody', { name: data.plugin.name })}

-
+
{rows.map(r => (
@@ -1946,7 +1946,7 @@ function DependencyResolveDialog({ data, t, busy, installedIds, onDownload, onCl

{t('admin.plugins.dep.resolveHint')}

)}
-
+
diff --git a/client/src/mobile/screens/admin/MAdminPluginsPanel.tsx b/client/src/mobile/screens/admin/MAdminPluginsPanel.tsx index a1368fb86..756183c83 100644 --- a/client/src/mobile/screens/admin/MAdminPluginsPanel.tsx +++ b/client/src/mobile/screens/admin/MAdminPluginsPanel.tsx @@ -1784,7 +1784,7 @@ function SignatureBlockSheet({ data, entry, busy, t, onRetrust, onClose }: { return ( -
+
@@ -1792,30 +1792,30 @@ function SignatureBlockSheet({ data, entry, busy, t, onRetrust, onClose }: {

{t(bodyKey as never)}

-
- {/* Fingerprints, not full keys: these exist to be COMPARED by a human — read the - new one back to the author over the phone. The full key travels in the request. */} - {canRetrust && ( -
- - -

{t('admin.plugins.sig.confirmOutOfBand')}

-
- )} - {!canRetrust && data.detail && ( -

{data.detail}

- )} -
-
- - {offerRetrust ? t('admin.plugins.sig.cancel') : t('common.close')} +
+
+ {/* Fingerprints, not full keys: these exist to be COMPARED by a human — read the + new one back to the author over the phone. The full key travels in the request. */} + {canRetrust && ( +
+ + +

{t('admin.plugins.sig.confirmOutOfBand')}

+
+ )} + {!canRetrust && data.detail && ( +

{data.detail}

+ )} +
+
+ + {offerRetrust ? t('admin.plugins.sig.cancel') : t('common.close')} + + {offerRetrust && ( + onRetrust(version, newKey)}> + {t('admin.plugins.sig.retrustConfirm')} - {offerRetrust && ( - onRetrust(version, newKey)}> - {t('admin.plugins.sig.retrustConfirm')} - - )} -
+ )}
) @@ -1846,7 +1846,7 @@ function UpdateConsentSheet({ data, unsigned, t, onApprove, onLater }: { }) { return ( -
+
@@ -1854,39 +1854,39 @@ function UpdateConsentSheet({ data, unsigned, t, onApprove, onLater }: {

{t('admin.plugins.updateConsentBody', { name: data.plugin.name, version: data.version })}

-
- {/* The admin is about to widen what this code may do — so say, right here, that - nothing ties this code to its author. One line, no checkbox, no extra click: - this informs, it does not block. */} - {unsigned && ( -

- - {t('admin.plugins.sig.consentUnsigned')} -

- )} - {data.newPermissions.length > 0 && ( -
-

{t('admin.plugins.updateNewPermissions')}

-
    - {data.newPermissions.map(perm => ( -
    • - ))} -
+
+
+ {/* The admin is about to widen what this code may do — so say, right here, that + nothing ties this code to its author. One line, no checkbox, no extra click: + this informs, it does not block. */} + {unsigned && ( +

+ + {t('admin.plugins.sig.consentUnsigned')} +

+ )} + {data.newPermissions.length > 0 && ( +
+

{t('admin.plugins.updateNewPermissions')}

+
    + {data.newPermissions.map(perm => ( +
    • + ))} +
+
+ )} + {data.newEgress.length > 0 && ( +
+

{t('admin.plugins.updateNewEgress')}

+
+ {data.newEgress.map(host => {host})}
- )} - {data.newEgress.length > 0 && ( -
-

{t('admin.plugins.updateNewEgress')}

-
- {data.newEgress.map(host => {host})} -
-
- )} -
-
- {t('admin.plugins.updateLater')} - {t('admin.plugins.updateApprove')} -
+
+ )} +
+
+ {t('admin.plugins.updateLater')} + {t('admin.plugins.updateApprove')}
) @@ -1906,7 +1906,7 @@ function DependencyResolveSheet({ data, t, busy, installedIds, onDownload, onClo ] return ( -
+
@@ -1914,30 +1914,30 @@ function DependencyResolveSheet({ data, t, busy, installedIds, onDownload, onClo

{t('admin.plugins.dep.resolveBody', { name: data.plugin.name })}

-
- {rows.map(r => ( -
-
-
{r.id}
-
- {r.installed - ? t('admin.plugins.dep.mismatch', { wanted: r.constraint, installed: r.installed }) - : t('admin.plugins.dep.requires', { version: r.constraint })} -
+
+
+ {rows.map(r => ( +
+
+
{r.id}
+
+ {r.installed + ? t('admin.plugins.dep.mismatch', { wanted: r.constraint, installed: r.installed }) + : t('admin.plugins.dep.requires', { version: r.constraint })}
-
- ))} - {rows.some(r => !installedIds.has(r.id)) && ( -

{t('admin.plugins.dep.resolveHint')}

- )} -
-
- {t('common.cancel')} -
+ +
+ ))} + {rows.some(r => !installedIds.has(r.id)) && ( +

{t('admin.plugins.dep.resolveHint')}

+ )} +
+
+ {t('common.cancel')}
)