diff --git a/frontend/playwright/ui/specs/escape-closes-panels.spec.js b/frontend/playwright/ui/specs/escape-closes-panels.spec.js new file mode 100644 index 0000000000..23f86539f8 --- /dev/null +++ b/frontend/playwright/ui/specs/escape-closes-panels.spec.js @@ -0,0 +1,138 @@ +import { test, expect } from "@playwright/test"; +import { WorkspacePage } from "../pages/WorkspacePage"; + +const VIEWPORT_POINTS = [ + [200, 200], + [400, 220], + [320, 420], +]; + +const openSearch = async (page) => { + await page.getByRole("button", { name: "Search" }).click(); + const findInput = page.getByRole("textbox", { name: /^Find/ }); + await expect(findInput).toBeVisible(); + return findInput; +}; + +const openShortcuts = async (page) => { + await page.keyboard.press("?"); + const searchInput = page.getByPlaceholder("Search shortcuts"); + await expect(searchInput).toBeVisible(); + return searchInput; +}; + +const saveVersionButton = (page) => + page.getByRole("button", { name: "Save version" }); + +const openHistoryAssertion = (page) => + expect(saveVersionButton(page)).toBeVisible(); + +test.beforeEach(async ({ page }) => { + await WorkspacePage.init(page); + const workspacePage = new WorkspacePage(page); + await workspacePage.setupEmptyFile(); + await workspacePage.mockRPC( + "get-file-snapshots?file-id=*", + "workspace/versions-snapshot-1.json", + ); + await workspacePage.goToWorkspace(); +}); + +test("Escape closes the layers search panel", async ({ page }) => { + const findInput = await openSearch(page); + + await page.keyboard.press("Escape"); + + await expect(findInput).toHaveCount(0); +}); + +test("Escape closes the search filters menu before the search panel", async ({ + page, +}) => { + const findInput = await openSearch(page); + await page.getByRole("button", { name: "Filter" }).click(); + const menuItem = page.getByText("Masks", { exact: true }); + await expect(menuItem).toBeVisible(); + + await page.keyboard.press("Escape"); + await expect(menuItem).toHaveCount(0); + await expect(findInput).toBeVisible(); + + await page.keyboard.press("Escape"); + await expect(findInput).toHaveCount(0); +}); + +test("Escape keeps the search panel open after clicking the viewport", async ({ + page, +}) => { + const workspacePage = new WorkspacePage(page); + const findInput = await openSearch(page); + + await workspacePage.clickAt(...VIEWPORT_POINTS[0]); + await page.keyboard.press("Escape"); + await expect(findInput).toBeVisible(); + + // The click moves focus out of the panel, so a real key press would + // never reach its handler. Dispatch Escape on the input instead, to + // exercise the case the hook exists for: the event reaches the panel + // although the user's last action was on the canvas. + await findInput.dispatchEvent("keydown", { + key: "Escape", + code: "Escape", + keyCode: 27, + bubbles: true, + }); + + await expect(findInput).toBeVisible(); +}); + +test("Escape closes the shortcuts panel right after it opens", async ({ + page, +}) => { + const searchInput = await openShortcuts(page); + + await page.keyboard.press("Escape"); + + await expect(searchInput).toHaveCount(0); +}); + +test("Escape closes History opened with its shortcut", async ({ page }) => { + await page.keyboard.press("ControlOrMeta+Alt+h"); + await openHistoryAssertion(page); + + await page.keyboard.press("Escape"); + + await expect(saveVersionButton(page)).toHaveCount(0); +}); + +test("Escape closes History opened from the main menu", async ({ page }) => { + await page.getByRole("button", { name: "Main menu" }).click(); + await page.getByText("file").last().click(); + await page.locator("#file-menu-show-version-history").click(); + await openHistoryAssertion(page); + + await page.keyboard.press("Escape"); + + await expect(saveVersionButton(page)).toHaveCount(0); +}); + +test("Escape finishes a path drawn with the shortcuts panel open", async ({ + page, +}) => { + const workspacePage = new WorkspacePage(page); + const searchInput = await openShortcuts(page); + await page.getByRole("button", { name: "BASICS" }).click(); + + await page.keyboard.press("p"); + for (const [x, y] of VIEWPORT_POINTS) { + await workspacePage.clickAt(x, y); + } + await page.keyboard.press("Escape"); + + // The shortcuts panel replaces the layers tree, so the new path is + // observed through the design tab of the selected shape. + await expect( + page.getByRole("region", { name: "Stroke section" }), + ).toBeVisible(); + await expect(searchInput).toBeVisible(); +}); diff --git a/frontend/src/app/main/data/workspace/shortcuts.cljs b/frontend/src/app/main/data/workspace/shortcuts.cljs index 17246e5889..b31fdd95d3 100644 --- a/frontend/src/app/main/data/workspace/shortcuts.cljs +++ b/frontend/src/app/main/data/workspace/shortcuts.cljs @@ -19,6 +19,7 @@ [app.main.data.workspace.comments :as dwcm] [app.main.data.workspace.drawing :as dwd] [app.main.data.workspace.drawing.common :as dwdc] + [app.main.data.workspace.history :as dwh] [app.main.data.workspace.layers :as dwly] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.shape-layout :as dwsl] @@ -655,7 +656,8 @@ :subsections [:panels] :section [:workspace] :fn #(emit-when-no-readonly - (dw/toggle-layout-flag :document-history))} + (-> (dwh/initialize-history) + (vary-meta assoc ::ev/origin "workspace:shortcuts")))} :toggle-colorpalette {:tooltip (ds/alt "P") :label (fn [] (tr "shortcuts.toggle-colorpalette")) diff --git a/frontend/src/app/main/ui/hooks.cljs b/frontend/src/app/main/ui/hooks.cljs index 5bc113bab9..ca3663ada9 100644 --- a/frontend/src/app/main/ui/hooks.cljs +++ b/frontend/src/app/main/ui/hooks.cljs @@ -16,6 +16,7 @@ [app.main.store :as st] [app.util.dom :as dom] [app.util.dom.dnd :as dnd] + [app.util.keyboard :as kbd] [app.util.storage :as storage] [app.util.timers :as ts] [app.util.webapi :as wapi] @@ -597,3 +598,44 @@ (rx/dispose! sub)))) [rowref limit]))) + +(defn use-escape-to-close + "Returns an `on-key-down` handler that closes a panel with Escape. + + The panel only closes while the user is working in it: the latest + pointer press or focus change must have landed inside the element + that owns the handler, or that element must hold focus when it + mounts. Focus left behind in a panel after the user moved on to the + canvas does not count, so Escape keeps its usual meaning there. + + The event is never stopped: the global Escape shortcut still runs + and finishes any in-progress operation. + + `dismiss-nested`, when given, is called first; it returns true when + it consumed the Escape (an open menu, an inline rename), and then + the panel stays open." + ([on-close] + (use-escape-to-close on-close nil)) + ([on-close dismiss-nested] + (let [last-target-ref (mf/use-ref nil)] + (mf/with-effect [] + (let [track (fn [event] + (mf/set-ref-val! last-target-ref (dom/get-target event)))] + ;; A panel that autofocuses its input on open has already + ;; received its `focusin` before this effect runs. + (mf/set-ref-val! last-target-ref (dom/get-active)) + (.addEventListener js/document "pointerdown" track true) + (.addEventListener js/document "focusin" track true) + #(do (.removeEventListener js/document "pointerdown" track true) + (.removeEventListener js/document "focusin" track true)))) + + (mf/use-fn + (mf/deps on-close dismiss-nested) + (fn [event] + (let [panel (dom/get-current-target event) + last-target (mf/ref-val last-target-ref)] + (when (and (kbd/esc? event) + (dom/child? last-target panel) + (not (and (fn? dismiss-nested) (dismiss-nested)))) + (dom/blur! (dom/get-target event)) + (on-close)))))))) diff --git a/frontend/src/app/main/ui/workspace/main_menu.cljs b/frontend/src/app/main/ui/workspace/main_menu.cljs index 5a64cf76c9..79d2b80a20 100644 --- a/frontend/src/app/main/ui/workspace/main_menu.cljs +++ b/frontend/src/app/main/ui/workspace/main_menu.cljs @@ -23,6 +23,7 @@ [app.main.data.shortcuts :as scd] [app.main.data.workspace :as dw] [app.main.data.workspace.comments :as dwcm] + [app.main.data.workspace.history :as dwh] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.mcp :as mcp] [app.main.data.workspace.shortcuts :as sc] @@ -617,9 +618,9 @@ on-show-version-history (mf/use-fn - (mf/deps file-id) (fn [_] - (st/emit! (dw/toggle-layout-flag :document-history)))) + (st/emit! (-> (dwh/initialize-history) + (vary-meta assoc ::ev/origin "workspace:menu"))))) on-show-version-history-key-down (mf/use-fn diff --git a/frontend/src/app/main/ui/workspace/sidebar/layers.cljs b/frontend/src/app/main/ui/workspace/sidebar/layers.cljs index 5525364ee3..f841fbee91 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/layers.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/layers.cljs @@ -386,6 +386,21 @@ (st/emit! dw/close-layers-search) (st/emit! (dw/open-layers-search :find {:force? true})))))) + close-search + (mf/use-fn + #(st/emit! dw/close-layers-search)) + + dismiss-filters-menu + (mf/use-fn + (mf/deps show-menu? hide-menu) + (fn [] + (when ^boolean show-menu? + (hide-menu) + true))) + + on-search-key-down + (hooks/use-escape-to-close close-search dismiss-filters-menu) + remove-filter (mf/use-fn (fn [event] @@ -587,7 +602,8 @@ #(mf/html (if show-search? [:* - [:div {:class (stl/css :tool-window-bar)} + [:div {:class (stl/css :tool-window-bar) + :on-key-down on-search-key-down} [:> search-bar* {:input-ref search-input-ref :class (stl/css :search-item) :on-change update-search-text @@ -612,7 +628,8 @@ :on-click toggle-search :icon i/close}]] - [:div {:class (stl/css :replace-wrapper)} + [:div {:class (stl/css :replace-wrapper) + :on-key-down on-search-key-down} (when ^boolean find-replace-mode? [:div {:class (stl/css :replace-row)} [:> input* {:type "text" diff --git a/frontend/src/app/main/ui/workspace/sidebar/shortcuts.cljs b/frontend/src/app/main/ui/workspace/sidebar/shortcuts.cljs index 016b9b8ba5..4d262f12be 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/shortcuts.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/shortcuts.cljs @@ -23,6 +23,7 @@ [app.main.ui.components.search-bar :refer [search-bar*]] [app.main.ui.ds.foundations.assets.icon :as i :refer [icon*]] [app.main.ui.ds.product.panel-title :refer [panel-title*]] + [app.main.ui.hooks :as hooks] [app.main.ui.shortcuts :as ss] [app.util.dom :as dom] [app.util.i18n :refer [tr]] @@ -60,7 +61,9 @@ filter-term* (mf/use-state "") filter-term (deref filter-term*) - close-fn #(st/emit! (dw/toggle-layout-flag :shortcuts)) + close-fn (mf/use-fn #(st/emit! (dw/toggle-layout-flag :shortcuts))) + + on-key-down (hooks/use-escape-to-close close-fn) {:keys [all-shortcuts all-sc-names all-sub-names all-section-names]} (ss/build-all-shortcuts workspace-shortcuts dashboard-shortcuts viewer-shortcuts) @@ -127,7 +130,8 @@ (mf/use-fn #(st/emit! (rt/nav :settings-shortcuts {} {::rt/new-window true})))] - [:div {:class (dm/str class " " (stl/css :shortcuts))} + [:div {:class (dm/str class " " (stl/css :shortcuts)) + :on-key-down on-key-down} [:> panel-title* {:class (stl/css :shortcuts-title) :text (tr "shortcuts.title") :on-close close-fn}]