From 6a25bd1909366c60ae3e4eb4a7820b3da38fbf35 Mon Sep 17 00:00:00 2001 From: Melody Ruch <396921921@qq.com> Date: Mon, 17 Aug 2026 14:52:00 +0800 Subject: [PATCH] fix(settings): confirm before discarding unsaved settings --- .../editor/EditorSettingsDialog.vue | 42 ++++++++++++++- .../EditorSettingsDialog.unsavedClose.spec.ts | 51 +++++++++++++++++++ apps/desktop/src/i18n/locales/en.ts | 4 ++ apps/desktop/src/i18n/locales/es.ts | 4 ++ apps/desktop/src/i18n/locales/it.ts | 4 ++ apps/desktop/src/i18n/locales/ja.ts | 4 ++ apps/desktop/src/i18n/locales/ko.ts | 4 ++ apps/desktop/src/i18n/locales/pt-BR.ts | 4 ++ apps/desktop/src/i18n/locales/zh-CN.ts | 4 ++ apps/desktop/src/i18n/locales/zh-TW.ts | 4 ++ .../__tests__/editorSettingsDraft.spec.ts | 29 ++++++++++- .../src/lib/settings/editorSettingsDraft.ts | 9 ++++ 12 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 apps/desktop/src/components/editor/__tests__/EditorSettingsDialog.unsavedClose.spec.ts diff --git a/apps/desktop/src/components/editor/EditorSettingsDialog.vue b/apps/desktop/src/components/editor/EditorSettingsDialog.vue index b5bd2f5e10..55425b0a18 100644 --- a/apps/desktop/src/components/editor/EditorSettingsDialog.vue +++ b/apps/desktop/src/components/editor/EditorSettingsDialog.vue @@ -126,7 +126,7 @@ import McpConnectionScopePicker from "@/components/settings/McpConnectionScopePi import ScheduledDatabaseBackupSettings from "@/components/backup/ScheduledDatabaseBackupSettings.vue"; import SqlFormatterSettingsPanel from "./SqlFormatterSettingsPanel.vue"; import { APP_CUSTOM_UI_COLOR_DEFS, APP_THEME_PALETTES, type AppCornerStyle, type AppCustomUiColors, type AppThemeAppearance, type AppThemeMode, type AppThemePalette } from "@/lib/app/appTheme"; -import { editorSettingsDraftChanged, editorSettingsDraftFromSettings, editorSettingsPatchFromDraft, normalizeQueryResultMaxRowsDraft, normalizeTableOpenPageSizeDraft, type EditorSettingsDraft } from "@/lib/settings/editorSettingsDraft"; +import { editorSettingsDraftChanged, editorSettingsDraftFromSettings, editorSettingsPatchFromDraft, normalizeQueryResultMaxRowsDraft, normalizeTableOpenPageSizeDraft, shouldConfirmEditorSettingsDialogClose, type EditorSettingsDraft } from "@/lib/settings/editorSettingsDraft"; import { useConnectionStore } from "@/stores/connectionStore"; import { useSavedSqlStore } from "@/stores/savedSqlStore"; import { usePromptTemplateStore } from "@/stores/promptTemplateStore"; @@ -221,11 +221,31 @@ const settingsContentClass = computed(() => ); const settingsTitleComponent = computed(() => (isSettingsPage.value ? "h2" : DialogTitle)); +const showUnsavedSettingsCloseConfirm = ref(false); + +function requestCloseSettings(nextOpen: boolean) { + if (shouldConfirmEditorSettingsDialogClose(nextOpen, hasChanges())) { + showUnsavedSettingsCloseConfirm.value = true; + return; + } + emit("update:open", nextOpen); +} + function onSettingsRootOpenChange(value: boolean) { - if (!isSettingsPage.value) emit("update:open", value); + if (isSettingsPage.value) return; + requestCloseSettings(value); } function closeSettings() { + requestCloseSettings(false); +} + +function cancelUnsavedSettingsClose() { + showUnsavedSettingsCloseConfirm.value = false; +} + +function discardUnsavedSettingsAndClose() { + showUnsavedSettingsCloseConfirm.value = false; emit("update:open", false); } @@ -6928,6 +6948,24 @@ onUnmounted(() => { + + + + + {{ t("settings.unsavedChangesCloseTitle") }} + +

{{ t("settings.unsavedChangesCloseMessage") }}

+ + + + + + + diff --git a/apps/desktop/src/components/editor/__tests__/EditorSettingsDialog.unsavedClose.spec.ts b/apps/desktop/src/components/editor/__tests__/EditorSettingsDialog.unsavedClose.spec.ts new file mode 100644 index 0000000000..5616e3796a --- /dev/null +++ b/apps/desktop/src/components/editor/__tests__/EditorSettingsDialog.unsavedClose.spec.ts @@ -0,0 +1,51 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const dialogSource = readFileSync(new URL("../EditorSettingsDialog.vue", import.meta.url), "utf8"); + +// Regression for https://github.com/t8y2/dbx/issues/5905: changing a shortcut +// or the sidebar activation mode (and every other draft-gated editor +// setting) was silently discarded whenever the dialog closed any way other +// than the explicit "Apply"/"Apply and Close" buttons — Escape, clicking +// outside, the X button, and every tab's "Close" footer button all bypassed +// the draft entirely. Users only noticed after a later app restart (e.g. to +// install an update) surfaced the still-default values, making it look like +// the update itself had reset their settings. +describe("EditorSettingsDialog unsaved-changes close guard", () => { + it("imports the shared unsaved-changes decision helper", () => { + expect(dialogSource).toContain('shouldConfirmEditorSettingsDialogClose, type EditorSettingsDraft } from "@/lib/settings/editorSettingsDraft"'); + }); + + it("routes every close path through the same guard instead of emitting update:open directly", () => { + expect(dialogSource).toContain("function requestCloseSettings(nextOpen: boolean) {"); + expect(dialogSource).toContain("if (shouldConfirmEditorSettingsDialogClose(nextOpen, hasChanges())) {"); + expect(dialogSource).toContain("showUnsavedSettingsCloseConfirm.value = true;"); + + // Escape / outside click / the dialog's own X button + expect(dialogSource).toContain("function onSettingsRootOpenChange(value: boolean) {\n if (isSettingsPage.value) return;\n requestCloseSettings(value);\n}"); + // Every tab footer's "Close" button — must work in both dialog and page + // (variant="page") mode, since App.vue relies on closeSettings() emitting + // update:open(false) in page mode too (see @update:open on EditorSettingsPage). + expect(dialogSource).toContain("function closeSettings() {\n requestCloseSettings(false);\n}"); + + // Neither close entrypoint may emit update:open directly anymore - only requestCloseSettings does. + const closeSettingsBody = dialogSource.slice(dialogSource.indexOf("function closeSettings()"), dialogSource.indexOf("function cancelUnsavedSettingsClose()")); + expect(closeSettingsBody).not.toContain('emit("update:open"'); + }); + + it("lets the user cancel back into the dialog without losing the draft", () => { + expect(dialogSource).toContain("function cancelUnsavedSettingsClose() {\n showUnsavedSettingsCloseConfirm.value = false;\n}"); + }); + + it("only actually closes (dropping the draft) once the user explicitly confirms discarding", () => { + expect(dialogSource).toContain('function discardUnsavedSettingsAndClose() {\n showUnsavedSettingsCloseConfirm.value = false;\n emit("update:open", false);\n}'); + }); + + it("renders a confirmation dialog wired to the guard state and both actions", () => { + expect(dialogSource).toContain(''); + expect(dialogSource).toContain('{{ t("settings.unsavedChangesCloseTitle") }}'); + expect(dialogSource).toContain('{{ t("settings.unsavedChangesCloseMessage") }}'); + expect(dialogSource).toContain('@click="cancelUnsavedSettingsClose"'); + expect(dialogSource).toContain('@click="discardUnsavedSettingsAndClose"'); + }); +}); diff --git a/apps/desktop/src/i18n/locales/en.ts b/apps/desktop/src/i18n/locales/en.ts index 4fe1edff76..fea25cc458 100644 --- a/apps/desktop/src/i18n/locales/en.ts +++ b/apps/desktop/src/i18n/locales/en.ts @@ -5807,6 +5807,10 @@ export default { duckDbWorkerProcessIsolationRestartRequired: "Restart DBX for this change to take effect.", restartDbx: "Restart DBX", restartDbxFailed: "Failed to restart DBX: {error}", + unsavedChangesCloseTitle: "Unsaved changes", + unsavedChangesCloseMessage: "You have unsaved settings changes that will be lost if you close now. Discard these changes?", + unsavedChangesCloseCancel: "Keep editing", + unsavedChangesCloseDiscard: "Discard changes", debugLogsCopy: "Copy logs", debugLogsCopied: "Copied", debugLogsDownload: "Download logs", diff --git a/apps/desktop/src/i18n/locales/es.ts b/apps/desktop/src/i18n/locales/es.ts index 2bf2d06a91..3be7be3941 100644 --- a/apps/desktop/src/i18n/locales/es.ts +++ b/apps/desktop/src/i18n/locales/es.ts @@ -5548,6 +5548,10 @@ export default withEnglishFallback({ duckDbWorkerProcessIsolationRestartRequired: "Reinicia DBX para que este cambio surta efecto.", restartDbx: "Reiniciar DBX", restartDbxFailed: "No se pudo reiniciar DBX: {error}", + unsavedChangesCloseTitle: "Cambios sin guardar", + unsavedChangesCloseMessage: "Tienes cambios de configuración sin guardar que se perderán si cierras ahora. ¿Descartar estos cambios?", + unsavedChangesCloseCancel: "Seguir editando", + unsavedChangesCloseDiscard: "Descartar cambios", debugLogsCopy: "Copiar logs", debugLogsCopied: "Copiado", debugLogsDownload: "Descargar logs", diff --git a/apps/desktop/src/i18n/locales/it.ts b/apps/desktop/src/i18n/locales/it.ts index 323e4f7aeb..7a43fc3e2a 100644 --- a/apps/desktop/src/i18n/locales/it.ts +++ b/apps/desktop/src/i18n/locales/it.ts @@ -5548,6 +5548,10 @@ export default withEnglishFallback({ duckDbWorkerProcessIsolationRestartRequired: "Riavvia DBX affinché questa modifica abbia effetto.", restartDbx: "Riavvia DBX", restartDbxFailed: "Impossibile riavviare DBX: {error}", + unsavedChangesCloseTitle: "Modifiche non salvate", + unsavedChangesCloseMessage: "Ci sono modifiche alle impostazioni non salvate che andranno perse se chiudi ora. Vuoi scartare queste modifiche?", + unsavedChangesCloseCancel: "Continua a modificare", + unsavedChangesCloseDiscard: "Scarta modifiche", debugLogsCopy: "Copia log", debugLogsCopied: "Copiati", debugLogsDownload: "Scarica log", diff --git a/apps/desktop/src/i18n/locales/ja.ts b/apps/desktop/src/i18n/locales/ja.ts index 0e07073a25..c07cd99197 100644 --- a/apps/desktop/src/i18n/locales/ja.ts +++ b/apps/desktop/src/i18n/locales/ja.ts @@ -5583,6 +5583,10 @@ export default withEnglishFallback({ duckDbWorkerProcessIsolationRestartRequired: "この変更を有効にするには DBX を再起動してください。", restartDbx: "DBX を再起動", restartDbxFailed: "DBX の再起動に失敗しました: {error}", + unsavedChangesCloseTitle: "未保存の変更", + unsavedChangesCloseMessage: "未保存の設定変更があります。今すぐ閉じると失われます。これらの変更を破棄しますか？", + unsavedChangesCloseCancel: "編集を続ける", + unsavedChangesCloseDiscard: "変更を破棄", debugLogsCopy: "ログをコピー", debugLogsCopied: "コピーしました", debugLogsDownload: "ログをダウンロード", diff --git a/apps/desktop/src/i18n/locales/ko.ts b/apps/desktop/src/i18n/locales/ko.ts index c902eb9293..ef0b2c188f 100644 --- a/apps/desktop/src/i18n/locales/ko.ts +++ b/apps/desktop/src/i18n/locales/ko.ts @@ -5277,6 +5277,10 @@ export default withEnglishFallback({ duckDbWorkerProcessIsolationRestartRequired: "이 변경 사항을 적용하려면 DBX를 다시 시작하세요.", restartDbx: "DBX 다시 시작", restartDbxFailed: "DBX 다시 시작 실패: {error}", + unsavedChangesCloseTitle: "저장하지 않은 변경 사항", + unsavedChangesCloseMessage: "저장하지 않은 설정 변경 사항이 있습니다. 지금 닫으면 사라집니다. 이 변경 사항을 취소하시겠습니까?", + unsavedChangesCloseCancel: "계속 편집", + unsavedChangesCloseDiscard: "변경 사항 취소", debugLogsCopy: "로그 복사", debugLogsCopied: "복사했습니다", debugLogsDownload: "로그 다운로드", diff --git a/apps/desktop/src/i18n/locales/pt-BR.ts b/apps/desktop/src/i18n/locales/pt-BR.ts index a62759b48f..129fc91e99 100644 --- a/apps/desktop/src/i18n/locales/pt-BR.ts +++ b/apps/desktop/src/i18n/locales/pt-BR.ts @@ -5550,6 +5550,10 @@ export default withEnglishFallback({ duckDbWorkerProcessIsolationRestartRequired: "Reinicie o DBX para que esta alteração entre em vigor.", restartDbx: "Reiniciar DBX", restartDbxFailed: "Falha ao reiniciar o DBX: {error}", + unsavedChangesCloseTitle: "Alterações não salvas", + unsavedChangesCloseMessage: "Você tem alterações de configuração não salvas que serão perdidas se fechar agora. Descartar essas alterações?", + unsavedChangesCloseCancel: "Continuar editando", + unsavedChangesCloseDiscard: "Descartar alterações", debugLogsCopy: "Copiar logs", debugLogsCopied: "Copiado", debugLogsDownload: "Baixar logs", diff --git a/apps/desktop/src/i18n/locales/zh-CN.ts b/apps/desktop/src/i18n/locales/zh-CN.ts index c14604359a..35ce4e5a15 100644 --- a/apps/desktop/src/i18n/locales/zh-CN.ts +++ b/apps/desktop/src/i18n/locales/zh-CN.ts @@ -5794,6 +5794,10 @@ export default withEnglishFallback({ duckDbWorkerProcessIsolationRestartRequired: "重启 DBX 后生效。", restartDbx: "重启 DBX", restartDbxFailed: "重启 DBX 失败：{error}", + unsavedChangesCloseTitle: "未保存的更改", + unsavedChangesCloseMessage: "当前设置有未保存的更改，关闭后将丢失。是否放弃这些更改？", + unsavedChangesCloseCancel: "继续编辑", + unsavedChangesCloseDiscard: "放弃更改", debugLogsCopy: "复制日志", debugLogsCopied: "已复制", debugLogsDownload: "下载日志", diff --git a/apps/desktop/src/i18n/locales/zh-TW.ts b/apps/desktop/src/i18n/locales/zh-TW.ts index 3c28bbf68a..fd7e325784 100644 --- a/apps/desktop/src/i18n/locales/zh-TW.ts +++ b/apps/desktop/src/i18n/locales/zh-TW.ts @@ -4885,6 +4885,10 @@ export default withEnglishFallback({ duckDbWorkerProcessIsolationRestartRequired: "重啟 DBX 後生效。", restartDbx: "重啟 DBX", restartDbxFailed: "重啟 DBX 失敗：{error}", + unsavedChangesCloseTitle: "未儲存的變更", + unsavedChangesCloseMessage: "目前設定有未儲存的變更，關閉後將遺失。是否放棄這些變更？", + unsavedChangesCloseCancel: "繼續編輯", + unsavedChangesCloseDiscard: "放棄變更", debugLogsCopy: "複製日誌", debugLogsCopied: "已複製", debugLogsDownload: "下載日誌", diff --git a/apps/desktop/src/lib/settings/__tests__/editorSettingsDraft.spec.ts b/apps/desktop/src/lib/settings/__tests__/editorSettingsDraft.spec.ts index db42934a19..076484ee89 100644 --- a/apps/desktop/src/lib/settings/__tests__/editorSettingsDraft.spec.ts +++ b/apps/desktop/src/lib/settings/__tests__/editorSettingsDraft.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { EDITOR_SETTINGS_DRAFT_KEYS, editorSettingsDraftFromSettings, editorSettingsDraftChanged, editorSettingsPatchFromDraft, normalizeQueryResultMaxRowsDraft, normalizeTableOpenPageSizeDraft } from "../editorSettingsDraft"; +import { EDITOR_SETTINGS_DRAFT_KEYS, editorSettingsDraftFromSettings, editorSettingsDraftChanged, editorSettingsPatchFromDraft, normalizeQueryResultMaxRowsDraft, normalizeTableOpenPageSizeDraft, shouldConfirmEditorSettingsDialogClose } from "../editorSettingsDraft"; import type { EditorSettings } from "@/stores/settingsStore"; function makeSettings(overrides: Partial = {}): EditorSettings { @@ -345,6 +345,33 @@ describe("editorSettingsDraftChanged - tabLayout", () => { }); }); +describe("shouldConfirmEditorSettingsDialogClose", () => { + // Regression for https://github.com/t8y2/dbx/issues/5905: customizing a + // shortcut or the sidebar activation mode and then dismissing the dialog + // via Escape/outside-click/the "Close" button (anything other than Apply) + // must not silently drop the draft. + it("requests confirmation when the dialog is closing with an unsaved shortcut/sidebarActivation edit", () => { + const settings = makeSettings({ sidebarActivation: "single" } as Partial); + const base = editorSettingsDraftFromSettings(settings); + const draft = editorSettingsDraftFromSettings(settings); + draft.sidebarActivation = "double"; + + expect(shouldConfirmEditorSettingsDialogClose(false, editorSettingsDraftChanged(draft, base))).toBe(true); + }); + + it("does not block closing when there is no unsaved draft", () => { + const settings = makeSettings(); + const base = editorSettingsDraftFromSettings(settings); + const draft = editorSettingsDraftFromSettings(settings); + + expect(shouldConfirmEditorSettingsDialogClose(false, editorSettingsDraftChanged(draft, base))).toBe(false); + }); + + it("never blocks opening the dialog, even with a stale dirty flag", () => { + expect(shouldConfirmEditorSettingsDialogClose(true, true)).toBe(false); + }); +}); + describe("editorSettingsPatchFromDraft - tabLayout", () => { it("includes tabLayout in patch when changed", () => { const settings = makeSettings({ tabLayout: "scroll" }); diff --git a/apps/desktop/src/lib/settings/editorSettingsDraft.ts b/apps/desktop/src/lib/settings/editorSettingsDraft.ts index 7ed80cb262..b8b2fd546b 100644 --- a/apps/desktop/src/lib/settings/editorSettingsDraft.ts +++ b/apps/desktop/src/lib/settings/editorSettingsDraft.ts @@ -134,3 +134,12 @@ export function editorSettingsPatchFromDraft(draft: EditorSettingsDraft, base: E export function editorSettingsDraftChanged(draft: EditorSettingsDraft, base: EditorSettingsDraft): boolean { return EDITOR_SETTINGS_DRAFT_KEYS.some((key) => draftValueChanged(key, draft[key], base[key])); } + +// Closing the settings dialog (Escape, clicking outside, the X button, or the +// "Close" footer button) must never silently drop an unapplied draft — the +// dialog only persists shortcuts/sidebarActivation/etc. to the store on an +// explicit Apply. Route every close attempt through this check so an unsaved +// draft always surfaces a confirmation instead of vanishing. +export function shouldConfirmEditorSettingsDialogClose(nextOpen: boolean, hasUnsavedChanges: boolean): boolean { + return nextOpen === false && hasUnsavedChanges; +}