From a17c4551ba5d0f97815ff4000300c3b08e257c9b Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:20:09 -0400 Subject: [PATCH] Fix: draw a settings separator only between rows that are visible The exemption was .row:last-child, which asks about position in the DOM. The Security group ends with two rows that stay hidden until password protection is on, so the exemption landed on a hidden row and Password Protection drew a border under itself with nothing beneath it. A selector cannot see an inline display, so the four rows that hide now carry d-none and the seven places that toggled them set the class. The rule asks whether any later row is visible, which also makes the drow copy of it redundant. --- CHANGELOG.md | 3 ++ ui/admin/index.html | 8 ++-- ui/css/admin.css | 7 ++- ui/js/admin-settings.js | 8 ++-- ui/js/admin.js | 6 +-- ui/test/admin-row-separator.test.mjs | 70 ++++++++++++++++++++++++++++ 6 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 ui/test/admin-row-separator.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a3ee083..6858e98a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The Books, Now Playing and Backup widgets now show why they failed, such as a missing API key or a rejected password, instead of a generic error. +- The last visible row in a Settings group no longer draws a separator under + itself, which showed under Password Protection whenever protection was off. + ### Security - A Connections tile that cannot reach a service now says so in plain words diff --git a/ui/admin/index.html b/ui/admin/index.html index 3a0fdb39..b6e8535c 100644 --- a/ui/admin/index.html +++ b/ui/admin/index.html @@ -89,8 +89,8 @@
Docker Container Health Checks
- - +
Hide Healthy Badge
+
Socket URLhttp://socket-proxy:2375

Errors shows warnings and errors only. Security events are always logged.

@@ -98,8 +98,8 @@
Security
Password Protection
- - +
PasswordNot set
+
Sign out all devices
diff --git a/ui/css/admin.css b/ui/css/admin.css index a5b617d6..19bb445d 100644 --- a/ui/css/admin.css +++ b/ui/css/admin.css @@ -92,7 +92,11 @@ html{background:var(--bg-color,var(--bg-outer))} /* ── Rows ── */ .row{display:flex;align-items:center;gap:10px;padding:0 16px;min-height:52px;border-bottom:1px solid var(--bd-inner)} -.row:last-child{border-bottom:none} +/* The separator belongs between two visible rows. :last-child is a DOM + position, and a group's trailing rows can be hidden, which left a border + under the last row anyone could see. Hidden rows carry .d-none, never an + inline display, so this can ask the question. */ +.row:not(:has(~ .row:not(.d-none))){border-bottom:none} .row-link{text-decoration:none;color:inherit;transition:background var(--t)} .row-link:hover{background:rgba(255,255,255,.04)} .rl{flex:1;font-size:14px;color:var(--tx);font-weight:400} @@ -140,7 +144,6 @@ html{background:var(--bg-color,var(--bg-outer))} .chip.on{background:var(--ac);color:#fff} .chip:hover:not(.on){color:var(--tx)} .row.drow{display:grid;grid-template-columns:20px 48px 1fr auto auto;gap:12px;padding:10px 16px;min-height:66px;cursor:grab;border-bottom:1px solid var(--bd-inner);align-items:center} -.row.drow:last-child{border-bottom:none} .row.drow:hover{background:rgba(255,255,255,.025)} .row.drow.dragging{opacity:.4;cursor:grabbing} .row.drow.drag-above{box-shadow:0 -2px 0 0 var(--ac)} diff --git a/ui/js/admin-settings.js b/ui/js/admin-settings.js index 47cccaac..f5f52fa4 100644 --- a/ui/js/admin-settings.js +++ b/ui/js/admin-settings.js @@ -152,9 +152,9 @@ export function loadSettings(c) { dockerEnEl.checked = !!s.server?.socketProxyUrl; const applyDocker = v => { if (dockerSubEl) dockerSubEl.classList.toggle('open', v); - if (hideHealthyRowEl) hideHealthyRowEl.style.display = v ? '' : 'none'; + if (hideHealthyRowEl) hideHealthyRowEl.classList.toggle('d-none', !v); const socketRow = el('ie-socket'); - if (socketRow) socketRow.style.display = v ? '' : 'none'; + if (socketRow) socketRow.classList.toggle('d-none', !v); const socketHint = el('socket-hint'); if (socketHint) socketHint.style.display = v ? '' : 'none'; }; @@ -188,7 +188,7 @@ export function loadSettings(c) { const on = !!secEnEl?.checked; if (secLogout) secLogout.classList.toggle('d-none', !on); const canRevoke = on && _passwordSet; - if (secRevokeRow) secRevokeRow.style.display = canRevoke ? '' : 'none'; + if (secRevokeRow) secRevokeRow.classList.toggle('d-none', !canRevoke); if (revokeTip) revokeTip.style.display = canRevoke ? '' : 'none'; }; secLogout?.addEventListener('click', async () => { @@ -226,7 +226,7 @@ export function loadSettings(c) { secEnEl.checked = !!d.enabled; const pwRow = el('ie-pw'); const pwHint = el('pw-hint-static'); - if (pwRow) pwRow.style.display = d.enabled ? '' : 'none'; + if (pwRow) pwRow.classList.toggle('d-none', !d.enabled); if (pwHint) pwHint.style.display = d.enabled ? '' : 'none'; } const pwValEl = el('ie-pw-v'); diff --git a/ui/js/admin.js b/ui/js/admin.js index c88143d6..38f2ba49 100644 --- a/ui/js/admin.js +++ b/ui/js/admin.js @@ -1096,7 +1096,7 @@ function initSecToggle() { const pwHint = el('pw-hint-static'); if (!en) return; function apply(on) { - if (pwRow) pwRow.style.display = on ? '' : 'none'; + if (pwRow) pwRow.classList.toggle('d-none', !on); if (pwHint) pwHint.style.display = on ? '' : 'none'; } apply(en.checked); @@ -1109,8 +1109,8 @@ function initDockerToggle() { const socketRow = el('ie-socket'); if (!en) return; function apply(on) { - if (hideRow) hideRow.style.display = on ? '' : 'none'; - if (socketRow) socketRow.style.display = on ? '' : 'none'; + if (hideRow) hideRow.classList.toggle('d-none', !on); + if (socketRow) socketRow.classList.toggle('d-none', !on); } apply(en.checked); en.addEventListener('change', () => apply(en.checked)); diff --git a/ui/test/admin-row-separator.test.mjs b/ui/test/admin-row-separator.test.mjs new file mode 100644 index 00000000..84cf702c --- /dev/null +++ b/ui/test/admin-row-separator.test.mjs @@ -0,0 +1,70 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/* A settings row's separator belongs between two rows the user can see. + + The rule was `.row:last-child { border-bottom: none }`, which asks a question + about position in the DOM rather than about what is on screen. Security's + group ends with two rows that stay hidden until password protection is on, so + the exemption landed on a hidden row and the visible last row, Password + Protection, drew a border under itself with nothing beneath it. + + The fix needs both halves to hold: the stylesheet has to ask whether a later + row is visible, and a hidden row has to be hidden in a way CSS can see. An + inline style="display:none" is invisible to a selector, so rows use .d-none. */ + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const read = p => fs.readFileSync(path.join(root, p), 'utf8'); +const css = read('css/admin.css'); +const html = read('admin/index.html'); + +test('the separator is removed from the last visible row, not the last child', () => { + assert.match( + css, + /\.row:not\(:has\(~ \.row:not\(\.d-none\)\)\)\{border-bottom:none\}/, + 'the visibility-aware rule is gone', + ); + assert.doesNotMatch( + css, + /\.row(\.\w+)?:last-child\{border-bottom:none\}/, + 'a :last-child exemption is back; it cannot see a hidden trailing row', + ); +}); + +test('a row hides with the class, never with an inline style', () => { + const offenders = [...html.matchAll(/]*class="row[^"]*"[^>]*>/g)] + .map(m => m[0]) + .filter(tag => /style="[^"]*display\s*:\s*none/.test(tag)) + .map(tag => { + const id = /id="([^"]+)"/.exec(tag); + return id ? id[1] : '(no id)'; + }); + assert.deepEqual( + offenders, + [], + `These rows hide in a way the stylesheet cannot see. Use class="… d-none":\n ${offenders.join('\n ')}`, + ); +}); + +test('nothing toggles a row back to an inline display', () => { + const offenders = []; + for (const file of ['js/admin.js', 'js/admin-settings.js']) { + const src = read(file).replace(/\/\*[\s\S]*?\*\//g, ''); + for (const m of src.matchAll(/(\w*[Rr]ow\w*)\.style\.display\s*=/g)) { + offenders.push(`${file}: ${m[0]}`); + } + } + assert.deepEqual( + offenders, + [], + `Use classList.toggle('d-none', …) so the separator rule still applies:\n ${offenders.join('\n ')}`, + ); +}); + +/* The class has to actually hide, and to win against the row's own display. */ +test('d-none is defined and beats the row layout', () => { + assert.match(css, /\.d-none\{display:none!important\}/); +});