From dacfac17bb630a301a4ec715c417fe0d54ed70f5 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Tue, 6 Oct 2026 13:33:36 +0200 Subject: [PATCH] :bug: Show the top stroke width in the old render (#12093) * :bug: Show the top stroke width in the old render With the old render, the stroke width input showed "Mixed" when the four sides had different widths or tokens. The old render only draws the top side, and the input must show it as the global stroke width. Move the input value and token logic into two pure functions. When per side strokes are disabled, both return the top side: its width and its token, or no token when the top has none. Closes #12063 AI-assisted-by: claude-opus-5-5 * :bug: Add e2e tests for the stroke width input in both renders Cover the global stroke width input when the four sides differ. With the new render it shows Mixed. With the old render it shows the top side: its width, or its token when the top has one. The old render cannot create per side data from the UI, and switching the render reloads the page and drops the test state. The tests patch the existing stroke token fixture instead, appending the per side fields to a rectangle and a board. Relates to #12063 AI-assisted-by: claude-opus-5-5 --- .../ui/specs/tokens/stroke-per-side.spec.js | 122 ++++++++++++++++++ .../sidebar/options/rows/stroke_row.cljs | 83 +++++++----- .../frontend_tests/ui/stroke_menu_test.cljs | 39 ++++++ 3 files changed, 214 insertions(+), 30 deletions(-) diff --git a/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js b/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js index 61952665bc..50c558478d 100644 --- a/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js +++ b/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js @@ -1,3 +1,4 @@ +import { readFile } from "node:fs/promises"; import { test, expect } from "@playwright/test"; import { BaseWebSocketPage } from "../../pages/BaseWebSocketPage"; import { WasmWorkspacePage } from "../../pages/WasmWorkspacePage"; @@ -54,6 +55,76 @@ async function unfoldStrokeWidth(page, workspace) { await unfoldTokenType(workspace.tokensSidebar, "stroke width"); } +const STROKE_TOKEN_FILE = "workspace/get-file-layout-stroke-token-json"; +const PER_SIDE_RECT = "Per side rect"; +const PER_SIDE_TOKEN_BOARD = "Per side token board"; + +// Transit fields appended at the end of a shape, so the earlier "^N" cache +// references stay valid; when a key repeats, the last value wins. +function perSideStrokeFields(name, top, appliedTokens = []) { + const stroke = [ + "^ ", + ...["~:stroke-style", "~:solid", "~:stroke-alignment", "~:inner"], + ...["~:stroke-color", "#000000", "~:stroke-opacity", 1], + ...["~:stroke-width", top, "~:stroke-width-top", top], + ...["~:stroke-width-right", 4, "~:stroke-width-bottom", 8], + ...["~:stroke-width-left", 16], + ]; + return [ + "~:name", + name, + "~:strokes", + [stroke], + "~:applied-tokens", + ["^ ", ...appliedTokens], + ]; +} + +// Serves the stroke token file with a rectangle and a board whose sides +// have different widths (top 2 or 20, right 4, bottom 8, left 16). Only the +// board has a token, `width-big`, on its top side. Both are top-level +// layers, so the layers panel shows them without unfolding anything. +async function mockPerSideStrokeFile(workspace) { + await workspace.mockGetFile(STROKE_TOKEN_FILE); + + const payload = JSON.parse( + await readFile(`playwright/data/${STROKE_TOKEN_FILE}`, "utf-8"), + ); + const [pageData] = Object.values(payload["~:data"]["~:pages-index"]); + const objects = pageData["~:objects"]["~#penpot/objects-map/v2"]; + + const patch = (id, fields) => { + const shape = objects[id]; + const extra = JSON.stringify(fields).slice(1, -1); + objects[id] = `${shape.slice(0, -2)},${extra}]]`; + }; + + patch( + "~u8506e3f3-e05b-807c-8007-6ceac380abc1", + perSideStrokeFields(PER_SIDE_RECT, 2), + ); + patch( + "~u6c65a5dc-fb40-8072-8007-6ce644905054", + perSideStrokeFields(PER_SIDE_TOKEN_BOARD, 20, [ + "~:stroke-width-top", + "width-big", + ]), + ); + + // The last matching route wins over the one set by mockGetFile. + await workspace.page.route(/get\-file\?/, (route) => + route.fulfill({ + status: 200, + contentType: "application/transit+json", + body: JSON.stringify(payload), + }), + ); +} + +function globalWidthInput(page) { + return strokeRow(page).getByRole("textbox", { name: GLOBAL }); +} + test.describe("Tokens: stroke per side", () => { test("per-side toggle is hidden when the feature flag is off", async ({ page, @@ -583,7 +654,58 @@ test.describe("Tokens: stroke per side (inspect)", () => { }); }); +test.describe("Tokens: stroke per side, global width input", () => { + test("shows Mixed for different sides with the new renderer", async ({ + page, + }) => { + const workspace = new WasmWorkspacePage(page); + await workspace.mockConfigFlags([ + "enable-stroke-per-side", + "enable-feature-token-input", + ]); + await workspace.setupEmptyFile(); + await mockPerSideStrokeFile(workspace); + await workspace.goToWorkspace(); + await workspace.waitForFirstRender(); + + await workspace.clickLeafLayer(PER_SIDE_RECT); + await expect(globalWidthInput(page)).toHaveAttribute("placeholder", MIXED); + + await workspace.clickLeafLayer(PER_SIDE_TOKEN_BOARD); + await expect(globalWidthInput(page)).toHaveAttribute("placeholder", MIXED); + }); +}); + test.describe("Tokens: stroke per side (non wasm)", () => { + test("the global width input shows the top side without the new renderer", async ({ + page, + }) => { + const workspace = new WorkspacePage(page); + await workspace.mockConfigFlags([ + "enable-stroke-per-side", + "enable-feature-token-input", + ]); + await workspace.setupEmptyFile(); + await mockPerSideStrokeFile(workspace); + await workspace.goToWorkspace(); + + // Top 2, other sides 4, 8 and 16: the old render draws the top side, + // so the input shows it instead of Mixed. + await workspace.clickLeafLayer(PER_SIDE_RECT); + await expect(globalWidthInput(page)).toHaveValue("2"); + await expect(globalWidthInput(page)).not.toHaveAttribute( + "placeholder", + MIXED, + ); + + // Only the top side has a token: the input shows that token. + await workspace.clickLeafLayer(PER_SIDE_TOKEN_BOARD); + await expect( + strokeRow(page).getByRole("button", { name: "width-big" }), + ).toHaveCount(1); + await expect(globalWidthInput(page)).toHaveCount(0); + }); + test("per-side toggle is disabled without the new renderer", async ({ page, }) => { diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs index dd774a3390..278626cab8 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs @@ -28,6 +28,51 @@ [app.util.i18n :as i18n :refer [tr]] [rumext.v2 :as mf])) +(defn stroke-width-input-value + "Value for the global stroke width input: the width when every side is + equal, `:multiple` otherwise. When per side strokes are disabled (old + render) the input shows the top side, the one that render draws." + [stroke per-side-disabled] + (let [width (:stroke-width stroke) + top (d/nilv (:stroke-width-top stroke) width) + right (d/nilv (:stroke-width-right stroke) width) + bottom (d/nilv (:stroke-width-bottom stroke) width) + left (d/nilv (:stroke-width-left stroke) width)] + (cond + per-side-disabled + top + + (= top right bottom left) + width + + :else + :multiple))) + +(defn stroke-width-input-token + "Token for the global stroke width input: the token when every side has + the same one, `:multiple` when only some sides have a token, nil when no + side has one. When per side strokes are disabled (old render) the input + shows the top side token, like `stroke-width-input-value` does." + [applied-tokens per-side-disabled] + (let [top (get applied-tokens :stroke-width-top) + right (get applied-tokens :stroke-width-right) + bottom (get applied-tokens :stroke-width-bottom) + left (get applied-tokens :stroke-width-left) + per-side [top right bottom left] + per-side-has-token? (some some? per-side)] + (cond + per-side-disabled + top + + (and (every? some? per-side) (apply = per-side)) + top + + per-side-has-token? + :multiple + + :else + nil))) + (mf/defc stroke-row* [{:keys [index stroke @@ -114,35 +159,15 @@ (not per-side-disabled) (true? (get stroke-per-side [ids index]))) - all-sides-equal? - (mf/with-memo [stroke] - (let [width (:stroke-width stroke) - top (d/nilv (:stroke-width-top stroke) width) - right (d/nilv (:stroke-width-right stroke) width) - bottom (d/nilv (:stroke-width-bottom stroke) width) - left (d/nilv (:stroke-width-left stroke) width)] - (= top right bottom left))) + stroke-width-value + (mf/with-memo [stroke per-side-disabled] + (stroke-width-input-value stroke per-side-disabled)) - show-multiple-placeholder? (or per-side? (not all-sides-equal?)) + show-multiple-placeholder? (or per-side? (= :multiple stroke-width-value)) applied-token-width - (mf/with-memo [applied-tokens] - (let [top (get applied-tokens :stroke-width-top) - right (get applied-tokens :stroke-width-right) - bottom (get applied-tokens :stroke-width-bottom) - left (get applied-tokens :stroke-width-left) - - per-side [top right bottom left] - per-side-has-token? (some some? per-side)] - (cond - (and (every? some? per-side) (apply = per-side)) - (first per-side) - - per-side-has-token? - :multiple - - :else - nil))) + (mf/with-memo [applied-tokens per-side-disabled] + (stroke-width-input-token applied-tokens per-side-disabled)) per-side-toggle-label (if per-side-disabled @@ -409,9 +434,7 @@ :placeholder (if show-multiple-placeholder? (tr "settings.multiple") "--") - :value (if all-sides-equal? - (:stroke-width stroke) - :multiple)}] + :value stroke-width-value}] [:> select* {:default-selected (d/name stroke-alignment) :options stroke-alignment-options :variant "icon-only" @@ -445,7 +468,7 @@ :size "s"}] [:> deprecated-input/numeric-input* {:value (if show-multiple-placeholder? nil - stroke-width) + stroke-width-value) :min 0 :placeholder (if show-multiple-placeholder? (tr "settings.multiple") diff --git a/frontend/test/frontend_tests/ui/stroke_menu_test.cljs b/frontend/test/frontend_tests/ui/stroke_menu_test.cljs index e219333e99..b6fcc7407e 100644 --- a/frontend/test/frontend_tests/ui/stroke_menu_test.cljs +++ b/frontend/test/frontend_tests/ui/stroke_menu_test.cljs @@ -6,8 +6,10 @@ (ns frontend-tests.ui.stroke-menu-test (:require + [app.common.types.token :as cto] [app.config :as cf] [app.main.ui.workspace.sidebar.options.menus.stroke :as stroke] + [app.main.ui.workspace.sidebar.options.rows.stroke-row :as stroke-row] [clojure.test :as t :include-macros true])) (def ^:private per-side-flags (conj cf/flags :stroke-per-side)) @@ -54,3 +56,40 @@ (t/testing "disabled when the feature flag is off" (with-redefs [cf/flags (disj cf/flags :stroke-per-side)] (t/is (false? (stroke/per-side-stroke-enabled? true)))))) + +(def ^:private per-side-stroke + {:stroke-width 2 + :stroke-width-top 2 + :stroke-width-right 4 + :stroke-width-bottom 8 + :stroke-width-left 16}) + +(t/deftest stroke-width-input-value-test + (t/testing "shows the width when every side is equal" + (t/is (= 5 (stroke-row/stroke-width-input-value {:stroke-width 5} false))) + (t/is (= 5 (stroke-row/stroke-width-input-value + (stroke/stroke-width-all-attrs 5) false)))) + + (t/testing "shows multiple when the sides differ" + (t/is (= :multiple (stroke-row/stroke-width-input-value per-side-stroke false)))) + + (t/testing "shows the top side when per side strokes are disabled" + (t/is (= 2 (stroke-row/stroke-width-input-value per-side-stroke true))))) + +(t/deftest stroke-width-input-token-test + (t/testing "shows the token when every side has the same one" + (t/is (= "sw" (stroke-row/stroke-width-input-token + (zipmap cto/per-side-stroke-width-keys (repeat "sw")) false)))) + + (t/testing "shows multiple when only some sides have the token" + (t/is (= :multiple (stroke-row/stroke-width-input-token + {:stroke-width-top "sw"} false)))) + + (t/testing "shows no token when no side has one" + (t/is (nil? (stroke-row/stroke-width-input-token {} false)))) + + (t/testing "shows the top side token when per side strokes are disabled" + (t/is (= "sw" (stroke-row/stroke-width-input-token + {:stroke-width-top "sw" :stroke-width-left "other"} true))) + (t/is (nil? (stroke-row/stroke-width-input-token + {:stroke-width-left "other"} true)))))