From a69d5d15d2ea614a91fef2d771c5ebee71d57bc1 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Fri, 30 Oct 2026 14:12:49 +0100 Subject: [PATCH] :bug: Wrap long shortcut key lists in the shortcuts panel (#12735) * :bug: Wrap long shortcut key lists in the shortcuts panel The Next board and Previous board rows of the viewer have five key combinations. Their keys ran past the panel edge and covered the label. Since #11081 the label sits inside a span with no flex rules, and the key list never wrapped. Now each combination is a group that never breaks, and the list wraps between groups. The comma stays at the end of its group and "or" leads the last one, so no line starts with a separator. The label keeps its width and the keys take the rest. Closes #12641 AI-assisted-by: claude-opus-5-5 * :recycle: Drop unused deprecated imports from the shortcuts styles shortcuts.scss no longer uses anything from refactor/ or ds/mixins. Remove both imports and use the typography module without a namespace. No visual change. AI-assisted-by: claude-opus-5-5 --- .../ui/visual-specs/workspace.spec.js | 22 ++++++++ frontend/src/app/main/ui/shortcuts.cljs | 50 ++++++++----------- frontend/src/app/main/ui/shortcuts.scss | 32 +++++++++--- 3 files changed, 67 insertions(+), 37 deletions(-) diff --git a/frontend/playwright/ui/visual-specs/workspace.spec.js b/frontend/playwright/ui/visual-specs/workspace.spec.js index df766736a0..a134ecf245 100644 --- a/frontend/playwright/ui/visual-specs/workspace.spec.js +++ b/frontend/playwright/ui/visual-specs/workspace.spec.js @@ -164,3 +164,25 @@ test.describe("Palette", () => { ).toBeVisible(); }); }); + +test.describe("Shortcuts panel", () => { + const shot = { maxDiffPixelRatio: 0, threshold: 0 }; + + test("Wraps long key lists between combinations", async ({ page }) => { + const workspace = new WorkspacePage(page); + await workspace.setupEmptyFile(); + await workspace.goToWorkspace(); + + await page.keyboard.press("Shift+?"); + const list = page.locator("[class*='shortcuts-list']"); + await expect(list).toBeVisible(); + await page.getByRole("textbox").first().fill("board"); + + const next = page.getByRole("listitem", { name: "Next board" }); + const prev = page.getByRole("listitem", { name: "Previous board" }); + await prev.scrollIntoViewIfNeeded(); + await page.mouse.move(640, 300); + await expect(next).toHaveScreenshot(shot); + await expect(prev).toHaveScreenshot(shot); + }); +}); diff --git a/frontend/src/app/main/ui/shortcuts.cljs b/frontend/src/app/main/ui/shortcuts.cljs index 4166314a37..64df9a10bf 100644 --- a/frontend/src/app/main/ui/shortcuts.cljs +++ b/frontend/src/app/main/ui/shortcuts.cljs @@ -539,39 +539,31 @@ (mf/defc shortcuts-keys* [{:keys [content command is-customized has-conflict? light-shortcut]}] - (let [managed-list (if (coll? content) - content - (conj () content)) - chars-list (map ds/split-sc managed-list) - last-element (last chars-list) - short-char-list (if (= 1 (count chars-list)) - chars-list - (drop-last chars-list)) - penultimate (last short-char-list)] + (let [managed-list (if (coll? content) + content + (conj () content)) + chars-list (map ds/split-sc managed-list) + last-index (dec (count chars-list)) + key-class (cond + has-conflict? (stl/css :conflict-key) + is-customized (stl/css :customized-key) + light-shortcut (stl/css :light-key))] + ;; Each combination is a group that never breaks; the list wraps + ;; between groups. The separator stays inside a group so no line + ;; starts with a comma. [:span {:class (stl/css :keys)} - (for [chars short-char-list] - [:* {:key (str/join chars)} + (for [[index chars] (d/enumerate chars-list)] + [:span {:key (dm/str index "-" (str/join chars)) + :class (stl/css :key-group)} + (when (and (pos? index) (= index last-index)) + [:span {:class (stl/css :space)} (tr "shortcuts.or")]) (for [char chars] [:> converted-chars* {:key (dm/str char "-" (name command)) :char char :command command - :class (cond - has-conflict? (stl/css :conflict-key) - is-customized (stl/css :customized-key) - light-shortcut (stl/css :light-key))}]) - - (when (not= chars penultimate) [:span {:class (stl/css :space)} ","])]) - (when (not= last-element penultimate) - [:* - [:span {:class (stl/css :space)} (tr "shortcuts.or")] - (for [char last-element] - [:> converted-chars* {:key (dm/str char "-" (name command)) - :char char - :command command - :class (cond - has-conflict? (stl/css :conflict-key) - is-customized (stl/css :customized-key) - light-shortcut (stl/css :light-key))}])])])) + :class key-class}]) + (when (< index (dec last-index)) + [:span {:class (stl/css :space)} ","])])])) (mf/defc shortcut-row-editable* [{:keys [elements custom-shortcuts command-translate conflicts section-key]}] @@ -863,7 +855,7 @@ :data-conflict (str has-conflict?) :aria-label command-translate :key command-translate} - [:span + [:span {:class (stl/css :command-label)} [:span {:class (stl/css-case :command-name true :not-customizable-label (not customizable?)) :id (dm/str command-translate "-label")} diff --git a/frontend/src/app/main/ui/shortcuts.scss b/frontend/src/app/main/ui/shortcuts.scss index 11eef8a482..dea29e9aba 100644 --- a/frontend/src/app/main/ui/shortcuts.scss +++ b/frontend/src/app/main/ui/shortcuts.scss @@ -4,12 +4,10 @@ // // Copyright (c) KALEIDOS SUBSIDIARY SL -@use "refactor/common-refactor.scss" as deprecated; @use "ds/_borders.scss" as *; @use "ds/_sizes.scss" as *; @use "ds/_utils.scss" as *; -@use "ds/mixins.scss" as *; -@use "ds/typography.scss" as t; +@use "ds/typography.scss" as *; .section { margin: 0; @@ -21,7 +19,7 @@ .section-title, .subsection-title { - @include t.use-typography("headline-small"); + @include use-typography("headline-small"); display: flex; align-items: center; @@ -60,6 +58,7 @@ display: flex; align-items: center; justify-content: space-between; + column-gap: var(--sp-s); inline-size: 100%; min-block-size: $sz-32; padding: px2rem(6); @@ -69,7 +68,7 @@ } .command-name { - @include t.use-typography("body-small"); + @include use-typography("body-small"); margin-inline-start: var(--sp-xxs); color: var(--color-foreground-primary); @@ -78,6 +77,16 @@ text-align: start; } +.command-label { + flex: 0 1 auto; +} + +// The keys take the space the label leaves and wrap between groups. +.shortcuts-name .shortcut-actions { + flex: 1 1 0; + justify-content: flex-end; +} + .not-customizable-label { padding-inline-start: px2rem(6); } @@ -163,13 +172,20 @@ .keys { display: flex; - justify-content: center; + flex-wrap: wrap; + justify-content: flex-end; + align-items: center; + gap: var(--sp-xxs); +} + +.key-group { + display: flex; align-items: center; gap: var(--sp-xxs); } .key { - @include t.use-typography("body-small"); + @include use-typography("body-small"); display: flex; justify-content: center; @@ -203,7 +219,7 @@ } .shortcut-placeholder { - @include t.use-typography("body-small"); + @include use-typography("body-small"); color: var(--color-foreground-secondary); }