Fix 'Discard changes' dialog appearing even when no changes are made (#3495)

* Fix 'Discard changes' dialog appearing even when no changes are made

https://forums.ankiweb.net/t/anki-24-10-beta/49989/166

* Fix geometry of deck options window not being saved

evt.accept() does not seem to trigger reject().

qt/aqt/deckoptions.py

@@ -69,8 +69,7 @@ class DeckOptionsDialog(QDialog):
 
     def closeEvent(self, evt: QCloseEvent) -> None:
         if self._close_event_has_cleaned_up:
-            evt.accept()
-            return
+            return super().closeEvent(evt)
         evt.ignore()
         self.check_pending_changes()
 

ts/routes/deck-options/[deckId]/+page.svelte

@@ -16,7 +16,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     globalThis.anki.deckOptionsPendingChanges = async (): Promise<void> => {
         await commitEditing();
         if (bridgeCommandsAvailable()) {
-            if (data.state.isModified()) {
+            if (await data.state.isModified()) {
                 bridgeCommand("confirmDiscardChanges");
             } else {
                 bridgeCommand("_close");
@@ -28,6 +28,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         globalThis.$deckOptions = new Promise((resolve, _reject) => {
             resolve(page);
         });
+        data.state.resolveOriginalConfigs();
         if (bridgeCommandsAvailable()) {
             bridgeCommand("deckOptionsReady");
         }

ts/routes/deck-options/lib.ts

@@ -11,7 +11,8 @@ import type {
 import { DeckConfig, DeckConfig_Config, DeckConfigsForUpdate_CurrentDeck_Limits } from "@generated/anki/deck_config_pb";
 import { updateDeckConfigs } from "@generated/backend";
 import { localeCompare } from "@tslib/i18n";
-import { cloneDeep, isEqual, pickBy } from "lodash-es";
+import { promiseWithResolver } from "@tslib/promise";
+import { cloneDeep, isEqual } from "lodash-es";
 import { tick } from "svelte";
 import type { Readable, Writable } from "svelte/store";
 import { get, readable, writable } from "svelte/store";
@@ -33,7 +34,7 @@ export interface ConfigListEntry {
     current: boolean;
 }
 
-type Configs =
+type AllConfigs =
     & Required<
         Pick<
             PlainMessage<UpdateDeckConfigsRequest>,
@@ -48,10 +49,6 @@ type Configs =
     >
     & { currentConfig: DeckConfig_Config };
 
-type DeepPartial<T extends object, K extends keyof T> = {
-    [key in keyof T]: key extends K ? Partial<T[key]> : T[key];
-};
-
 export class DeckOptionsState {
     readonly currentConfig: Writable<DeckConfig_Config>;
     readonly currentAuxData: Writable<Record<string, unknown>>;
@@ -68,7 +65,8 @@ export class DeckOptionsState {
     readonly daysSinceLastOptimization: Writable<number>;
     readonly currentPresetName: Writable<string>;
     /** Used to detect if there are any pending changes */
-    readonly originalConfigs: Configs;
+    readonly originalConfigsPromise: Promise<AllConfigs>;
+    readonly originalConfigsResolve: (value: AllConfigs) => void;
 
     private targetDeckId: DeckOptionsId;
     private configs: ConfigWithCount[];
@@ -124,16 +122,9 @@ export class DeckOptionsState {
         this.currentConfig.subscribe((val) => this.onCurrentConfigChanged(val));
         this.currentAuxData.subscribe((val) => this.onCurrentAuxDataChanged(val));
 
-        this.originalConfigs = cloneDeep<Configs>({
-            configs: this.configs.map(c => c.config!),
-            cardStateCustomizer: data.cardStateCustomizer,
-            limits: get(this.deckLimits),
-            newCardsIgnoreReviewLimit: data.newCardsIgnoreReviewLimit,
-            applyAllParentLimits: data.applyAllParentLimits,
-            fsrs: data.fsrs,
-            fsrsReschedule: get(this.fsrsReschedule),
-            currentConfig: get(this.currentConfig),
-        });
+        // Must be resolved after all components are mounted, as some components
+        // may modify the config during their initialization.
+        [this.originalConfigsPromise, this.originalConfigsResolve] = promiseWithResolver<AllConfigs>();
     }
 
     setCurrentIndex(index: number): void {
@@ -326,24 +317,28 @@ export class DeckOptionsState {
         return list;
     }
 
-    isModified(): boolean {
-        const original: DeepPartial<Configs, "limits"> = {
-            ...this.originalConfigs,
-            limits: omitUndefined(this.originalConfigs.limits),
-        };
-        const current: typeof original = {
+    private getAllConfigs(): AllConfigs {
+        return cloneDeep({
             configs: this.configs.map(c => c.config),
             cardStateCustomizer: get(this.cardStateCustomizer),
-            limits: omitUndefined(get(this.deckLimits)),
+            limits: get(this.deckLimits),
             newCardsIgnoreReviewLimit: get(this.newCardsIgnoreReviewLimit),
             applyAllParentLimits: get(this.applyAllParentLimits),
             fsrs: get(this.fsrs),
             fsrsReschedule: get(this.fsrsReschedule),
             currentConfig: get(this.currentConfig),
-        };
+        });
+    }
 
+    async isModified(): Promise<boolean> {
+        const original = await this.originalConfigsPromise;
+        const current = this.getAllConfigs();
         return !isEqual(original, current);
     }
+
+    resolveOriginalConfigs(): void {
+        this.originalConfigsResolve(this.getAllConfigs());
+    }
 }
 
 function bytesToObject(bytes: Uint8Array): Record<string, unknown> {
@@ -413,11 +408,6 @@ export class ValueTab {
     }
 }
 
-/** Returns a copy of the given object with the properties whose values are 'undefined' omitted */
-function omitUndefined<T extends object>(obj: T): Partial<T> {
-    return pickBy(obj, val => val !== undefined);
-}
-
 /** Ensure blur handler has fired so changes get committed. */
 export async function commitEditing(): Promise<void> {
     if (document.activeElement instanceof HTMLElement) {