github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 05:50:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Show the top stroke width in the old render (#12093)

Browse Source 
* 🐛 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

* 🐛 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
This commit is contained in:
Eva Marco 2026-10-06 13:33:36 +02:00 committed by GitHub
parent d70b754652
commit dacfac17bb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 214 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

122
frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js
View File

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

83
frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs
View File

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

39
frontend/test/frontend_tests/ui/stroke_menu_test.cljs
View File

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