Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions qt/aqt/deckoptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ def _on_bridge_cmd(self, cmd: str) -> None:

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()

Expand Down
3 changes: 2 additions & 1 deletion ts/routes/deck-options/[deckId]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
}
Expand Down
50 changes: 20 additions & 30 deletions ts/routes/deck-options/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -33,7 +34,7 @@ export interface ConfigListEntry {
current: boolean;
}

type Configs =
type AllConfigs =
& Required<
Pick<
PlainMessage<UpdateDeckConfigsRequest>,
Expand All @@ -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>>;
Expand All @@ -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[];
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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) {
Expand Down