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 <noreply@anthropic.com>
This commit is contained in:
1 parent
89b95aa8ac
commit
66868de10a
6 files changed
+59
-4
No files matched your search
@@ -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;
|
||||
|
||||
@@ -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) {
|
||||
|
||||
+11
-1
@@ -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
|
||||
|
||||
Reference in new issue
Block a user