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 <mudler@localai.io>
This commit is contained in:
Ettore Di Giacinto committed 2026-11-03 08:40:00 +00:00
1 parent 4eab1148be
commit fc087c13e8
6 files changed
+72 -8

No files matched your search

@@ -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')
})
@@ -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); }
@@ -12,10 +12,10 @@ export default function NetworkPolicySettings({ settings, initial, onChange, pen
return (
<div className="ad-host-control" data-testid="host-control-network-policy">
<p className="st-note">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.</p>
<EgressSection settings={settings} initial={initial} onChange={onChange} />
<EgressSection settings={settings} initial={initial} onChange={onChange} showKey={false} />
<div className="st-list">
{NETWORK_POLICY_FIELDS.filter(key => key !== 'chats_egress_audit_days').map(key => FIELD_BY_KEY[key] && (
<SettingField key={key} field={FIELD_BY_KEY[key]} settings={settings} onChange={onChange} pending={pending.has(key)} />
<SettingField key={key} field={FIELD_BY_KEY[key]} settings={settings} onChange={onChange} pending={pending.has(key)} showKey={false} />
))}
</div>
</div>
@@ -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 }) {
<p className="st-row__desc">{t('settings.egress.intro')}</p>
{info.isolation === 'host' && <Callout tone="warn">{t('settings.egress.hostNote')}</Callout>}
<p className="st-row__desc" data-testid="settings-egress-counts">{t('settings.egress.counts', { allowed: info.allowed_24h || 0, blocked: info.blocked_24h || 0 })}</p>
<div className="st-row__meta"><code className="st-key">chats_egress_allow_private</code></div>
{showKey && <div className="st-row__meta"><code className="st-key">chats_egress_allow_private</code></div>}
<PrivateSwitch settings={settings} initialOn={!!getValue(PRIVATE, initial)} onChange={onChange} />
<details className="st-tools__controls" data-testid="settings-egress-registries">
<summary>{t('settings.egress.registries', { count: (info.registries || []).length })}</summary>
@@ -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 }) {
<label className="dk-btn dk-btn--secondary dk-btn--sm st-upload" aria-busy={a.busy || undefined}>
<Icon name="upload" /> {a.busy ? 'Uploading…' : 'Upload'}
<input
type="file" className="dk-sr-only" disabled={a.busy} aria-label={`Upload ${field.label}`}
type="file" className="dk-sr-only" disabled={a.busy} aria-label={`Upload ${labelOf(field)}`}
accept="image/png,image/jpeg,image/svg+xml,image/webp,image/x-icon,.ico"
onChange={(e) => {
const file = e.target.files?.[0]
@@ -174,7 +186,7 @@ const MODEL_CAPABILITY = {
// One setting: its name and words on the left, its control on the right, and
// under the words what is worth knowing (where it was, its key, when it
// applies, whether it differs from the default and how to put that back).
export default function SettingField({ field, settings, onChange, assets, showWhere, pending }) {
export default function SettingField({ field, settings, onChange, assets, showWhere, pending, showKey = true }) {
const id = `st-${field.key.replace(/[^a-z0-9_]/gi, '-')}`
const labelId = `${id}-label`
const needsOn = !field.needs || !!getValue(FIELD_BY_KEY[field.needs], settings)
@@ -189,14 +201,14 @@ export default function SettingField({ field, settings, onChange, assets, showWh
<div className="st-row" data-wide={wide || undefined} data-bool={field.kind === 'bool' || undefined} data-changed={changed || undefined} data-draft={pending || undefined} data-disabled={disabled || undefined} data-field={field.key}>
<div className="st-row__main">
<div className="st-row__title">
<label id={labelId} htmlFor={field.kind === 'bool' ? undefined : id} className="st-row__label">{field.label}</label>
<label id={labelId} htmlFor={field.kind === 'bool' ? undefined : id} className="st-row__label">{labelOf(field)}</label>
{pending && <span className="st-tag" data-tone="draft">Draft</span>}
{changed && <span className="st-tag" data-tone="changed">Changed</span>}
</div>
<p className="st-row__desc">{field.desc}</p>
<div className="st-row__meta">
{showWhere && <span className="st-where">{where}{was && was !== where ? <> · was {was}</> : null}</span>}
{!field.asset && <code className="st-key">{field.key}</code>}
{showKey && !field.asset && <code className="st-key">{field.key}</code>}
<ApplyHint field={field} />
{changed && hasDefault(field) && (
<span className="st-default">
@@ -114,3 +114,16 @@ test('add-on settings: the SDK settings kit is the host\'s, registered for every
assert.ok(css.includes(`${prop}:`), `${name}: ${prop} is defined`)
}
})
test('settings rows: no field is labelled with its raw snake_case key', async () => {
// SettingField.jsx imports JSX, which node cannot load; its label rule is read from the source and run here.
const text = readFileSync(join(src, 'components/settings/SettingField.jsx'), 'utf8')
const rule = text.match(/const RAW_KEY = (\/.*\/)\n/)[1]
const raw = new RegExp(rule.slice(1, -1))
for (const key of NETWORK_POLICY_FIELDS) assert.ok(FIELD_BY_KEY[key], `${key} has a field`)
for (const field of Object.values(FIELD_BY_KEY)) {
assert.ok(field.label, `${field.key} has a label`)
assert.ok(!raw.test(field.label), `${field.key} is labelled with a raw key: ${field.label}`)
}
assert.match(text, /export function labelOf/)
})