From ac52c419036a5226738c2594aa904fe4b106c683 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Wed, 28 Oct 2026 09:20:22 +0100 Subject: [PATCH] :recycle: Use DS icons in the comments panel filter (#12561) * :recycle: Use DS icons in the comments panel filter The comments panel filter (workspace and viewer sidebar) draws its trigger arrow and menu ticks with the DS icons, as the viewer comments filter does. The trigger button says whether its menu is open (aria-expanded, aria-haspopup). The menu items stay: they move with the menus group. The dropdown item placeholder styles any svg inside a span with a 12px box and a stroke color, so the tick icon class sets its 16px box and currentColor stroke. Nothing changes on screen. A visual test covers the trigger and the open menu. AI-assisted-by: claude-opus-5-5 * :recycle: Move comments panel styles closer to the DS The comments panel stylesheet resolves its legacy sizes, spacing, mixins and color aliases to DS values and tokens, with logical properties, and inlines the asset element placeholder of the filter trigger. refactor/ stays only for the dropdown placeholders, which move with the menus group. The legacy body-small typography becomes the DS one, which differs only in its line height (1.3 instead of 1.4): the menu labels sit 1px higher. The trigger looks the same at rest and on hover; the visual test now also covers the hover. AI-assisted-by: claude-opus-5-5 --- .../ui/visual-specs/workspace.spec.js | 26 +++++ .../src/app/main/ui/workspace/comments.cljs | 21 ++-- .../src/app/main/ui/workspace/comments.scss | 109 ++++++++++-------- 3 files changed, 104 insertions(+), 52 deletions(-) diff --git a/frontend/playwright/ui/visual-specs/workspace.spec.js b/frontend/playwright/ui/visual-specs/workspace.spec.js index 5893ae0d80..693d10ecff 100644 --- a/frontend/playwright/ui/visual-specs/workspace.spec.js +++ b/frontend/playwright/ui/visual-specs/workspace.spec.js @@ -1223,3 +1223,29 @@ test.describe("Assets tab controls", () => { await expect(modal).toHaveScreenshot(shot); }); }); + +test.describe("Comments panel filter", () => { + test("Shows the filter trigger and its menu", async ({ page }) => { + const shot = { maxDiffPixelRatio: 0, threshold: 0 }; + const workspace = new WorkspacePage(page); + await workspace.setupEmptyFile(); + await workspace.goToWorkspace(); + await workspace.showComments(); + + const trigger = workspace.rightSidebar.getByRole("button", { + name: "Show all comments", + }); + await expect(trigger).toBeVisible(); + await page.mouse.move(640, 600); + await expect(trigger).toHaveScreenshot(shot); + + await trigger.hover(); + await expect(trigger).toHaveScreenshot(shot); + + await trigger.click(); + const menu = page.locator("[class*='comment-mode-dropdown']"); + await expect(menu).toBeVisible(); + await page.mouse.move(640, 600); + await expect(menu).toHaveScreenshot(shot); + }); +}); diff --git a/frontend/src/app/main/ui/workspace/comments.cljs b/frontend/src/app/main/ui/workspace/comments.cljs index a6596bf3b6..7ab6ca7dda 100644 --- a/frontend/src/app/main/ui/workspace/comments.cljs +++ b/frontend/src/app/main/ui/workspace/comments.cljs @@ -17,10 +17,9 @@ [app.main.ui.comments :as cmt] [app.main.ui.components.dropdown :refer [dropdown]] [app.main.ui.context :as ctx] - [app.main.ui.ds.foundations.assets.icon :as i] + [app.main.ui.ds.foundations.assets.icon :refer [icon*] :as i] [app.main.ui.ds.product.empty-state :refer [empty-state*]] [app.main.ui.ds.product.panel-title :refer [panel-title*]] - [app.main.ui.icons :as deprecated-icon] [app.util.dom :as dom] [app.util.i18n :as i18n :refer [tr]] [rumext.v2 :as mf])) @@ -56,25 +55,29 @@ :on-click update-mode} [:span {:class (stl/css :label)} (tr "labels.show-all-comments")] - [:span {:class (stl/css :icon)} deprecated-icon/tick]] + [:span {:class (stl/css :icon)} + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :tick-icon)}]]] [:li {:class (stl/css-case :dropdown-item true :selected (= :yours cmode)) :data-value "yours" :on-click update-mode} [:span {:class (stl/css :label)} (tr "labels.show-your-comments")] - [:span {:class (stl/css :icon)} deprecated-icon/tick]] + [:span {:class (stl/css :icon)} + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :tick-icon)}]]] [:li {:class (stl/css-case :dropdown-item true :selected (= :mentions cmode)) :data-value "mentions" :on-click update-mode} [:span {:class (stl/css :label)} (tr "labels.show-mentions")] - [:span {:class (stl/css :icon)} deprecated-icon/tick]] + [:span {:class (stl/css :icon)} + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :tick-icon)}]]] [:li {:class (stl/css :separator)}] [:li {:class (stl/css-case :dropdown-item true :selected (= :pending cshow)) :on-click update-show} [:span {:class (stl/css :label)} (tr "labels.hide-resolved-comments")] - [:span {:class (stl/css :icon)} deprecated-icon/tick]]])) + [:span {:class (stl/css :icon)} + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :tick-icon)}]]]])) (mf/defc comments-sidebar* [{:keys [profiles threads page-id from-viewer]}] @@ -131,6 +134,9 @@ :on-close close-section}] [:button {:class (stl/css :mode-dropdown-wrapper) + :type "button" + :aria-haspopup "menu" + :aria-expanded options? :on-click toggle-mode-selector} [:span {:class (stl/css :mode-label)} @@ -138,7 +144,8 @@ (nil :all) (tr "labels.show-all-comments") :yours (tr "labels.show-your-comments") :mentions (tr "labels.show-mentions"))] - [:div {:class (stl/css :arrow-icon)} deprecated-icon/arrow]] + [:div {:class (stl/css :arrow-icon)} + [:> icon* {:icon-id i/arrow-down :size "s" :class (stl/css :arrow-svg)}]]] [:& dropdown {:show options? :on-close #(reset! state* false)} diff --git a/frontend/src/app/main/ui/workspace/comments.scss b/frontend/src/app/main/ui/workspace/comments.scss index b94930789d..9e7471e394 100644 --- a/frontend/src/app/main/ui/workspace/comments.scss +++ b/frontend/src/app/main/ui/workspace/comments.scss @@ -6,17 +6,22 @@ @use "ds/_sizes.scss" as *; @use "ds/_borders.scss" as *; +@use "ds/_utils.scss" as *; +@use "ds/typography.scss" as *; + +// refactor/ stays for the dropdown placeholders (%dropdown-wrapper, +// %dropdown-element-base), which move with the menus group. @use "refactor/common-refactor.scss" as deprecated; .comments-section { position: relative; - background-color: var(--panel-background-color); + background-color: var(--color-background-primary); display: grid; - grid-template-rows: deprecated.$s-40 deprecated.$s-48 1fr; + grid-template-rows: $sz-40 $sz-48 1fr; } .from-viewer { - padding: 0 deprecated.$s-8; + padding: 0 var(--sp-s); } .comments-title { @@ -24,53 +29,61 @@ } .mode-dropdown-wrapper { - @include deprecated.button-style; - @extend %asset-element; + @include use-typography("body-small"); - background-color: var(--color-background-tertiary); + --mode-dropdown-bg-color: var(--color-background-tertiary); + + border: none; + align-items: center; + color: var(--color-foreground-primary); + background-color: var(--mode-dropdown-bg-color); display: flex; - width: 100%; - max-width: deprecated.$s-256; - height: deprecated.$s-32; - padding: deprecated.$s-8; - border-radius: deprecated.$br-8; - margin: deprecated.$s-16 auto 0 auto; + inline-size: 100%; + max-inline-size: px2rem(256); + block-size: $sz-32; + padding: var(--sp-s); + border-radius: $br-8; + margin: var(--sp-l) auto 0 auto; cursor: pointer; position: relative; + + &:hover { + --mode-dropdown-bg-color: var(--color-background-quaternary); + } } .mode-label { - padding-right: 8px; + padding-inline-end: var(--sp-s); flex-grow: 1; display: flex; justify-content: flex-start; } .arrow-icon { - @include deprecated.flex-center; + display: flex; + justify-content: center; + align-items: center; + block-size: $sz-24; + inline-size: $sz-24; - height: deprecated.$s-24; - width: deprecated.$s-24; - - svg { - @extend %button-icon-small; - - transform: rotate(90deg); - stroke: var(--icon-foreground); + .arrow-svg { + flex-shrink: 0; + stroke-width: 1.33px; + color: var(--color-foreground-secondary); } } .comment-mode-dropdown { @extend %dropdown-wrapper; - top: deprecated.$s-92; - left: deprecated.$s-12; - max-width: deprecated.$s-256; - width: 100%; + inset-block-start: px2rem(92); + inset-inline-start: var(--sp-m); + max-inline-size: px2rem(256); + inline-size: 100%; } .viewer-dropdown { - left: deprecated.$s-8; + inset-inline-start: var(--sp-s); } .dropdown-item { @@ -79,35 +92,41 @@ justify-content: space-between; .icon { - @include deprecated.flex-center; + display: flex; + justify-content: center; + align-items: center; + block-size: $sz-24; + inline-size: $sz-24; - height: deprecated.$s-24; - width: deprecated.$s-24; - - svg { - @extend %button-icon-small; - - stroke: transparent; + // %dropdown-element-base styles any svg inside a span (12px box and a + // stroke color); the DS icon needs its 16px box and currentColor. + .tick-icon { + flex-shrink: 0; + inline-size: $sz-16; + block-size: $sz-16; + stroke: currentcolor; + stroke-width: 1.33px; + color: transparent; } } .label { - @include deprecated.body-small-typography; + @include use-typography("body-small"); } &:hover { - .icon svg { - stroke: transparent; + .tick-icon { + color: transparent; } } &.selected { .label { - color: var(--menu-foreground-color); + color: var(--color-foreground-primary); } - .icon svg { - stroke: var(--icon-foreground-hover); + .tick-icon { + color: var(--color-foreground-primary); } } } @@ -116,13 +135,13 @@ position: relative; block-size: var(--sp-xs); inline-size: calc(100% + var(--sp-s)); - border-top: $b-1 solid var(--color-background-quaternary); - left: calc(-1 * var(--sp-xs)); - margin-top: var(--sp-s); + border-block-start: $b-1 solid var(--color-background-quaternary); + inset-inline-start: calc(-1 * var(--sp-xs)); + margin-block-start: var(--sp-s); } .comments-section-content { - height: 100%; + block-size: 100%; overflow-y: auto; }