From bac62a27bf23bbd047e98d320dc467e45d7391f6 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Mon, 26 Oct 2026 11:36:46 +0100 Subject: [PATCH] :recycle: Use DS icons in the viewer comments filter (#12519) * :recycle: Use DS icons in the viewer comments filter The "Comments" filter trigger in the viewer header was a div with a click handler, and its menu lived inside it. It is now a button with aria-expanded and aria-haspopup, and the menu sits next to it in a wrapper, since a button cannot hold the menu entries. The arrow and the ticks use DS icons. The menu entries stay as they are. A visual test covers the filter closed and open. AI-assisted-by: claude-opus-5-5 * :recycle: Move viewer comments styles off the deprecated styles Spacing, sizes, borders, typographies and color aliases move to DS tokens and logical properties. The DS icons take their color from the text and no longer shrink, so the old icon placeholders go away. The DS body-small line-height is more compact, so "Comments" sits 1px higher. The refactor/ import stays only for the shared menu placeholders. The z-index values (1 and 10) have no DS token and keep a TODO. AI-assisted-by: claude-opus-5-5 --- .../ui/visual-specs/visual-viewer.spec.js | 32 +++++ frontend/src/app/main/ui/viewer/comments.cljs | 31 ++--- frontend/src/app/main/ui/viewer/comments.scss | 115 ++++++++---------- 3 files changed, 99 insertions(+), 79 deletions(-) diff --git a/frontend/playwright/ui/visual-specs/visual-viewer.spec.js b/frontend/playwright/ui/visual-specs/visual-viewer.spec.js index 9fb96ad4b3..95b3e58edc 100644 --- a/frontend/playwright/ui/visual-specs/visual-viewer.spec.js +++ b/frontend/playwright/ui/visual-specs/visual-viewer.spec.js @@ -190,3 +190,35 @@ test.describe("Thumbnails panel", () => { }); }); }); + +test.describe("Comments filter", () => { + test("Shows the comments filter closed and open", async ({ page }) => { + const viewerPage = new ViewerPage(page); + await viewerPage.setupLoggedInUser(); + await viewerPage.setupFileWithComments(); + await viewerPage.goToViewer({ + fileId: singleBoardFileId, + pageId: singleBoardPageId, + }); + await viewerPage.showComments(); + + const trigger = page.getByTestId("viewer-comments-dropdown"); + await expect(trigger).toBeVisible(); + await page.mouse.move(640, 600); + // Closed: the "Comments" trigger with its arrow. + await expect(trigger).toHaveScreenshot({ + maxDiffPixelRatio: 0, + threshold: 0, + }); + + await trigger.click(); + await expect(page.getByText("Show comments list")).toBeVisible(); + await page.mouse.move(640, 600); + // Open: the filter menu with the selected option ticked. + const menu = page.getByText("Show comments list").locator("../.."); + await expect(menu).toHaveScreenshot({ + maxDiffPixelRatio: 0, + threshold: 0, + }); + }); +}); diff --git a/frontend/src/app/main/ui/viewer/comments.cljs b/frontend/src/app/main/ui/viewer/comments.cljs index aac174d880..9e56e7f4e8 100644 --- a/frontend/src/app/main/ui/viewer/comments.cljs +++ b/frontend/src/app/main/ui/viewer/comments.cljs @@ -20,7 +20,7 @@ [app.main.store :as st] [app.main.ui.comments :as cmt] [app.main.ui.components.dropdown :refer [dropdown]] - [app.main.ui.icons :as deprecated-icon] + [app.main.ui.ds.foundations.assets.icon :as i :refer [icon*]] [app.main.ui.workspace.comments :as wc] [app.main.ui.workspace.viewport.utils :as utils] [app.util.dom :as dom] @@ -66,11 +66,17 @@ (parse-boolean))] (st/emit! (dcm/update-options {:show-sidebar? (not mode)})))))] - [:div {:class (stl/css :view-options) - :data-testid "viewer-comments-dropdown" - :on-click toggle-dropdown} - [:span {:class (stl/css :dropdown-title)} (tr "labels.comments")] - [:span {:class (stl/css :icon-dropdown)} deprecated-icon/arrow] + [:div {:class (stl/css :view-options-wrapper)} + [:button {:type "button" + :class (stl/css :view-options) + :data-testid "viewer-comments-dropdown" + :aria-expanded (deref show-dropdown?) + :aria-haspopup "menu" + :on-click toggle-dropdown} + [:span {:class (stl/css :dropdown-title)} (tr "labels.comments")] + [:> icon* {:icon-id i/arrow-down + :size "s" + :class (stl/css :icon-dropdown)}]] [:& dropdown {:show @show-dropdown? :on-close hide-dropdown} @@ -83,7 +89,7 @@ :on-click update-mode} [:span {:class (stl/css :label)} (tr "labels.show-all-comments")] (when (or (= :all cmode) (nil? cmode)) - [:span {:class (stl/css :icon)} deprecated-icon/tick])] + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :icon)}])] [:li {:class (stl/css-case :dropdown-element true @@ -92,8 +98,7 @@ :on-click update-mode} [:span {:class (stl/css :label)} (tr "labels.show-your-comments")] (when (= :yours cmode) - [:span {:class (stl/css :icon)} - deprecated-icon/tick])] + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :icon)}])] [:li {:class (stl/css-case :dropdown-element true @@ -102,8 +107,7 @@ :on-click update-mode} [:span {:class (stl/css :label)} (tr "labels.show-mentions")] (when (= :mentions cmode) - [:span {:class (stl/css :icon)} - deprecated-icon/tick])] + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :icon)}])] [:li {:class (stl/css :separator)}] @@ -114,8 +118,7 @@ :on-click update-show} [:span {:class (stl/css :label)} (tr "labels.hide-resolved-comments")] (when (= :pending cshow) - [:span {:class (stl/css :icon)} - deprecated-icon/tick])] + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :icon)}])] [:li {:class (stl/css :separator)}] @@ -126,7 +129,7 @@ :on-click update-options} [:span {:class (stl/css :label)} (tr "labels.show-comments-list")] (when show-sidebar? - [:span {:class (stl/css :icon)} deprecated-icon/tick])]]]])) + [:> icon* {:icon-id i/tick :size "s" :class (stl/css :icon)}])]]]])) (defn- update-thread-position diff --git a/frontend/src/app/main/ui/viewer/comments.scss b/frontend/src/app/main/ui/viewer/comments.scss index c254f0b9ba..09ed1d98f7 100644 --- a/frontend/src/app/main/ui/viewer/comments.scss +++ b/frontend/src/app/main/ui/viewer/comments.scss @@ -4,91 +4,71 @@ // // Copyright (c) KALEIDOS SUBSIDIARY SL -@use "ds/_borders.scss" as *; +// Only for the shared menu placeholders, which move to the DS with the +// menus and dropdowns. @use "refactor/common-refactor.scss" as deprecated; +@use "ds/_borders.scss" as *; +@use "ds/_sizes.scss" as *; +@use "ds/_utils.scss" as *; +@use "ds/typography.scss" as *; // COMMENT DROPDOWN ON HEADER +.view-options-wrapper { + position: relative; +} + .view-options { - @include deprecated.body-small-typography; + @include use-typography("body-small"); display: flex; align-items: center; - position: relative; - gap: deprecated.$s-4; - height: deprecated.$s-32; - padding: deprecated.$s-8; - border-radius: deprecated.$br-8; - background-color: var(--input-background-color); + border: none; + gap: var(--sp-xs); + block-size: $sz-32; + padding: var(--sp-s); + border-radius: $br-8; + background-color: var(--color-background-tertiary); cursor: pointer; } .dropdown { @extend %menu-dropdown; - right: deprecated.$s-2; - top: calc(deprecated.$s-2 + deprecated.$s-48); - width: deprecated.$s-272; - padding: deprecated.$s-6; + inset-inline-end: px2rem(2); + inset-block-start: px2rem(50); + inline-size: px2rem(272); + padding: px2rem(6); } .dropdown-title { - @include deprecated.body-small-typography; + @include use-typography("body-small"); flex-grow: 1; - color: var(--input-foreground-color-active); + color: var(--color-foreground-primary); } .label { flex-grow: 1; - color: var(--input-foreground-color); + color: var(--color-foreground-secondary); } .icon, .icon-dropdown { - @include deprecated.flex-center; - - height: 100%; - width: deprecated.$s-16; - - svg { - @extend %button-icon-small; - - stroke: var(--icon-foreground); - } -} - -.icon-dropdown svg { - transform: rotate(90deg); + flex-shrink: 0; + color: var(--color-foreground-secondary); } .dropdown-element { @extend %dropdown-element-base; - .icon { - @include deprecated.flex-center; - - height: 100%; - width: deprecated.$s-16; - - svg { - @extend %button-icon-small; - - stroke: var(--icon-foreground); - } - } - &:hover .label { - color: var(--input-foreground-color-active); + color: var(--color-foreground-primary); } } .dropdown-element.selected { .label { - color: var(--input-foreground-color-active); - } - - .icon svg { - stroke: var(--input-foreground-color); + color: var(--color-foreground-primary); } } @@ -96,36 +76,41 @@ 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); } // FLOATING COMMENT .viewer-comments-container { position: absolute; - top: 0; - left: 0; - width: 100%; - height: 100%; - z-index: deprecated.$z-index-1; + inset-block-start: 0; + inset-inline-start: 0; + inline-size: 100%; + block-size: 100%; + + // TODO: no DS z-index token for 1 (floating elements). + z-index: 1; } .threads { position: absolute; - top: 0; - left: 0; + inset-block-start: 0; + inset-inline-start: 0; } // COMMENT SIDEBAR .comments-sidebar { position: absolute; - right: 0; - top: deprecated.$s-44; - width: var(--right-sidebar-width); - height: calc(100vh - deprecated.$s-48); - z-index: deprecated.$z-index-10; - background-color: var(--panel-background-color); + inset-inline-end: 0; + inset-block-start: px2rem(44); + inline-size: var(--right-sidebar-width); + block-size: calc(100vh - #{$sz-48}); + background-color: var(--color-background-primary); + + // TODO: no DS z-index token for 10; it keeps the list above the viewer + // content. + z-index: 10; } .settings-bar-inside { @@ -133,5 +118,5 @@ } .comments-section { - background-color: var(--panel-background-color); + background-color: var(--color-background-primary); }