diff --git a/frontend/playwright/ui/pages/ShortcutsPage.js b/frontend/playwright/ui/pages/ShortcutsPage.js index d434f9fb5b..578311570e 100644 --- a/frontend/playwright/ui/pages/ShortcutsPage.js +++ b/frontend/playwright/ui/pages/ShortcutsPage.js @@ -305,6 +305,13 @@ export class ShortcutsPage extends BaseWebSocketPage { }); } + async confirmImportApply() { + const applyButton = this.page.getByRole("button", { + name: /apply/i, + }); + await applyButton.click(); + } + async getExportedJson() { const [download] = await Promise.all([ this.page.waitForEvent("download"), diff --git a/frontend/playwright/ui/specs/shortcuts.spec.js b/frontend/playwright/ui/specs/shortcuts.spec.js index afb8fbbd7f..c0b487b883 100644 --- a/frontend/playwright/ui/specs/shortcuts.spec.js +++ b/frontend/playwright/ui/specs/shortcuts.spec.js @@ -77,10 +77,12 @@ test.describe("Shortcut Import", () => { workspace: { "align-bottom": "ctrl+y" }, }); + await shortcutsPage.confirmImportApply(); + await shortcutsPage.expectShortcutCustomized("Align bottom"); - await shortcutsPage.searchForShortcut("Align bottom"); await shortcutsPage.clickTab("Personalized"); + await shortcutsPage.searchForShortcut("Align bottom"); await shortcutsPage.expectShortcutVisible("Align bottom"); }); @@ -116,6 +118,8 @@ test.describe("Shortcut Import", () => { dashboard: { "toggle-theme": "alt+m" }, }); + await shortcutsPage.confirmImportApply(); + await expect( page.getByRole("alert").filter({ hasText: /Invalid data/i }), ).not.toBeVisible(); @@ -144,6 +148,8 @@ test.describe("Shortcut Import", () => { }, }); + await shortcutsPage.confirmImportApply(); + await shortcutsPage.expectShortcutCustomized("Align bottom"); await shortcutsPage.expandSubsection("Alignment"); @@ -183,6 +189,8 @@ test.describe("Shortcut Export", () => { workspace: { "align-bottom": "" }, }); + await shortcutsPage.confirmImportApply(); + await shortcutsPage.searchForShortcut("Align bottom"); await shortcutsPage.expectShortcutDisabled("Align bottom"); @@ -228,11 +236,36 @@ test.describe("Shortcut Reset", () => { await shortcutsPage.importShortcuts({ workspace: { "align-bottom": "ctrl+y" }, }); + await shortcutsPage.confirmImportApply(); await shortcutsPage.expectShortcutCustomized("Align bottom"); await shortcutsPage.restoreAllShortcuts(); await shortcutsPage.expectShortcutNotCustomized("Align bottom"); }); + + test("Reset button shows pending message and requires save to apply", async ({ + page, + }) => { + const shortcutsPage = new ShortcutsPage(page); + await shortcutsPage.goToShortcuts(); + + await shortcutsPage.expandSubsection("Alignment"); + await shortcutsPage.clickEditShortcut("Align bottom"); + await shortcutsPage.pressKey("Control+y"); + await shortcutsPage.saveShortcut(); + await shortcutsPage.expectShortcutCustomized("Align bottom"); + + await shortcutsPage.clickEditShortcut("Align bottom"); + await shortcutsPage.resetShortcut("Align bottom"); + + await expect( + page.getByText(/If you save, this shortcut will return/i), + ).toBeVisible(); + + await shortcutsPage.saveShortcut(); + + await shortcutsPage.expectShortcutNotCustomized("Align bottom"); + }); }); test.describe("Shortcut Persistence", () => { @@ -315,6 +348,7 @@ test.describe("Shortcut Round-Trip", () => { await shortcutsPage.expectShortcutNotCustomized("Align bottom"); await shortcutsPage.importShortcuts(exported); + await shortcutsPage.confirmImportApply(); await shortcutsPage.expectShortcutCustomized("Align bottom"); const reExported = await shortcutsPage.getExportedJson(); diff --git a/frontend/src/app/main/ui/settings/import_shortcuts_diff_modal.cljs b/frontend/src/app/main/ui/settings/import_shortcuts_diff_modal.cljs index a5a78c3331..bb0c8a81af 100644 --- a/frontend/src/app/main/ui/settings/import_shortcuts_diff_modal.cljs +++ b/frontend/src/app/main/ui/settings/import_shortcuts_diff_modal.cljs @@ -12,7 +12,6 @@ [app.common.i18n :refer [tr]] [app.main.data.dashboard.shortcuts :as dsc] [app.main.data.modal :as modal] - [app.main.data.shortcuts :as ds] [app.main.data.viewer.shortcuts :as vsc] [app.main.data.workspace.path.shortcuts :as psc] [app.main.data.workspace.shortcuts :as wsc] @@ -30,7 +29,7 @@ :dashboard dsc/shortcuts :viewer vsc/shortcuts}) -(defn- compute-diff +(defn compute-diff [imported-shortcuts custom-shortcuts] (let [contexts (filter #(contains? imported-shortcuts %) [:workspace :dashboard :viewer])] (mapcat diff --git a/frontend/src/app/main/ui/shortcuts.cljs b/frontend/src/app/main/ui/shortcuts.cljs index d6eb9fac9a..668d697e11 100644 --- a/frontend/src/app/main/ui/shortcuts.cljs +++ b/frontend/src/app/main/ui/shortcuts.cljs @@ -390,7 +390,7 @@ on-reset-shortcut (mf/use-fn (mf/deps effective-section-key command-info) - (fn [shortcut-key] + (fn [_] (let [original-cmd (:original-command command-info)] (reset! reset-pending* true) (reset! recorded-command* original-cmd) diff --git a/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs b/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs index 0def5fb1c9..8bbf5af6b2 100644 --- a/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs +++ b/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs @@ -1,6 +1,7 @@ (ns frontend-tests.ui.settings-shortcuts-test (:require [app.main.data.profile :as du] + [app.main.ui.settings.import-shortcuts-diff-modal :as diff-modal] [app.main.ui.settings.restore-shortcuts-modal :as restore-modal] [app.main.ui.settings.shortcuts :as sut] [app.main.ui.shortcuts :as ui-shortcuts] @@ -150,6 +151,48 @@ (t/is (= "ctrl+up" (get customs :move-to-top)) "Last imported entry with duplicate command should survive"))))) +(t/deftest compute-diff-shows-new-shortcut-changes + (let [imported {:workspace {:add-comment "alt+c"}} + diff (diff-modal/compute-diff imported {})] + (t/is (= 1 (count diff))) + (let [entry (first diff)] + (t/is (= :workspace (:context entry))) + (t/is (= :add-comment (:key entry))) + (t/is (= "alt+c" (:imported entry))) + (t/is (not= "alt+c" (:current entry)))))) + +(t/deftest compute-diff-shows-conflict-as-disabled + (let [imported {:workspace {:select-all "ctrl+shift+a"}} + diff (diff-modal/compute-diff imported {})] + (t/is (pos? (count diff))) + (let [select-all-entry (first (filter #(= :select-all (:key %)) diff))] + (t/is (= :workspace (:context select-all-entry))) + (t/is (= "ctrl+shift+a" (:imported select-all-entry)))))) + +(t/deftest compute-diff-shows-empty-string-as-disabled-shortcut + (let [imported {:workspace {:escape ""}} + current {} + diff (diff-modal/compute-diff imported current)] + (t/is (pos? (count diff))) + (let [entry (first (filter #(= :escape (:key %)) diff))] + (t/is (= :workspace (:context entry))) + (t/is (= "" (:imported entry)))))) + +(t/deftest compute-diff-excludes-unchanged-shortcuts + (let [imported {:workspace {:escape ""}} + current {:workspace {:escape ""}} + diff (diff-modal/compute-diff imported current)] + (t/is (empty? diff)))) + +(t/deftest compute-diff-detects-multiple-contexts + (let [imported {:workspace {:add-comment "alt+c"} + :viewer {:next-frame "right"}} + diff (diff-modal/compute-diff imported {})] + (t/is (pos? (count diff))) + (let [contexts (set (map :context diff))] + (t/is (contains? contexts :workspace)) + (t/is (contains? contexts :viewer))))) + (t/deftest extract-shortcut-keys-uses-correct-context-override (let [customs {:workspace {:undo "shift+z" :move-nodes "shift+m"}