✨ Add skills for the DS migration epic (#12739)

* ✨ Add skills for the DS migration epic

Two skills that drive the DS migration (#12375) one area at a time, so
anyone working with an agent follows the same flow.

refactor-scss moves one .scss off frontend/resources/styles/common/
refactor/ to the DS tokens, mixins and px2rem as a pure refactor, with
a self-review for the issues reviewers flag and a zero-tolerance visual
check.

migrate-ds-area takes one area of a group issue (buttons and icons,
selects, ...): claim it in the group's PRs table, add a visual test,
change the controls, run refactor-scss, compare before and after with
the bundled diff scripts, then open the draft PR and post the QA
comment. It also lists devenv traps seen while migrating the first
areas.

AI-assisted-by: claude-opus-5-5

* ✨ Add the claim rules to the migrate-ds-area skill

The group issue body is saved whole, so two people claiming at the same
time can lose a row. The skill now rereads the body after saving and
puts the row back if it is gone. A claim with no PR for 7 days is free
once its owner has been asked in a comment and did not answer.

AI-assisted-by: claude-opus-5-5
This commit is contained in:
Eva Marco 2026-11-04 15:29:07 +01:00 committed by GitHub
parent f532b8c12e
commit 2f060e9baf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 809 additions and 0 deletions

View File

@ -0,0 +1,330 @@
---
name: migrate-ds-area
description: Migrate one Penpot UI area to the Design System components for one group of the DS migration epic (#12375): buttons and icons (icon-button*, button*, icon*), selects (select*) and later groups. Moves the area's SCSS off the deprecated styles, proves every change with a zero-tolerance visual test, then opens the draft PR and updates the group issue (PRs table row, QA comment). Use when the user asks to migrate an area or module to the DS, continue a DS migration group, or names an area of a group issue.
---
# Skill: migrate-ds-area
Move one UI area (one `.cljs` + `.scss` pair, or a few related modules)
from legacy components and raw HTML to the Design System components,
prove with pixels what changed, and ship it as a draft PR that QA can
test.
The epic is #12375. Each control type is a group with its own issue
(buttons and icons #12376, selects #12377, radios #12378, …). The group
issue holds the plan (order, waiting areas), the changes QA should
expect, and the PRs table where people claim areas. Read it first.
## Input
- The group issue (e.g. `#12376`) and the area: a module path (e.g.
`dashboard/fonts`) or an area named in the group issue.
- No area given: take the next unclaimed area in the group issue's plan
order, and say which one.
## Ground rules
- Never `git push`. The user pushes; then you open the PR.
- PRs go to `develop`; branches start from `origin/develop`.
- **Stay in the group.** Only the controls of this group change. In the
buttons/icons group, selects, radios, numeric/text inputs, checkboxes,
menus (`%menu-*`, `%dropdown-*`) and modal structure (`%modal-*`) stay;
a checkbox tick or an input label icon stays legacy.
- **Accept the DS look.** Do not add CSS to imitate legacy sizes or
colors. Do keep positions: icons and labels must not move unless the
DS size forces it, and then say so.
- No negative margins to compensate; use positive DS spacing or
`px2rem()`.
- **Gaps in the DS wait.** An area that needs an icon or a feature the
DS lacks keeps its legacy control; say so in the PR.
- Skip legacy shared components that another group will replace
(`context_menu_a11y`, `forms`, `select`, `dropdown`, `radio_buttons`,
`search_bar`, `title_bar`, …): they wait for that group's cleanup,
where they are updated if still used or deleted if not.
- Debug-only UI gets no QA comment and no compare image.
## Devenv
Run every test, lint and `pnpm` command inside the workspace container,
never on the host (a host `pnpm` relinks `node_modules` and breaks the
container's watcher). The container is `penpot-devenv-ws<N>-main`, where
`<N>` is the workspace number (`ws0` is the main repo; `wsN` lives in
`~/.penpot/penpot_workspaces/wsN`). Write output to a file, then read it:
```bash
C=penpot-devenv-ws1-main # your workspace
docker exec -u penpot -w /home/penpot/penpot/frontend $C \
bash -lc '<command>' > <scratch>/out.txt 2>&1
```
When the app does not behave as the code says, check the build first:
- **Stale build after a branch switch** ("is not a function", or a page
with empty `stl/css` classes): `touch` the stale `.cljs`/`.scss`, wait
for the compiled file under `frontend/resources/public/` to update,
hard-reload.
- **A whole area unstyled:** a new `.scss` added after the watcher
started is missing from `main.css` (grep `main.css` for the module
prefix), or `main.css` was written corrupted after many `.scss`
changed at once. Touch a `.scss`; if the module is still missing,
restart the watcher in the workspace's tmux session (`penpot:0`,
`./scripts/watch app`).
- **Blank app after switching between `develop` and `staging`:**
dependencies differ. Run `pnpm install --frozen-lockfile` in the
container and restart the watcher.
## Workflow
### 1. Claim the area and branch
1. Read the group issue: plan, waiting areas, expected changes, PRs table.
2. Add a row to the PRs table with no PR number, the user's @login as
Owner and the state `Claimed` (`gh issue view/edit <n> --body-file`).
Read the body right before editing, and read it again after saving:
when two people edit at the same time, GitHub keeps only the last
edit, so a row can vanish. Put yours back if it is gone.
Skip an area someone else already claimed, unless the claim has had
no PR for 7 days and its owner did not answer a comment asking.
3. `git switch -c <user>-ds-<group>-<area> origin/develop`.
4. Survey the area: every control of the group in the `.cljs`, the
legacy icons, the placeholders and mixins the `.scss` uses, the
`div`/`span` with `:on-click`.
### 2. Visual baseline
Add a `test.describe("<Area> controls", …)` block to the matching spec
in `frontend/playwright/ui/visual-specs/` (project `ds`, dark theme).
Put it next to the blocks of the same screen, not always at the end of
the file: two PRs that both append at the end conflict.
- Every shot: `toHaveScreenshot({ maxDiffPixelRatio: 0, threshold: 0 })`.
- Show every changed control at rest, on hover and open or pressed.
Capture the element or the smallest stable container.
- Locators must work before and after the change: text, role, testid,
`[class*='…']`, or `a.or(b)` when the element type changes (raw
`[class*='menu-btn']` before, `getByRole("button", { name: "Options" })`
after). Scope `.or()` to the area so it cannot match elsewhere.
- Missing data? Add a fixture under `frontend/playwright/data/` and mock
it (`dashboardPage.mockRPC(...)`, `DashboardPage.mockConfigFlags` for
flags). Watch for expired or inconsistent fixtures (dates in the past,
wrong file for a mock).
- Capture with `--update-snapshots=all`:
```bash
npx playwright test --project=ds <spec> -g "<describe>" --update-snapshots=all
```
Plain `--update-snapshots` keeps a snapshot within the project's 0.5%
tolerance, so a real diff looks like 0.
- Copy the snapshots (`<spec>.spec.js-snapshots/<Describe>-*`) to
`<scratch>/diffs/<area>/before/` and look at them: the state you
wanted must really be on screen.
### 3. Controls commit
#### Buttons and icons
| Legacy | DS |
|---|---|
| icon-only raw button | `[:> icon-button* {:variant "ghost" :aria-label (tr …) :icon i/x :on-click f}]` (always pass `:variant`; `"action"` for 24px icon toggles) |
| pressed/selected | `:aria-pressed (boolean x)` |
| text button | `[:> button* {:variant "primary"/"secondary"/"ghost"/"destructive" :type "button"} label]` |
| `[:input {:type "button"}]` | `button*` |
| modal close | `icon-button*` ghost, `:aria-label (tr "labels.close")`, `:tooltip-class` carrying the corner position |
| legacy icon outside a button | `[:> icon* {:icon-id i/x :class …}]` |
- Not every raw button changes: rows, tabs, swatches, avatar widgets,
full-height segments, round custom buttons and link-style text buttons
stay raw (their legacy icon still changes). List the kept ones in the
PR.
- A `div`/`span` with `:on-click` in the area becomes a `[:button]`
(with `aria-expanded`/`aria-haspopup` when it opens something). An
`[:a]` without `href` stays; list it in the PR.
- **Tooltip wrapper.** `icon-button*` renders inside a tooltip trigger
wrapper. Position (`position: absolute`, grid placement, margins
against siblings) goes on `:tooltip-class`; look (`--button-height`,
colors) goes on `:class`.
- **Tooltip at the right edge.** Small icon buttons at the right end of
a sidebar row or title bar get `:tooltip-placement "top-left"`; list
it as an expected change.
- **Sizes through custom properties:** `--button-height: #{px2rem(64)}` /
`--button-width`. Sass does not evaluate functions inside custom
properties without `#{}`.
- **A grown `button*` keeps its label at the start**: add
`justify-content: center` when it is wider than its content.
- **Icon colors:** DS icons use `stroke: currentColor`. Set `color` on
the icon class, not `stroke`/`fill`. A legacy `color: transparent` left
on the button or wrapper makes the DS icon invisible: drop it. A class
that redefines the color variable on the icon breaks the hover that
changes it on the button: define the variable on the button.
- **Every DS icon needs a color of its own** (on its class or a parent):
one that inherits none shows in the devenv debug yellow.
- **Icon sizes:** `"s"` draws 12px inside a 16px box (compensate the
margin by 2px per side to keep the drawing in place); `"m"` 16px; `"l"`
24px; `"xl"` 32px.
- **Stroke width:** DS icons keep the DS stroke width. Never set
`stroke-width` on a DS icon to match the legacy one; a thinner or
thicker line is part of the DS look.
- **Legacy placeholders that style `svg` descendants** (e.g.
`%dropdown-element-base` → `span svg { 12px box; stroke }`) override a
DS icon placed inside them: give the icon class its own box
(`inline-size`/`block-size` 16px) and `stroke: currentcolor` with more
specificity.
- **Flex shrink:** a DS svg inside a small flex button can shrink and be
cut; add `flex-shrink: 0` to the icon class.
- **Rotated legacy arrows:** prefer `i/arrow-left`/`-right`/`-up`/`-down`
over rotating `i/arrow` (same path), unless a rotation class drives the
open/closed state.
- **Menus anchored by the button size** move when the button grows; open
them from the top (`inset-block-start: calc(100% + var(--sp-xs))`) or
fix the context menu offset, and say so.
#### Selects
`[:& select {:default-value v :options [{:value :label}] :on-change f}]`
→ `[:> select* {:default-selected <id> :options <memoized vector of {:id :label}> :on-change f}]`
(`app.main.ui.ds.controls.select`).
- **Ids are non-blank strings.** A blank id shows a dimmed "--", and an
id that matches no option falls back to the first one. Give mixed,
none and auto their own ids (`"multiple"`, `"none"`, `"auto"`) and map
them back in the handler. Use `""` only where "--" is the wanted look.
- **Convert the value.** The legacy select ran `d/read-string`, so
handlers got keywords, numbers or uuids; `select*` passes the `:id`
string. Convert with `keyword`, `d/parse-integer` or `uuid/parse`.
- **Memoized options, live selection.** `select*` resets its selection
when `:default-selected` or the `:options` identity changes. Build
options with `mf/with-memo` and `mapv` (the schema wants a non-empty
vector) and pass `:default-selected` from the live value, never a
constant.
- **Never pass nil.** In dev builds a present prop with value nil
(`:class`, `:wrapper-class`, `:disabled`, `:default-selected`) throws
`invalid props on component select*`. Use `(boolean x)`, `d/nilv`, or
add the prop only when it has a value.
- **Props the legacy select ignored stay out**: `select*` passes rest
props to its button, so they would reach the DOM.
- **Classes:** `:wrapper-class` for layout, `:class` for look.
- **Portal:** add `:has-portal true` only where the visual test shows the
list cut at the bottom of a panel, and say so in the QA comment.
- **Locators:** the button is a `combobox` with no accessible name; use
`getByRole("combobox").filter({ hasText: "Linear" })` (it matches the
legacy select too) and `getByRole("option", { name: … })`.
- **Keyboard:** the arrows no longer change the value while the list is
closed. It is an expected change, already listed in #12377.
#### All groups
- New labels: add the key to `en.po` and `es.po` by hand, in sorted
position, with a `#:` reference to the cljs line. Never run the
translations sync.
- The `.scss` only gets what the new controls need in this commit.
- Some e2e specs are not Prettier-clean on develop: never run
`prettier --write` on a whole spec; format only your block.
Then wait for the watcher (grep the compiled JS/CSS under
`frontend/resources/public/` for the change), recapture with `=all`,
copy to `after/`, diff:
```bash
SK=.agents/skills/migrate-ds-area/scripts
python3 -I $SK/diff.py <before> <after> # per-file pixel count + bbox
python3 -I $SK/trip.py <b> <a> <out.png> <scale> # before | after | red diff
```
`trip.py` pads images of different sizes, so a taller "after" still
renders.
Explain every diff. Fix what is not a DS-look change (moved icons, cut
icons, lost hover color). Rerun a shot that changes on unchanged code
before trusting it; flaky spots seen so far: the invite modal textarea
border, the search input caret, the user account page, the path handler
menu trigger.
Lint, then commit (read `mem:workflow/creating-commits` first):
```bash
npx prettier --check <spec> <fixtures>; npx stylelint <scss>; clj-kondo --lint <cljs>; cljfmt check <cljs>
git add -f <cljs> <scss> <po> <spec> <fixtures> # -f: visual-specs/ is gitignored
./scripts/check-commit
```
A plain `git add` with an ignored path in the list adds nothing; always
`-f`, and check `git show --stat`. Never commit the screenshots.
Subject: `:recycle: Use DS <controls> in <area>`. A pre-existing bug
found and fixed on the way goes in its own `:bug:` commit.
### 4. SCSS commit
Run the `refactor-scss` skill on every `.scss` the PR touched (only
those), in its own commit. Keep `refactor/` only for component-family
placeholders (`%modal-*`, `%menu-*`, `%input-*`, `%checkbox-*`) and say
why in a comment above the `@use`. Unused selectors are deleted, not
commented out (list them in the commit body). Typography without an
exact match uses the closest DS typography. Run the skill's
**Self-review** before committing: no bare element selectors (add
classes in the `.cljs`), no copied properties that do nothing, z-index
tokens by purpose. Compare against the controls commit: it should be
0 px; explain anything else (typically a typography or border change).
For files that several modules share or that are huge, stop and propose
a separate PR instead.
### 5. Hand off, PR and tracking
1. Report to the user: commits, visible changes, kept raw controls,
pre-existing bugs, and the compare image path
(`<scratch>/diffs/<area>/compare.png`, before | after | diff). Give
the push command: `git push -u origin <branch>` (the branch tracks
`origin/develop`, so a bare `git push` is refused).
2. After the user pushes, check `git rev-parse HEAD origin/<branch>`,
then (read `mem:workflow/creating-prs` first):
```bash
gh pr create --repo penpot/penpot --draft --base develop --head <branch> \
--title "<commit subject>" --body-file <body> \
--label "WG adoption" --label "no issue required" \
--milestone <group issue milestone> --project Main
gh pr edit <n> --repo penpot/penpot --remove-label "needs triage"
```
Never add `no changelog` to a PR: that label goes on issues.
Body: AI note, `Part of #<group> (DS migration epic #12375).`, a
short What / How, the kept raw controls, an empty `## Screenshots`
when something is visible (the user attaches the image). Never
`Closes #<group>`. Keep it short: reviewers find long PRs hard to read.
3. In the group issue's PRs table, fill your row:
`| #<n> | <area>: <controls> (<visible changes>) | @<owner> | Draft |`.
4. Post a QA comment on the group issue: `**#<n>: <area>**`, *Where* (how
to reach it), *Expected changes* (every visible change; QA treats
anything not listed as a bug), *No other change* (what still works
the same).
Do not mark the draft ready or ask for review: the user decides when.
### 6. Review fixes
When the reviewer comments on a PR:
1. Fix each comment in a new commit on the branch (never amend pushed
commits). A comment you do not apply gets a short reason the user can
post, with an image when it is visual.
2. Rerun the area's visual spec before and after the fix with `=all`.
Build a compare image for every visible change and give the user its
full path, saying which PR it goes to.
3. Update everything the fix makes stale, in the same turn: the QA
comment (*Expected changes* must list the new visible changes), the
PRs table row, and the PR body.
4. When working on several PRs in one go, report per PR: branch, new
commit, what changed, image path. Never mix two branches in one list.
5. Give the push command per branch. If a rebase is coming soon, say
that pushing now costs one extra CI run; the user decides.
6. After a round of review fixes, check whether the same issue exists in
the user's other open PRs of the group, and fix it there too.
### 7. After merges
Whenever PRs merge (or go from draft to ready), sync the state column of
the group's PRs table with GitHub for **every** row, not only your own:
read each row's PR (`gh pr view <n> --json state,isDraft`) and set
Merged / In review / Draft. The table drifts when other people merge.
## Output
- Branch with 2 commits (controls, SCSS) plus any `:bug:` commit.
- Visual spec block with zero-tolerance shots.
- Compare image in the scratch folder.
- After the push: draft PR, row and QA comment in the group issue.

View File

@ -0,0 +1,25 @@
"""Pixel diff of two folders of PNG snapshots with the same file names.
Usage: python3 -I diff.py <before-dir> <after-dir>
Prints, per file, the number of differing pixels and their bounding box,
or the two sizes when they differ.
"""
import os
import sys
from PIL import Image, ImageChops
before, after = sys.argv[1], sys.argv[2]
for name in sorted(os.listdir(before)):
other = os.path.join(after, name)
if not os.path.exists(other):
print(name, "missing in", after)
continue
a = Image.open(os.path.join(before, name)).convert("RGB")
b = Image.open(other).convert("RGB")
if a.size != b.size:
print(name, "size", a.size, b.size)
continue
diff = ImageChops.difference(a, b)
count = sum(1 for p in diff.getdata() if p != (0, 0, 0))
print(name, count, diff.getbbox())

View File

@ -0,0 +1,33 @@
"""Before | after | red diff image, one row per snapshot.
Usage: python3 -I trip.py <before-dir> <after-dir> <out.png> <scale>
Both folders hold PNGs with the same names (crop them first if needed).
"""
import os
import sys
from PIL import Image, ImageChops
before, after, out, scale = sys.argv[1], sys.argv[2], sys.argv[3], int(sys.argv[4])
rows = []
for name in sorted(os.listdir(before)):
a = Image.open(os.path.join(before, name)).convert("RGB")
b = Image.open(os.path.join(after, name)).convert("RGB")
w, h = max(a.width, b.width), max(a.height, b.height)
pad_a = Image.new("RGB", (w, h), (200, 200, 200)); pad_a.paste(a, (0, 0))
pad_b = Image.new("RGB", (w, h), (200, 200, 200)); pad_b.paste(b, (0, 0))
mask = ImageChops.difference(pad_a, pad_b).convert("L").point(lambda v: 255 if v else 0)
red = Image.new("RGB", (w, h), (255, 255, 255))
red.paste((255, 0, 0), mask=mask)
row = Image.new("RGB", (w * 3 + 20, h), (200, 200, 200))
for i, im in enumerate([pad_a, pad_b, red]):
row.paste(im, (i * (w + 10), 0))
rows.append(row.resize((row.width * scale, row.height * scale), Image.NEAREST))
width = max(r.width for r in rows)
height = sum(r.height + 10 for r in rows)
canvas = Image.new("RGB", (width, height), (255, 255, 255))
y = 0
for r in rows:
canvas.paste(r, (0, y))
y += r.height + 10
canvas.save(out)

View File

@ -0,0 +1,421 @@
---
name: refactor-scss
description: Migrate one Penpot SCSS module off the deprecated styles under frontend/resources/styles/common/refactor/ to the current Design System (ds/ tokens, mixins and px2rem). Pure refactor, no visual change. Use when the user asks to refactor, migrate or clean up a .scss file to the DS.
---
# Skill: refactor-scss
Refactor a single SCSS file so that it no longer depends on the deprecated
styling system, using the current Design System instead.
This is a **pure refactor**. Do NOT introduce visual or behavioral changes.
Preserve the generated CSS behavior exactly unless a rule below requires
otherwise. The rules that allow a small visible change (closest DS
typography, DS border width) want it shown in the visual check and listed
in the report and the commit body.
Reviewers read every line of the result. A line that is
copied but does nothing, a bare element selector or a raw z-index comes
back as a review comment, so the rules below are strict about them.
## Input
- The path to one `.scss` file (e.g. `frontend/src/app/main/ui/foo/bar.scss`).
- If no path is given, ask for it. Do not pick a file yourself.
## Workflow
1. Read the target `.scss` file and the `.cljs` file with the same name next
to it.
2. Read every deprecated source the file uses, so you inline real values,
never remembered ones:
- `frontend/resources/styles/common/refactor/mixins.scss`
- `frontend/resources/styles/common/refactor/design-tokens.scss`
- `frontend/resources/styles/common/refactor/spacing.scss`
- any other file under `frontend/resources/styles/common/refactor/` that
the stylesheet pulls in (placeholders, borders, z-index, fonts...).
3. Read the DS modules you will map to:
- `frontend/src/app/main/ui/ds/_utils.scss` (`px2rem()`)
- `frontend/src/app/main/ui/ds/spacing.scss`
- `frontend/src/app/main/ui/ds/_sizes.scss`
- `frontend/src/app/main/ui/ds/typography.scss` (`use-typography()`)
- `frontend/src/app/main/ui/ds/_borders.scss`
- `frontend/src/app/main/ui/ds/z-index.scss`
4. Apply the rules below and edit the file in place.
5. Verify (see **Verification** and **Visual check**).
6. Report (see **Output**). Do not commit. When the refactor goes with other
changes, it belongs in its own commit, after the task's commits.
## Primary goal
After the refactor, the SCSS file must not depend on anything under
`frontend/resources/styles/common/refactor/`. This includes imports, mixins,
placeholder selectors, functions, deprecated variables and design-token
aliases.
### Partial refactor
Some placeholders stand for a whole component family that will move to the DS
in its own PR: `%input-element`, `%input-with-label`, `%input-checkbox`,
`%modal-*`, `%menu-dropdown`, `%menu-item-base`, `%shortcut-*` and similar.
Inlining them in a small task produces big, risky diffs. In that case:
- migrate everything else (tokens, sizes, spacing, small mixins);
- keep the `refactor/` import only for those placeholders;
- list each remaining dependency and the selectors that use it in the report,
so the user can plan it as a separate PR.
## General rules
- Keep the resulting SCSS as small and readable as possible.
- Avoid duplicated declarations and duplicated selectors.
- Do not change class names.
- Do not change DOM structure.
- Do not change specificity unless required.
- Follow the project's SCSS formatting (Prettier, see **Verification**).
## Imports
Use the DS modules. Import them with `as *` when they expose mixins,
variables or functions:
```scss
@use "ds/_utils.scss" as *;
```
not
```scss
@use "ds/_utils.scss" as utils;
```
Do not introduce namespaces such as `utils.`, `typography.`, `borders.`,
`spacing.` or `sizes.` unless a module explicitly requires one.
Remove every unused import.
## Deprecated mixins
If the stylesheet uses mixins from
`frontend/resources/styles/common/refactor/mixins.scss`:
- locate the mixin;
- inline its generated CSS (then apply the rest of these rules to it);
- remove the `@include`;
- remove the import.
Do not leave deprecated mixins.
## Placeholders
Replace every `@extend %placeholder` with the generated CSS. Remove every
deprecated placeholder, except the component-family ones described in
**Partial refactor**.
Inline only what does something on that element. Do not paste a
placeholder (or mixin) body as it is. For each property, check:
- **It applies to the element.** `fill`, `stroke` and `stroke-width` only
act on SVG shapes: drop them from a `span`, `div` or `button`.
`color: transparent` on a wrapper that only holds an icon does nothing
useful: drop it and set the real color.
- **It is not the default or already given.** `stroke-width: 1px` is the
SVG default; `fill: none` and `stroke: currentColor` are already set by
the DS `icon*` (`.icon`). Drop them.
- **A later rule does not override it.**
- **It is not a stroke width on a DS icon.** DS icons keep the DS
stroke width; drop any `stroke-width` that only imitates the legacy
icon (1.33px, 2.25px...).
`%button-icon` is the usual case: on a wrapper it leaves `color:
transparent; fill: none; stroke-width: 1px`, and none of the three is
needed.
## Element selectors
Do not style HTML elements directly (`svg {}`, `h3 {}`, `button {}`,
`input.x {}`, or `.component { svg {} }`).
When no class exists, **add one in the `.cljs`**: a DS component takes
`:class (stl/css :x)` (`icon*`, `heading*`, `button*`…), and a plain
element takes `{:class (stl/css :x)}`. Touching the `.cljs` for this is
part of the refactor. A comment like "the icon has no class" is not an
answer.
Prefer the DS component when one fits: `heading*` for `h1`–`h6`
(`:level`, `:typography`), `text*` for text, `icon*` for icons.
`element.class` (e.g. `input.element-title`) is not needed to beat the
input defaults: try the class alone and check it in the visual check.
An element selector stays only for markup you cannot reach (injected
HTML, third-party widgets), with a comment saying so.
**Icon colors.** A DS icon draws with `stroke: currentColor`. Set
`color` on the icon class or its parent, never `stroke`/`fill` on an
`svg` selector.
## Color variables
Whenever a color changes across states, introduce a component-local CSS
variable.
Instead of
```scss
.button {
color: var(--color-foreground-primary);
&:hover {
color: var(--color-accent-primary);
}
&:disabled {
color: var(--color-foreground-disabled);
}
}
```
write
```scss
.button {
--button-foreground-color: var(--color-foreground-primary);
color: var(--button-foreground-color);
&:hover {
--button-foreground-color: var(--color-accent-primary);
}
&:disabled {
--button-foreground-color: var(--color-foreground-disabled);
}
}
```
Use this pattern for `color`, `background-color`, `border-color`,
`outline-color`, `fill`, `stroke` and any similar property.
## Design token aliases
Remove every alias defined in
`frontend/resources/styles/common/refactor/design-tokens.scss`. Resolve each
alias to its final DS token. Do not keep intermediate aliases.
Instead of `color: var(--button-foreground-hover);` write
`color: var(--color-accent-primary);`.
## Spacing
Replace deprecated spacing variables using
`frontend/src/app/main/ui/ds/spacing.scss` for `margin`, `padding`, `gap` and
`inset`.
If no exact spacing token exists, use `px2rem()`. Do not approximate with the
closest token.
## Sizes
Replace deprecated size variables using
`frontend/src/app/main/ui/ds/_sizes.scss` for `inline-size`, `block-size`,
`width`, `height` and min/max sizes.
If no exact size exists, use `px2rem()` from
`frontend/src/app/main/ui/ds/_utils.scss`. Do not leave hardcoded pixel values
unless absolutely necessary.
## Typography
When any of `font-size`, `font-weight`, `line-height`, `font-family` or
`letter-spacing` appear, check whether together they match one of the
definitions in `frontend/src/app/main/ui/ds/typography.scss`.
If they match, replace all the individual declarations with:
```scss
@include use-typography("<typography-name>");
```
If there is no exact match, use the **closest DS typography** (same role:
a panel title in uppercase → `headline-small`, body text → `body-*`),
even when more than one detail differs. Show the change in the visual
check and state it in the report and in the commit body (e.g. "11px
regular → 12px medium").
Keep the original declarations only when no DS typography fits the role
(e.g. a 32px display number in a 14px-only area); then leave a TODO
naming the closest one and ask the user.
## Borders
Replace deprecated border radius and border width values using
`frontend/src/app/main/ui/ds/_borders.scss`. A raw width close to a token
(`1.75px`) takes the token (`$b-2`); say so in the report. If nothing is
close, use `px2rem()`.
## Logical properties
Prefer logical properties whenever they preserve the existing behavior:
- `width` → `inline-size`
- `height` → `block-size`
- `max-width` → `max-inline-size`
- `margin-left` → `margin-inline-start`
- `padding-right` → `padding-inline-end`
- `top` → `inset-block-start`
- `left` → `inset-inline-start`
## Z-index
Never leave magic z-index values. Use tokens from
`frontend/src/app/main/ui/ds/z-index.scss`, and pick the token **by what
the element is, not by its number**: a dropdown, menu or select →
`--z-index-dropdown`; a toolbar, palette, sidebar or panel →
`--z-index-panels`; and so on (read the comments in that file). A raw `4`
on a dropdown becomes `var(--z-index-dropdown)` even though the number
changes. Check the visual check for overlap.
Leave a TODO only when the element fits no token's purpose.
Legacy z-index values between sibling elements that do not overlap can be
removed, but only when the **Visual check** shows no pixel change in every
state that matters (open menus, collapsed, hidden). Say so in the report.
## Duplicated properties
Within the same selector, avoid duplicated properties, duplicated CSS
variables and duplicated declarations introduced by the refactor. Allow
duplicates only when they are intentional browser fallbacks.
## Unused classes
Open the `.cljs` file with the same name as the SCSS file and check whether
every CSS class is referenced (as `(stl/css :class-name)`,
`(stl/css-case :class-name ...)` or similar).
CSS modules prefix every class with its module
(`main_ui_<path>__<class>`), so only the `.cljs` of the same module can use
it. Before calling a selector unused, also search the whole `frontend/src`
for the class name as a string, a `:global` use, or a dynamic class built
at runtime.
If a selector is unused after that check:
- delete it (do not leave it commented out, do not add a TODO);
- list every deleted selector in the report and in the commit body.
A rule that matches nothing because the markup changed (e.g. `.icon svg`
when the DS icon is the `svg` itself) is deleted the same way. Before
deleting a descendant rule, check every element it matched, not only
the one you changed: `.placeholder .icon svg` can also color a second
icon, which then has no color (in the devenv it shows in the debug
yellow).
Every DS icon needs a color of its own (`color` on its class or on a
parent with a defined color): an icon that inherits nothing shows in
the debug yellow in the devenv.
Classes used by markup injected from outside (e.g. an external widget)
stay, with a comment saying so.
Also check the other direction: every class the `.cljs` passes to
`stl/css` / `stl/css-case` must exist in the `.scss`. A missing one silently
does nothing; report it.
## Sass and CSS pitfalls
- Sass does not evaluate functions inside CSS custom properties. Write
`--button-height: calc(var(--height) - #{px2rem(16)});`, not
`... - px2rem(16))`, or the browser gets the literal `px2rem(16)`.
- A `/` between Sass size variables is read as a division
(`grid-template: $sz-32 / auto` breaks). Split it into
`grid-template-rows` / `grid-template-columns` (or areas).
- A CSS-wide keyword cannot be part of a shorthand: `inset-inline: unset 0`
is invalid and stylelint does not catch it. Use the longhands.
- After editing, check the compiled CSS for any of these
(`frontend/resources/public/css/main.css`, search the module class).
## Uncertainty
If any transformation is uncertain, never guess. Leave a TODO in the code
and explain why in the report.
## Self-review
Before the report, read the final diff as the reviewer would and search
the file for these; each hit must be fixed or have a real reason:
```bash
rg -n 'svg \{|^\s*(h[1-6]|p|hr|input|button|span|a)\b[^-_]|[a-z]\.[a-z-]+ \{' <file>
rg -n 'stroke-width|color: transparent|fill: none|z-index: [0-9]|TODO' <file>
rg -n '[0-9]\.[0-9]+px|[^(]\b[0-9]+px' <file>
```
- an element selector → add a class (see **Element selectors**);
- a property that does not apply or is a default → drop it (see
**Placeholders**);
- a raw z-index → token by purpose;
- a typography TODO → closest DS typography;
- a raw px value → token or `px2rem()`.
## Verification
Run the project scripts from `frontend/` inside the devenv container
(`penpot-devenv-ws<N>-main` for workspace `wsN`; never run `pnpm` on the
host), writing output to a file first (never pipe lint output into
filters):
```bash
docker exec -u penpot -w /home/penpot/penpot/frontend penpot-devenv-ws<N>-main \
bash -lc 'pnpm run lint:scss; echo LINT=$?; pnpm run check-fmt:scss; echo FMT=$?' \
> <scratch>/scss-check.txt 2>&1
```
If the format check fails, run `pnpm run fmt:scss` and check again.
**Dependency check.** A grep for `refactor/` is not enough. For the final
file, check:
- `@use` / `@import` sources: all must be `ds/...` (or the sidebar common
module, which is DS-only);
- every `@include`, `@extend` and `%placeholder`;
- every `$variable`: which imported module defines it (`$br-*` exists in both
`ds/_borders.scss` and `refactor/borders.scss`; the `@use` decides);
- every `var(--token)`: defined in the DS (colors, spacing, sizes) or only as a
`refactor/` alias.
Report any failure as it is; do not hide it.
## Visual check
"No visual change" must be shown, not assumed. When a visual spec covers the
component (`frontend/playwright/ui/visual-specs/`, project `ds`), or one can be
added:
1. Capture before (base branch styles) and after with
`pnpm exec playwright test --project=ds -g "<test>" --update-snapshots=all`
in the container. `=all` is required: plain `--update-snapshots` only
rewrites past the 0.5% tolerance, so a real diff can look like 0.
2. Before each capture, wait until the compiled CSS/JS shows the expected
version (search `main.css` for the changed rule).
3. Compare with zero tolerance. If pixels change, build the before | after |
diff (red) image and explain every change in the report.
## Output
1. The diff of the file (`git diff -- <file>`).
2. A summary with:
- removed imports;
- removed deprecated mixins;
- removed placeholders;
- typography replacements;
- design-token aliases removed;
- logical properties introduced;
- deleted unused selectors;
- TODOs requiring manual review, each with its reason;
- remaining `refactor/` dependencies (partial refactor) and where they are
used;
- classes missing from the `.scss`, if any.
3. The verification results, including the visual check (pixel count, or why
it was not possible).