From fc087c13e87f7fd03ff660188549743fe73f7e6b Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Tue, 3 Nov 2026 08:19:32 +0000 Subject: [PATCH] fix(ui): style the host controls of an add-on like the Settings page The controls sit in a fieldset, which drew the browser border and padding around the rows. The rows also showed raw setting keys such as chats_egress_ceiling next to their labels. Reset the fieldset, hide the raw keys in the add-on controls, and make the row label fall back to the key in words, so a snake_case key is never a label. Add e2e and unit tests that fail when a raw key is drawn. Assisted-by: Claude Code:claude-sonnet-5-5 Signed-off-by: Ettore Di Giacinto --- .../react-ui/e2e/addon-host-settings.spec.js | 35 +++++++++++++++++++ .../react-ui/src/components/addons/addons.css | 4 +++ .../hostcontrols/NetworkPolicySettings.jsx | 4 +-- .../src/components/settings/EgressSection.jsx | 4 +-- .../src/components/settings/SettingField.jsx | 20 ++++++++--- .../tests/unit/addonsettings.test.mjs | 13 +++++++ 6 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 core/http/react-ui/e2e/addon-host-settings.spec.js diff --git a/core/http/react-ui/e2e/addon-host-settings.spec.js b/core/http/react-ui/e2e/addon-host-settings.spec.js new file mode 100644 index 0000000000..80813c01be --- /dev/null +++ b/core/http/react-ui/e2e/addon-host-settings.spec.js @@ -0,0 +1,35 @@ +import { test, expect } from '@playwright/test' +import { ADMIN, app, installedAddon, sessionFor, mockAddons } from './addons-fixtures.js' + +// The host controls on an add-on's Settings tab are drawn with the same rows +// and styles as the Settings page: human labels, no raw setting key as a +// label, and the theme tokens applied. +const SHOTS = process.env.UI_SHOTS_DIR + +test('the host controls of an add-on are styled like the Settings page and show no raw key as a label', async ({ page }) => { + const controlled = app({ id: 'sandbox', name: 'sandbox', title: 'Sandbox', description: 'Run commands.', shadows: ['bash'], ui: { entry: `/api/addons/sandbox/assets/${'a'.repeat(64)}/ui.js`, sdk: { min: 2, max: 2 }, controls: ['network-policy'] } }) + await page.route('**/api/auth/status', route => route.fulfill({ json: sessionFor(ADMIN, { addons: true }) })) + await page.route('**/api/features', route => route.fulfill({ json: { addon_navigation: true } })) + await page.route('**/api/addons/builtin', route => route.fulfill({ json: { builtin: [] } })) + await mockAddons(page, { apps: [controlled], installed: [installedAddon({ id: 'sandbox', title: 'Sandbox' })], hostSettings: { chats_egress_ceiling: 'registries' } }) + await page.route('**/api/settings', route => route.fulfill({ json: { chats_egress_ceiling: 'registries', chats_egress_audit_days: 14 } })) + await page.route('**/api/chats/admin/egress', route => route.fulfill({ json: { ceiling: 'registries', default: 'off', ports: [80, 443], audit_days: 14, registries: [], allowed_24h: 0, blocked_24h: 0 } })) + await page.setViewportSize({ width: 1360, height: 1400 }) + await page.goto('/app/addons?detail=sandbox&view=settings') + const form = page.getByTestId('addon-host-controls') + await expect(form.getByTestId('host-control-network-policy')).toBeVisible() + if (SHOTS) await page.screenshot({ path: `${SHOTS}/${process.env.UI_SHOT_NAME || 'host-settings'}.png` }) + + // Every row has a human label, never the raw snake_case key. + const labels = await form.locator('.st-row__label').allTextContents() + expect(labels.length).toBeGreaterThan(3) + for (const text of labels) expect(text).not.toMatch(/^[a-z0-9]+(_[a-z0-9]+)+$/) + // No raw setting key is drawn anywhere in the block, and the fieldset that + // groups the controls has no browser border. + await expect(form.locator('.st-key')).toHaveCount(0) + for (const key of ['chats_egress_ceiling', 'chats_egress_default', 'chats_egress_ports', 'chats_egress_allow_private']) await expect(form).not.toContainText(key) + expect(await form.locator('fieldset').evaluate(el => getComputedStyle(el).borderTopWidth)).toBe('0px') + // The rows are styled: a row is a grid, not a plain block. + const display = await form.locator('.st-row').first().evaluate(el => getComputedStyle(el).display) + expect(display).toBe('grid') +}) diff --git a/core/http/react-ui/src/components/addons/addons.css b/core/http/react-ui/src/components/addons/addons.css index ec33400a94..2323bb98ce 100644 --- a/core/http/react-ui/src/components/addons/addons.css +++ b/core/http/react-ui/src/components/addons/addons.css @@ -475,6 +475,10 @@ } .ad-pane__tab .lai-icon { width: 14px; height: 14px; } .ad-settings { display: flex; flex-direction: column; gap: var(--sp-5); min-width: 0; } +.ad-host-controls { display: flex; flex-direction: column; gap: var(--sp-3); min-width: 0; } +/* The controls sit in a fieldset so one attribute can disable them all; the browser's own border, padding and min-width are not wanted. */ +.ad-host-controls > fieldset { margin: 0; padding: 0; border: 0; min-width: 0; display: flex; flex-direction: column; gap: var(--sp-4); } +.ad-host-control { display: flex; flex-direction: column; gap: var(--sp-3); min-width: 0; } .ad-settings__server { display: flex; flex-direction: column; gap: var(--sp-2); padding-top: var(--sp-4); border-top: 1px solid var(--dk-edge); } .ad-settings__links { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; border: 1px solid var(--dk-edge); border-radius: var(--r-md); } .ad-settings__link { display: flex; flex-wrap: wrap; align-items: flex-start; justify-content: space-between; gap: var(--sp-2) var(--sp-3); padding: var(--sp-3); } diff --git a/core/http/react-ui/src/components/hostcontrols/NetworkPolicySettings.jsx b/core/http/react-ui/src/components/hostcontrols/NetworkPolicySettings.jsx index 6ac992702f..f4341c4e53 100644 --- a/core/http/react-ui/src/components/hostcontrols/NetworkPolicySettings.jsx +++ b/core/http/react-ui/src/components/hostcontrols/NetworkPolicySettings.jsx @@ -12,10 +12,10 @@ export default function NetworkPolicySettings({ settings, initial, onChange, pen return (

Allowed hosts accept exact names, *.example.com (descendants only; add example.com separately for the apex), or * (public destinations only). Wildcards still obey the ceiling, allowed ports and private-address guard. Extra registries require exact names. The owner validates every change.

- +
{NETWORK_POLICY_FIELDS.filter(key => key !== 'chats_egress_audit_days').map(key => FIELD_BY_KEY[key] && ( - + ))}
diff --git a/core/http/react-ui/src/components/settings/EgressSection.jsx b/core/http/react-ui/src/components/settings/EgressSection.jsx index 21aafc049f..1f17de0f51 100644 --- a/core/http/react-ui/src/components/settings/EgressSection.jsx +++ b/core/http/react-ui/src/components/settings/EgressSection.jsx @@ -109,7 +109,7 @@ function AuditTable() { ) } -export default function EgressSection({ settings, initial, onChange }) { +export default function EgressSection({ settings, initial, onChange, showKey = true }) { const { t } = useTranslation('admin') const [state, setState] = useState({ loading: true, info: null, error: '' }) const [reload, setReload] = useState(0) @@ -141,7 +141,7 @@ export default function EgressSection({ settings, initial, onChange }) {

{t('settings.egress.intro')}

{info.isolation === 'host' && {t('settings.egress.hostNote')}}

{t('settings.egress.counts', { allowed: info.allowed_24h || 0, blocked: info.blocked_24h || 0 })}

-
chats_egress_allow_private
+ {showKey &&
chats_egress_allow_private
}
{t('settings.egress.registries', { count: (info.registries || []).length })} diff --git a/core/http/react-ui/src/components/settings/SettingField.jsx b/core/http/react-ui/src/components/settings/SettingField.jsx index 05036e4adf..7c85e38278 100644 --- a/core/http/react-ui/src/components/settings/SettingField.jsx +++ b/core/http/react-ui/src/components/settings/SettingField.jsx @@ -21,6 +21,18 @@ function ApplyHint({ field }) { return null } +const RAW_KEY = /^[a-z0-9]+(_[a-z0-9]+)+$/ + +// The name a row shows. A field without a human label, or one that carries its +// own snake_case key, gets the key in words ("chats_egress_ports" becomes +// "Chats egress ports"), so a raw key is never the label of a row. +export function labelOf(field) { + const label = field.label && !RAW_KEY.test(field.label) ? field.label : '' + if (label) return label + const words = String(field.label || field.key || '').replace(/_/g, ' ').trim() + return words ? words.charAt(0).toUpperCase() + words.slice(1) : '' +} + function coerceInt(field, text) { const floor = field.min ?? 0 const n = Math.max(floor, parseInt(text, 10) || floor) @@ -136,7 +148,7 @@ function Control({ field, settings, onChange, id, labelId, disabled, assets }) {