From 48a5875a7161a03fc16c4ce723bf4227f22a70f1 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Mon, 26 Oct 2026 12:48:43 +0100 Subject: [PATCH] :recycle: Use DS icon buttons in the viewer board navigation (#12527) * :recycle: Use DS icon buttons in the viewer board navigation The previous, next and first board buttons of the viewer become secondary DS icon buttons with the DS arrow and reload icons. They keep their size and the viewer background at rest; on hover they take the DS secondary look. The first board button had no accessible name; it is now labelled. The position classes move to the tooltip wrappers, so the fullscreen and comments sidebar offsets still apply. New key: viewer.go-to-first-board (en, es). A visual test covers the buttons at rest and on hover. AI-assisted-by: claude-opus-5-5 * :recycle: Move viewer styles off the deprecated styles Spacing, sizes, borders, typographies and color aliases move to DS tokens and logical properties, and the file no longer imports refactor/. The section scrollbar uses the DS custom-scrollbar mixin; the text selection and placeholder styles that %new-scrollbar gave to the comments layer stay, with DS tokens of the same value. An invalid padding-right with four values, which the browser ignored, is gone. The DS body-small typography differs from the old one only in a more compact line-height, so the board counter sits 1px higher. The z-index values (10, 2) have no DS token and keep a TODO. The .viewer-header, .not-fixed, .fixed and .frame-children rules are not used by this module and are commented out with a TODO. AI-assisted-by: claude-opus-5-5 * :fire: Remove unused styles from the viewer The SCSS refactor of the viewer left rules commented out with a TODO because the module does not use them, or because they target an svg inside a DS icon that is the svg itself. CSS modules scope the classes to this module and no other source uses them, so they go: .viewer-header, .not-fixed, .fixed, .frame-children g. No visual change. AI-assisted-by: claude-opus-5-5 --- .../ui/visual-specs/visual-viewer.spec.js | 44 +++++ frontend/src/app/main/ui/viewer.cljs | 41 ++-- frontend/src/app/main/ui/viewer.scss | 177 +++++++++--------- frontend/translations/en.po | 4 + frontend/translations/es.po | 4 + 5 files changed, 165 insertions(+), 105 deletions(-) diff --git a/frontend/playwright/ui/visual-specs/visual-viewer.spec.js b/frontend/playwright/ui/visual-specs/visual-viewer.spec.js index 95b3e58edc..65ffbe4af8 100644 --- a/frontend/playwright/ui/visual-specs/visual-viewer.spec.js +++ b/frontend/playwright/ui/visual-specs/visual-viewer.spec.js @@ -222,3 +222,47 @@ test.describe("Comments filter", () => { }); }); }); + +test.describe("Board navigation", () => { + test("Shows the previous, next and first board buttons", async ({ page }) => { + const viewerPage = new ViewerPage(page); + await viewerPage.setupLoggedInUser(); + await viewerPage.setupFileWithMultipleBoards(); + await viewerPage.goToViewer({ + fileId: "dd5cc0bb-91ff-81b9-8004-77df9cd3edb0", + pageId: "dd5cc0bb-91ff-81b9-8004-77df9cd3edb3", + }); + + const next = page.getByRole("button", { name: "Next" }); + await expect(next).toBeVisible(); + await page.mouse.move(640, 300); + // Wait for the slide-in transition of the controls. + await page.waitForTimeout(800); + // First board: the next and first board buttons. + await expect(page).toHaveScreenshot({ + maxDiffPixelRatio: 0, + threshold: 0, + }); + + await next.hover(); + await expect(next).toHaveScreenshot({ + maxDiffPixelRatio: 0, + threshold: 0, + }); + + await next.click(); + const previous = page.getByRole("button", { name: "Previous" }); + await previous.hover(); + await expect(previous).toHaveScreenshot({ + maxDiffPixelRatio: 0, + threshold: 0, + }); + + const first = page.locator("[class*='viewer-bottom'] button"); + await first.hover(); + await expect(first).toHaveScreenshot({ + maxDiffPixelRatio: 0, + threshold: 0, + }); + }); +}); diff --git a/frontend/src/app/main/ui/viewer.cljs b/frontend/src/app/main/ui/viewer.cljs index f3cb52db0b..2422d347c2 100644 --- a/frontend/src/app/main/ui/viewer.cljs +++ b/frontend/src/app/main/ui/viewer.cljs @@ -22,9 +22,10 @@ [app.main.refs :as refs] [app.main.store :as st] [app.main.ui.context :as ctx] + [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] + [app.main.ui.ds.foundations.assets.icon :as i] [app.main.ui.ds.product.loader :refer [loader*]] [app.main.ui.hooks :as hooks] - [app.main.ui.icons :as deprecated-icon] [app.main.ui.modal :refer [modal-container*]] [app.main.ui.viewer.comments :refer [comments-layer comments-sidebar*]] [app.main.ui.viewer.header :as header] @@ -99,23 +100,33 @@ go-first-frame (mf/use-fn #(st/emit! dv/select-first-frame))] [:* (when (pos? index) - [:button {:class (stl/css-case :viewer-go-prev true - :left-bar left-bar) - :on-click go-prev-frame - :aria-label (tr "labels.previous")} - deprecated-icon/arrow]) + [:> icon-button* {:variant "secondary" + :class (stl/css :viewer-go-button) + :tooltip-class (stl/css-case :viewer-go-prev true + :left-bar left-bar) + :tooltip-placement "right" + :aria-label (tr "labels.previous") + :icon i/arrow-left + :on-click go-prev-frame}]) (when (< (+ index 1) num-frames) - [:button {:class (stl/css-case :viewer-go-next true - :comment-sidebar comment-sidebar - :right-bar right-bar) - :on-click go-next-frame - :aria-label (tr "labels.next")} - deprecated-icon/arrow]) + [:> icon-button* {:variant "secondary" + :class (stl/css :viewer-go-button) + :tooltip-class (stl/css-case :viewer-go-next true + :comment-sidebar comment-sidebar + :right-bar right-bar) + :tooltip-placement "left" + :aria-label (tr "labels.next") + :icon i/arrow-right + :on-click go-next-frame}]) [:div {:class (stl/css-case :viewer-bottom true :left-bar left-bar)} - [:button {:on-click go-first-frame - :class (stl/css :reset-button)} - deprecated-icon/reload] + [:> icon-button* {:variant "secondary" + :class (stl/css :reset-button) + :tooltip-class (stl/css :reset-button-wrapper) + :tooltip-placement "top-right" + :aria-label (tr "viewer.go-to-first-board") + :icon i/reload + :on-click go-first-frame}] [:span {:class (stl/css :counter)} (str/join " / " [(+ index 1) num-frames])] [:span]]])) diff --git a/frontend/src/app/main/ui/viewer.scss b/frontend/src/app/main/ui/viewer.scss index d727764672..bef25833d1 100644 --- a/frontend/src/app/main/ui/viewer.scss +++ b/frontend/src/app/main/ui/viewer.scss @@ -4,12 +4,16 @@ // // 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 *; .viewer-layout { - height: 100vh; + block-size: 100vh; display: grid; - grid-template-rows: deprecated.$s-48 auto; + grid-template-rows: $sz-48 auto; grid-template-columns: 1fr; user-select: none; } @@ -18,41 +22,41 @@ overflow: hidden; grid-row: 2 / span 1; display: grid; - grid-template-rows: deprecated.$s-252 auto; + grid-template-rows: $sz-252 auto; grid-template-columns: 1fr; - background-color: var(--viewer-background-color); + background-color: var(--color-background-secondary); } .empty-state { - @include deprecated.body-small-typography; + @include use-typography("body-small"); - color: var(--empty-message-foreground-color); + color: var(--color-foreground-secondary); display: grid; place-items: center; - height: 100%; - width: 100%; -} - -.viewer-header { - grid-row: 1 / span 1; + block-size: 100%; + inline-size: 100%; } .inspect-layout { display: grid; - grid-template-rows: deprecated.$s-48 auto; + grid-template-rows: $sz-48 auto; grid-template-columns: 1fr; - height: 100vh; - margin-top: 0; + block-size: 100vh; + margin-block-start: 0; user-select: none; } .thumbnails-close { - @include deprecated.button-style; - + border: none; + background: none; + cursor: pointer; grid-row: 1 / span 2; grid-column: 1 / span 1; - z-index: deprecated.$z-index-10; - background-color: var(--overlay-color); + + // TODO: no DS z-index token for 10; it keeps the backdrop above the + // viewer content (z-index 2), level with the thumbnails panel. + z-index: 10; + background-color: var(--color-overlay-default); } .thumbnails-close.invisible { @@ -60,98 +64,104 @@ } .viewer-section { - @extend %new-scrollbar; + @include custom-scrollbar($include-selection: false, $include-placeholder: false); grid-row: 1 / span 2; grid-column: 1 / span 1; display: flex; align-items: center; flex-wrap: wrap; - height: calc(100vh - deprecated.$s-48); + block-size: calc(100vh - #{$sz-48}); overflow: auto; + + // The comments layer renders inside the section: its text selection and + // placeholders keep the styles they had. + ::selection { + background: var(--color-accent-background-select); + color: var(--color-static-white); + } + + ::placeholder { + @include use-typography("body-small"); + + color: var(--color-foreground-secondary); + } } .inspect-layout .viewer-section { flex-wrap: nowrap; - margin-top: 0; - height: 100%; + margin-block-start: 0; + block-size: 100%; overflow: hidden; } +// Tooltip wrappers of the previous and next buttons. .viewer-go-prev, .viewer-go-next { - @extend %button-secondary; - @include deprecated.flex-center; - position: absolute; - right: deprecated.$s-8; - height: deprecated.$s-64; - width: deprecated.$s-32; - top: calc(50vh - deprecated.$s-32); - z-index: deprecated.$z-index-2; - background-color: var(--viewer-controls-background-color); + inset-inline-end: var(--sp-s); + inset-block-start: calc(50vh - #{$sz-32}); + + // TODO: no DS z-index token for 2; it keeps the controls above the + // viewer section. + z-index: 2; transition: transform 400ms ease 300ms; +} - svg { - @extend %button-icon; - - stroke: var(--icon-foreground); - } +.viewer-go-button { + --button-height: #{$sz-64}; + --button-bg-color: var(--color-background-primary); + --button-border-color: var(--color-background-tertiary); } .viewer-go-next.comment-sidebar { - right: deprecated.$s-280; + inset-inline-end: #{px2rem(280)}; } .viewer-go-prev { - left: deprecated.$s-8; - right: unset; - - svg { - transform: rotate(180deg); - } + inset-inline-start: var(--sp-s); + inset-inline-end: unset; } .viewer-bottom { position: fixed; - bottom: 0; + inset-block-end: 0; display: flex; align-items: flex-start; justify-content: space-between; - width: 100%; - height: deprecated.$s-40; - padding-right: 0 deprecated.$s-8 deprecated.$s-40 deprecated.$s-8; + inline-size: 100%; + block-size: $sz-40; transition: transform 400ms ease 300ms; - z-index: deprecated.$z-index-2; + + // TODO: no DS z-index token for 2; it keeps the controls above the + // viewer section. + z-index: 2; pointer-events: none; } +.reset-button-wrapper { + margin-inline-start: var(--sp-s); +} + .reset-button { - @extend %button-secondary; - @include deprecated.flex-center; + --button-width: #{$sz-28}; + --button-bg-color: var(--color-background-primary); + --button-border-color: var(--color-background-tertiary); - height: deprecated.$s-32; - width: deprecated.$s-28; - margin-left: deprecated.$s-8; - background-color: var(--viewer-controls-background-color); pointer-events: all; - - svg { - @extend %button-icon; - - stroke: var(--icon-foreground); - } } .counter { - @include deprecated.flex-center; - @include deprecated.body-small-typography; + @include use-typography("body-small"); - border-radius: deprecated.$br-8; - width: deprecated.$s-64; - height: deprecated.$s-32; - color: var(--viewer-thumbnails-control-foreground-color); - background-color: var(--viewer-controls-background-color); + display: flex; + justify-content: center; + align-items: center; + border-radius: $br-8; + inline-size: $sz-64; + block-size: $sz-32; + color: var(--color-foreground-secondary); + background-color: var(--color-background-primary); } .viewer-wrapper { @@ -169,8 +179,8 @@ .viewer-overlay-background { position: absolute; - top: 0; - left: 0; + inset-block-start: 0; + inset-inline-start: 0; &.visible { background-color: rgb(0 0 0 / 0.2); @@ -185,24 +195,11 @@ clip-path: inset(0 0 0 0); grid-column: 1 / 1; grid-row: 1 / 1; - - .not-fixed { - position: absolute; - } - - .fixed { - position: fixed; - pointer-events: none; - - .frame-children g { - pointer-events: auto; - } - } } /** FULLSCREEN */ [data-fullscreen="true"] .viewer-bottom { - transform: translateY(deprecated.$s-40); + transform: translateY($sz-40); } [data-force-visible="true"] .viewer-bottom { @@ -210,20 +207,20 @@ } [data-fullscreen="true"] .viewer-go-next { - transform: translateX(deprecated.$s-40); + transform: translateX($sz-40); } [data-fullscreen="true"] .viewer-go-prev { - transform: translateX(-#{deprecated.$s-40}); + transform: translateX(-#{$sz-40}); } [data-fullscreen="true"] .viewer-content { - transform: translateY(-#{deprecated.$s-48}); - height: 100vh; + transform: translateY(-#{$sz-48}); + block-size: 100vh; } [data-fullscreen="true"] .viewer-section { - height: 100vh; + block-size: 100vh; } [data-force-visible="true"] .viewer-go-next { diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 63fcfeddb3..dde95de91f 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -6091,6 +6091,10 @@ msgstr "No boards found on the page." msgid "viewer.frame-not-found" msgstr "Board not found." +#: src/app/main/ui/viewer.cljs:127 +msgid "viewer.go-to-first-board" +msgstr "Go to the first board" + #: src/app/main/ui/viewer/header.cljs:341 msgid "viewer.header.comments-section" msgstr "Comments (%s)" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 195a1912eb..1bd162954c 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -6172,6 +6172,10 @@ msgstr "No se ha encontrado ningún tablero." msgid "viewer.frame-not-found" msgstr "No se encuentra el tablero." +#: src/app/main/ui/viewer.cljs:127 +msgid "viewer.go-to-first-board" +msgstr "Ir al primer tablero" + #: src/app/main/ui/viewer/header.cljs:341 msgid "viewer.header.comments-section" msgstr "Comentarios (%s)"