♻️ Use DS icons in the viewer comments filter (#12519)

* ♻️ 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

* ♻️ 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
This commit is contained in:
Eva Marco 2026-10-26 11:36:46 +01:00 committed by GitHub
parent ef17aea39e
commit bac62a27bf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 99 additions and 79 deletions

View File

@ -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,
});
});
});

View File

@ -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

View File

@ -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);
}