From 66868de10af210616954e0c63da1d458ee757058 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Sat, 26 Sep 2026 22:53:00 +0200 Subject: [PATCH] Show destructive menu entries in red text, not on a red fill A red filled row in a context menu shouted louder than the action deserves. The entry now looks like every other entry and only its text is red, so the menu stays calm and the warning is still there. - Menu: danger entries are ghost buttons with the new text style danger. On hover they get the same grey fill as all other entries. - Theme: new text style danger in the normal font size. The existing warning style is small and did not fit a menu row. - Button: two getters expose the drawn text colour and fill, so a test can measure the look instead of reading the danger flag. - Test hook __gm.menuLooks() lists both per menu entry. - Confirm dialogs keep their filled red Delete button. That is a separate widget and was not part of the request. Tests: - ./check.sh: 172 files, 2498 tests, CHECK EXIT 0 - e2e 25: ALL PASSED, 31 checks. New check in 25g opens the menu with a real right click: Delete row has text #e0553f and no fill, Insert frames keeps the normal text colour. - Counter-check: with the old styling the check fails with text #ffffff on fill #e0553f. Original restored per cmp and rebuilt. - Not checked: other menus (graph, assets) have no own check. They run through the same Menu code. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/editor/debug/uiHooks.ts | 12 +++++++++++- src/ui/Theme.ts | 4 ++++ src/ui/widgets/Button.ts | 21 ++++++++++++++++++++- src/ui/widgets/Menu.ts | 12 +++++++++++- tools/e2e/25-animation.mjs | 12 ++++++++++++ 6 files changed, 59 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d3fd258f..e090a5f8 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ npm run build # writes dist/ and regenerates index.html; the folder is wh The browser suite and the three example games need a machine with a GPU: ```bash -node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1417 checks (GM_STEPS=20 for one step) +node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1418 checks (GM_STEPS=20 for one step) # every tool signs in first – see the Accounts page node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs node tools/seed-tetris.mjs && node tools/smoke-tetris.mjs diff --git a/src/editor/debug/uiHooks.ts b/src/editor/debug/uiHooks.ts index 5990a162..06825f97 100644 --- a/src/editor/debug/uiHooks.ts +++ b/src/editor/debug/uiHooks.ts @@ -9,6 +9,10 @@ import type { UsersPane } from '../users/UsersPane'; import { Color, type ColorSource, type Container, Text } from 'pixi.js'; import { centreOf, type HookContext } from './hookHelpers'; +function hex(color: number): string { + return `#${color.toString(16).padStart(6, '0')}`; +} + /** Widgets found by their visible text, the panels, menus, pickers and dialogs around the tabs. */ export function uiHooks({ app, state, editor }: HookContext) { const { hierarchy, assets, inspector } = editor; @@ -176,7 +180,7 @@ export function uiHooks({ app, state, editor }: HookContext) { /** What a panel dragged shut keeps: its header. Follows the theme, so checks need not. */ panelHeader: Panel.HEADER_HEIGHT, /** A theme colour as a hex string, so a check names the colour instead of its current value. */ - themeColor: (name: keyof typeof Theme.colors) => `#${Theme.colors[name].toString(16).padStart(6, '0')}`, + themeColor: (name: keyof typeof Theme.colors) => hex(Theme.colors[name]), inspectorInfo: () => inspector.debugInfo(), textColor, overlayTexts, @@ -189,6 +193,12 @@ export function uiHooks({ app, state, editor }: HookContext) { menuTicks: () => currentMenu()?.ticksDrawn() ?? null, /** Per entry of `menuRows`, whether it can be picked (a greyed-out one cannot). */ menuEnabled: () => currentMenu()?.enabledDrawn() ?? null, + /** Per entry of `menuRows`, `{ text, fill }` as drawn: label colour and background (`null` = none). */ + menuLooks: () => + currentMenu() + ?.looksDrawn() + .map((look) => look && { text: hex(look.text), fill: look.fill === null ? null : hex(look.fill) }) ?? + null, textOverflows, labelsDropped, shownLabel, diff --git a/src/ui/Theme.ts b/src/ui/Theme.ts index 0e67bfe4..945dfbb1 100644 --- a/src/ui/Theme.ts +++ b/src/ui/Theme.ts @@ -93,6 +93,7 @@ export type TextStyleToken = | 'link' | 'small' | 'warning' + | 'danger' | 'live' | 'title' | 'mono' @@ -130,6 +131,9 @@ export function textStyle(token: TextStyleToken = 'body'): TextStyle { case 'warning': style = new TextStyle({ ...base, fontSize: Theme.smallFontSize, fill: Theme.colors.danger }); break; + case 'danger': + style = new TextStyle({ ...base, fill: Theme.colors.danger }); + break; case 'live': style = new TextStyle({ ...base, fontSize: Theme.smallFontSize, fill: Theme.colors.hot }); break; diff --git a/src/ui/widgets/Button.ts b/src/ui/widgets/Button.ts index e70e7d86..10f88227 100644 --- a/src/ui/widgets/Button.ts +++ b/src/ui/widgets/Button.ts @@ -1,4 +1,11 @@ -import { type FederatedPointerEvent, Graphics, Text, CanvasTextMetrics } from 'pixi.js'; +import { + Color, + type ColorSource, + type FederatedPointerEvent, + Graphics, + Text, + CanvasTextMetrics, +} from 'pixi.js'; import { drawControlBox } from '../draw'; import { centreY } from '../math'; import { drawSvg } from '../svg'; @@ -46,6 +53,7 @@ export class Button extends Widget { private readonly iconGfx = new Graphics(); /** Colour the icon was last drawn in; it follows the label from muted to the accent text. */ private iconColor = -1; + private drawnFill: number | null = null; onClick?: () => void; constructor(text: string, options: ButtonOptions = {}) { @@ -169,6 +177,16 @@ export class Button extends Widget { return weight === 'bold' || weight === 'bolder' || Number(weight) >= 600; } + /** Colour the box behind the label was last filled with, `null` for none (test hook). */ + get fillDrawn(): number | null { + return this.drawnFill; + } + + /** Colour the label is drawn in (test hook). */ + get labelColor(): number { + return new Color(this.labelText.style.fill as ColorSource).toNumber(); + } + /** Whether an icon is drawn in front of the label (test hook). */ get hasIcon(): boolean { return this.icon !== null; @@ -226,6 +244,7 @@ export class Button extends Widget { if (this.hovered && !this.toggled && flat) fill = c.controlHover; if (this.pressed && flat) fill = c.controlActive; const stroke = this.focused ? c.accent : border; + this.drawnFill = fill; this.bg.clear(); if (fill !== null && stroke !== null) { diff --git a/src/ui/widgets/Menu.ts b/src/ui/widgets/Menu.ts index 1aed0266..e4b97428 100644 --- a/src/ui/widgets/Menu.ts +++ b/src/ui/widgets/Menu.ts @@ -75,8 +75,11 @@ export class Menu extends Popup { continue; } const { checked } = item; + // A destructive entry says so in red text only: a filled red row shouted louder than the + // menu around it and read like the one under the pointer. const button = new Button(item.label, { - variant: item.danger ? 'danger' : 'ghost', + variant: 'ghost', + style: item.danger ? 'danger' : 'body', align: 'left', icon: checked ? tickIcon(checked()) : item.current ? UI_ICON_SVG.current : item.icon, }); @@ -136,6 +139,13 @@ export class Menu extends Popup { }); } + /** Per entry of `rowsDrawn`: label colour and background fill as drawn; `null` for a separator (test hook). */ + looksDrawn(): ({ text: number; fill: number | null } | null)[] { + return this.list.children + .filter((child) => child !== this.empty && child.visible) + .map((child) => (child instanceof Button ? { text: child.labelColor, fill: child.fillDrawn } : null)); + } + /** Per entry of `rowsDrawn`, whether it shows an icon; `null` for a separator (test hook). */ iconsDrawn(): (boolean | null)[] { return this.list.children diff --git a/tools/e2e/25-animation.mjs b/tools/e2e/25-animation.mjs index 2193dc6f..21b28f1f 100644 --- a/tools/e2e/25-animation.mjs +++ b/tools/e2e/25-animation.mjs @@ -379,6 +379,18 @@ export async function animation(t) { const rowHeader = await json(`__gm.animation.headerAt(${JSON.stringify(trackId)})`); await click(rowHeader.x, rowHeader.y, 'right'); const deleteItem = await find('Delete row', true); + // A destructive entry is red text on the plain menu, not a filled red row. + const menuRows = await json('__gm.menuRows()'); + const looks = await json('__gm.menuLooks()'); + const danger = await json(`__gm.themeColor('danger')`); + const plain = await json(`__gm.themeColor('text')`); + const deleteLook = looks[menuRows.indexOf('Delete row')]; + const insertLook = looks[menuRows.findIndex((row) => row?.startsWith('Insert frames'))]; + check( + 'Delete row is drawn in red text without a red fill, a plain entry in the normal text colour', + deleteLook?.text === danger && deleteLook.fill !== danger && insertLook?.text === plain, + JSON.stringify({ deleteLook, insertLook, danger, plain }), + ); await click(deleteItem.x, deleteItem.y); const rowsGone = (await walk()).tracks.length; const afterDelete = await json('__gm.state.selection');