From f1746fa4b864197d297467e70511b6ae50940b78 Mon Sep 17 00:00:00 2001 From: Immick <12298761+Immick@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:03:51 +0300 Subject: [PATCH] fix(customisation-sync): hide identical items with "Hide not applicable items" The checkbox only toggled a CSS class for rows labelled "All the same or non-existent", while `hideNotApplicable`, which filters the offered devices by whether their copy can be applied, was hard-coded to false. Items present on another device therefore stayed visible even when every copy was the same. Drive `hideNotApplicable` from the checkbox, so the comparison only runs while it is on, and pass it to the reactive update so toggling it re-evaluates the rows. The term selection moves into PluginTerms.ts so it can be unit-tested. Fixes #1193 --- src/features/ConfigSync/PluginCombo.svelte | 31 +++++------ src/features/ConfigSync/PluginPane.svelte | 3 +- src/features/ConfigSync/PluginTerms.ts | 32 +++++++++++ .../ConfigSync/PluginTerms.unit.spec.ts | 54 +++++++++++++++++++ 4 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 src/features/ConfigSync/PluginTerms.ts create mode 100644 src/features/ConfigSync/PluginTerms.unit.spec.ts diff --git a/src/features/ConfigSync/PluginCombo.svelte b/src/features/ConfigSync/PluginCombo.svelte index 7daba34a..91798795 100644 --- a/src/features/ConfigSync/PluginCombo.svelte +++ b/src/features/ConfigSync/PluginCombo.svelte @@ -12,6 +12,7 @@ // import { askString } from "../../common/utils"; import { Menu } from "@/deps.ts"; import { $msg as translateMessage } from "@/common/translation"; + import { selectSourceTerms } from "./PluginTerms.ts"; export let list: IPluginDataExDisplay[] = []; export let thisTerm = ""; @@ -182,24 +183,20 @@ } } - async function updateTerms(list: IPluginDataExDisplay[], selectNewest: boolean, isMaintenanceMode: boolean) { + async function updateTerms( + list: IPluginDataExDisplay[], + selectNewest: boolean, + isMaintenanceMode: boolean, + hideNotApplicable: boolean + ) { const local = list.find((e) => e.term == thisTerm); // selected = ""; - if (isMaintenanceMode) { - terms = [...new Set(list.map((e) => e.term))]; - } else if (hideNotApplicable) { - const termsTmp = []; - const wk = [...new Set(list.map((e) => e.term))]; - for (const termName of wk) { - const remote = list.find((e) => e.term == termName); - if ((await comparePlugin(local, remote)).canApply) { - termsTmp.push(termName); - } - } - terms = [...termsTmp]; - } else { - terms = [...new Set(list.map((e) => e.term))].filter((e) => e != thisTerm); - } + terms = await selectSourceTerms( + list, + thisTerm, + { isMaintenanceMode, hideNotApplicable }, + async (local, remote) => (await comparePlugin(local, remote)).canApply + ); let newest: IPluginDataExDisplay | undefined = local; if (selectNewest) { for (const term of terms) { @@ -230,7 +227,7 @@ } // currentSelectNewest = selectNewest; } - updateTerms(list, doSelectNewest, isMaintenanceMode); + updateTerms(list, doSelectNewest, isMaintenanceMode, hideNotApplicable); currentSelectNewest = selectNewest; } $: { diff --git a/src/features/ConfigSync/PluginPane.svelte b/src/features/ConfigSync/PluginPane.svelte index a6be2a6e..18a65109 100644 --- a/src/features/ConfigSync/PluginPane.svelte +++ b/src/features/ConfigSync/PluginPane.svelte @@ -32,7 +32,8 @@ export let core :LiveSyncBaseCore; // $: core = plugin.core; - $: hideNotApplicable = false; + // Comparing every copy is only worth it while the user asks to hide the unchanged ones. + $: hideNotApplicable = hideEven; $: thisTerm = core.services.setting.getDeviceAndVaultName(); const addOn = core.getAddOn(ConfigSync.name)!; diff --git a/src/features/ConfigSync/PluginTerms.ts b/src/features/ConfigSync/PluginTerms.ts new file mode 100644 index 00000000..71c1d503 --- /dev/null +++ b/src/features/ConfigSync/PluginTerms.ts @@ -0,0 +1,32 @@ +import type { IPluginDataExDisplay } from "./CmdConfigSync.ts"; + +export type CanApplyFrom = ( + local: IPluginDataExDisplay | undefined, + remote: IPluginDataExDisplay | undefined +) => Promise; + +/** + * Devices offered as sources for one item in the Customisation Sync dialogue. + * + * Maintenance mode lists every device, including this one. Otherwise every other + * device that has the item is listed; with `hideNotApplicable`, only devices whose + * copy can actually be applied remain, so an item that is the same everywhere ends + * up with no source and is shown as "All the same or non-existent". + */ +export async function selectSourceTerms( + list: IPluginDataExDisplay[], + thisTerm: string, + options: { isMaintenanceMode: boolean; hideNotApplicable: boolean }, + canApplyFrom: CanApplyFrom +): Promise { + const terms = [...new Set(list.map((e) => e.term))]; + if (options.isMaintenanceMode) return terms; + if (!options.hideNotApplicable) return terms.filter((term) => term != thisTerm); + const local = list.find((e) => e.term == thisTerm); + const applicable: string[] = []; + for (const term of terms) { + const remote = list.find((e) => e.term == term); + if (await canApplyFrom(local, remote)) applicable.push(term); + } + return applicable; +} diff --git a/src/features/ConfigSync/PluginTerms.unit.spec.ts b/src/features/ConfigSync/PluginTerms.unit.spec.ts new file mode 100644 index 00000000..f6fd714a --- /dev/null +++ b/src/features/ConfigSync/PluginTerms.unit.spec.ts @@ -0,0 +1,54 @@ +import { describe, expect, it, vi } from "vitest"; +import type { IPluginDataExDisplay } from "./CmdConfigSync.ts"; +import { selectSourceTerms } from "./PluginTerms.ts"; + +const copyOn = (term: string) => ({ term, files: [] }) as unknown as IPluginDataExDisplay; +const list = [copyOn("desktop"), copyOn("phone"), copyOn("tablet")]; +const differsOn = + (...terms: string[]) => + (_local: IPluginDataExDisplay | undefined, remote: IPluginDataExDisplay | undefined) => + Promise.resolve(terms.includes(remote?.term ?? "")); + +describe("selectSourceTerms", () => { + it("offers every other device without comparing copies by default", async () => { + const canApplyFrom = vi.fn(differsOn()); + const terms = await selectSourceTerms( + list, + "desktop", + { isMaintenanceMode: false, hideNotApplicable: false }, + canApplyFrom + ); + expect(terms).toEqual(["phone", "tablet"]); + expect(canApplyFrom).not.toHaveBeenCalled(); + }); + + it("offers every device, including this one, in maintenance mode", async () => { + const terms = await selectSourceTerms( + list, + "desktop", + { isMaintenanceMode: true, hideNotApplicable: true }, + differsOn() + ); + expect(terms).toEqual(["desktop", "phone", "tablet"]); + }); + + it("leaves out devices whose copy is the same when hiding items that are not applicable", async () => { + const terms = await selectSourceTerms( + list, + "desktop", + { isMaintenanceMode: false, hideNotApplicable: true }, + differsOn("tablet") + ); + expect(terms).toEqual(["tablet"]); + }); + + it("offers no source for an item that is the same on every device", async () => { + const terms = await selectSourceTerms( + list, + "desktop", + { isMaintenanceMode: false, hideNotApplicable: true }, + differsOn() + ); + expect(terms).toEqual([]); + }); +});