From aa41b1c71d1473b53ef77e113ca5ed32da09af97 Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:34:09 -0400 Subject: [PATCH] Fix: resolve showIf per field, not per key Two fields may share a key, one per branch of a showIf. Resolving visibility by key gave both the same answer, so the backup widget showed the Duplicati job picker alongside the Kopia one, and showed no picker at all for Duplicati. Add visibleFieldFlags, which answers by position, and apply it to the rendered rows by index. --- CHANGELOG.md | 3 ++ ui/js/admin-logic.js | 40 ++++++++++++++++++------- ui/js/widget-config-form.js | 12 ++++---- ui/test/config-card-visibility.test.mjs | 19 ++++++++++-- 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f53c3b3a..d22e7aeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The App, Widget and Folder tiles under Add New show their names instead of their internal keys. +- The Backup widget shows the job picker for the provider you picked. Kopia + showed the Duplicati picker as well, and Duplicati showed no picker at all. + ## [1.6.0] - 2026-08-13 ### Added diff --git a/ui/js/admin-logic.js b/ui/js/admin-logic.js index 4662c86f..1c720201 100644 --- a/ui/js/admin-logic.js +++ b/ui/js/admin-logic.js @@ -30,26 +30,44 @@ export function showIfMatches(cond, current) { } /* Follow showIf chains. A hidden controller still holds a value underneath, and - its dependants would show themselves against it. */ -export function visibleFieldKeys(fields, readValue) { - const byKey = new Map(fields.map(f => [f.key, f])); + its dependants would show themselves against it. + + Resolve each field by its position. Two fields may share a key, one per + branch of a showIf, and resolving by key gives both the same answer: the + wrong branch is shown, or both branches are hidden. */ +export function visibleFieldFlags(fields, readValue) { + const idxsByKey = new Map(); + fields.forEach((f, i) => { + if (!f || f.key == null) return; + const at = idxsByKey.get(f.key); + if (at) at.push(i); + else idxsByKey.set(f.key, [i]); + }); const memo = new Map(); - const isShown = key => { - if (memo.has(key)) return memo.get(key); - memo.set(key, false); /* guard against a cycle in malformed manifests */ - const f = byKey.get(key); + const isShown = i => { + if (memo.has(i)) return memo.get(i); + memo.set(i, false); /* guard against a cycle in malformed manifests */ + const f = fields[i]; let ok = true; if (f && f.showIf) { const dep = f.showIf.field; - ok = byKey.has(dep) - ? isShown(dep) && showIfMatches(f.showIf, readValue(dep)) + const depIdxs = idxsByKey.get(dep); + ok = depIdxs + ? depIdxs.some(j => isShown(j)) && showIfMatches(f.showIf, readValue(dep)) : showIfMatches(f.showIf, readValue(dep)); } - memo.set(key, ok); + memo.set(i, ok); return ok; }; + return fields.map((f, i) => (f && f.key != null ? isShown(i) : false)); +} + +export function visibleFieldKeys(fields, readValue) { + const flags = visibleFieldFlags(fields, readValue); const out = new Set(); - for (const f of fields) if (f.key != null && isShown(f.key)) out.add(f.key); + fields.forEach((f, i) => { + if (flags[i]) out.add(f.key); + }); return out; } diff --git a/ui/js/widget-config-form.js b/ui/js/widget-config-form.js index d4a5adf7..0fa2b745 100644 --- a/ui/js/widget-config-form.js +++ b/ui/js/widget-config-form.js @@ -11,7 +11,7 @@ import { collectFieldValues, requiredFieldMissing, groupBounds, - visibleFieldKeys, + visibleFieldFlags, } from '/js/admin-logic.js?v=056a11e9'; import { optionsErrorText } from '/js/admin-error.js?v=af729113'; import { qi } from '/js/utils.js?v=84d58686'; @@ -489,11 +489,11 @@ function _wireShowIf(built) { const fields = built.map(b => b.field); const readValue = key => (liveByKey[key] ? liveByKey[key]() : undefined); const apply = () => { - const shown = visibleFieldKeys(fields, readValue); - for (const b of built) { - if (!b.field.showIf) continue; - b.el.style.display = shown.has(b.field.key) ? '' : 'none'; - } + const shown = visibleFieldFlags(fields, readValue); + built.forEach((b, i) => { + if (!b.field.showIf) return; + b.el.style.display = shown[i] ? '' : 'none'; + }); }; for (const b of built) { if (b.control) b.control.addEventListener('change', apply); diff --git a/ui/test/config-card-visibility.test.mjs b/ui/test/config-card-visibility.test.mjs index d520f92e..008fbe23 100644 --- a/ui/test/config-card-visibility.test.mjs +++ b/ui/test/config-card-visibility.test.mjs @@ -28,7 +28,7 @@ import { register } from 'node:module'; register('./js-root-hooks.mjs', import.meta.url); const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); -const { visibleFieldKeys } = await import('../js/admin-logic.js'); +const { visibleFieldKeys, visibleFieldFlags } = await import('../js/admin-logic.js'); const widgets = fs .readdirSync(path.join(root, 'widgets'), { withFileTypes: true }) @@ -149,5 +149,20 @@ test('a field is hidden when its condition is not met', () => { test('the form applies that to the rendered rows', () => { const src = fs.readFileSync(path.join(root, 'js/widget-config-form.js'), 'utf8'); - assert.match(src, /b\.el\.style\.display = shown\.has\(b\.field\.key\) \? '' : 'none'/); + assert.match(src, /b\.el\.style\.display = shown\[i\] \? '' : 'none'/); +}); + +/* Two fields may share a key, one per branch. Resolving by key gave both the + same answer: with kopia picked the duplicati job picker showed too, and with + duplicati picked neither showed. */ +test('fields sharing a key follow their own condition', () => { + const fields = [ + { key: 'provider', type: 'select' }, + { key: 'jobId', type: 'select', optionsFrom: 'duplicati-jobs', showIf: { field: 'provider', equals: 'duplicati' } }, + { key: 'jobId', type: 'select', optionsFrom: 'kopia-sources', showIf: { field: 'provider', equals: 'kopia' } }, + ]; + const at = provider => visibleFieldFlags(fields, k => (k === 'provider' ? provider : undefined)); + assert.deepEqual(at('kopia'), [true, false, true]); + assert.deepEqual(at('duplicati'), [true, true, false]); + assert.deepEqual(at(''), [true, false, false]); });