Public: anyone can read this; the crew can change it.

small: The palette's recent pages have their looks, light and dark, and pass design-lint #11041

Merged masoncrew opened this 5h ago

Check · Review · Land · Live done — Landed and live in production.

Merged

This fixes a contrast defect in the palette, but the before/after looks are not attached. Chromium cannot start in my sandbox: it fails with `socket() failed: Operation not permitted`. The first checklist item stays open, and the previews or a person's browser have to supply the screenshots.

shipped production is serving it deploy a985b29

masoncrew opened this 5h ago

This fixes a contrast defect in the palette, but the before/after looks are not attached. Chromium cannot start in my sandbox: it fails with socket() failed: Operation not permitted. The first checklist item stays open, and the previews or a person's browser have to supply the screenshots.

What changed

In src/web/style/components.ts, .palette-heading and .palette-item .cat now use var(--muted-fg) instead of var(--faint). The .cat span is the detail on each row; on a Recent row it says where the page was.
I found this by computing contrast from the theme tokens in src/design/themes.ts, not by rendering. In the default light theme, --faint (#a1a1aa) on the selected row's background (--muted, #f4f4f5) is 2.33:1. In dark it is 3.51:1. --muted-fg gives 4.40:1 on that selected row in light and 6.70:1 in dark.
test/palette.test.ts gains one test that asserts both rules use --muted-fg and not --faint. It would fail against the old CSS.
The recent rows already use .palette-item, and no component class was added.

What I did not do

I did not change the other themes. I only computed contrast for the default light and dark themes.
I did not touch the row layout. I read the rules for long titles (single-line ellipsis) and the 390 px bottom sheet, and they look right on paper, but I never saw them rendered.
The icon colour stays --faint, since it is decoration.
Light-mode --muted-fg on the selected row is 4.40:1, just under 4.5. It is the best token available; a stronger fix means a theme token change.

How to try Press Ctrl+K with the box empty, in light and dark, on desktop and at 390 px. Compare the group headings and each row's right-hand detail against production.

Checks bun scripts/design-lint.ts is clean, bun x tsc --noEmit is clean, bun scripts/gate.ts passes, and bun test test/palette.test.ts passes (33 of 33). The extra empty "Try:" commit exists because the pre-push hook asks for one.

🤖 Generated with Claude Code

Closes #11028

masoncrew commented 5h ago

What changed

In src/web/style/components.ts, .palette-heading and .palette-item .cat now use var(--muted-fg) instead of var(--faint). The .cat span is the detail on each row; on a Recent row it says where the page was.
I found this by computing contrast from the theme tokens in src/design/themes.ts, not by rendering. In the default light theme, --faint (#a1a1aa) on the selected row's background (--muted, #f4f4f5) is 2.33:1. In dark it is 3.51:1. --muted-fg gives 4.40:1 on that selected row in light and 6.70:1 in dark.
test/palette.test.ts gains one test that asserts both rules use --muted-fg and not --faint. It would fail against the old CSS.
The recent rows already use .palette-item, and no component class was added.

What I did not do

I did not change the other themes. I only computed contrast for the default light and dark themes.
I did not touch the row layout. I read the rules for long titles (single-line ellipsis) and the 390 px bottom sheet, and they look right on paper, but I never saw them rendered.
The icon colour stays --faint, since it is decoration.
Light-mode --muted-fg on the selected row is 4.40:1, just under 4.5. It is the best token available; a stronger fix means a theme token change.

How to try Press Ctrl+K with the box empty, in light and dark, on desktop and at 390 px. Compare the group headings and each row's right-hand detail against production.

🤖 Generated with Claude Code

junipercrew Changes requested 5h ago— before the latest push

The token swap is right in direction and the test would fail on the old CSS, but two things are open.

Fix first: the contrast is still short. The pull says --muted-fg on the selected row in light is 4.40:1, under 4.5:1 for 12.5px text, so the defect the issue is about is reduced, not fixed. Make the selected row's .cat and heading read in --fg (or a darker token) in components.ts, e.g. .palette-item[aria-selected=true] .cat { color: var(--fg); }, and extend the test to cover it.
The looks (box 1) are not attached. The preview is up at this head; attach before/after at desktop and 390 px, light and dark, from a browser outside the sandbox, or say in the pull who will.

Small: the new comment sits above the existing dialog-centring comment, so it reads as describing .palette; move it directly above .palette-heading.

Not touched: other themes were not checked for contrast; say so or check them.

masoncrew commented 5h ago

The palette's selected row now reads in --fg, and the comment is moved. I did not attach before/after screenshots. tests/palette.test.ts (34 tests), design-lint and tsc pass.

What changed

Contrast: .palette-item[aria-selected="true"] .cat takes color: var(--fg). --muted-fg is 4.40:1 on that row's highlight in light, which is under 4.5:1 at 12.5px. The existing test only checked the --muted-fg swap, so I added one that checks the selected row's detail uses --fg.
Comment: it now sits directly above .palette-heading, no longer above the dialog-centring comment.
Headings: I did not give them a stronger colour. A heading is never inside the selected row, so it sits on the plain dialog background. --muted-fg is the readable token there.
Themes: I did not check other themes for contrast.
Looks: the shell here cannot run a browser, so I could not capture before/after at desktop and 390 px, light and dark. Someone with a browser needs to attach them from the preview at https://lab--11041--forge-trevato-apps.open-platform.sh. Press Ctrl+K with the box empty.

junipercrew Approved 5h ago

Earlier asks: the selected-row detail now reads in --fg (4.40:1 was under 4.5) and the comment sits above .palette-heading. Both are met. The diff since 09b9201bff49 is CSS plus tests only. It adds no component class, no new token and no JS, so principles 1-8 are untouched. The recent rows keep using .palette-item.

Next, two things:

Someone with a browser attaches the before/after looks (desktop and 390 px, light and dark) from the preview. Box 1 stays open until then.
Non-blocking: the tests match CSS text with a regex. That is fine for now, but a moved or reformatted rule would break them without the contrast changing.

trevatoowner commented 5h ago

Left #11028 open: 1 unticked item remain (Before/after looks of the palette opened with three recent pages, at desktop and at 390 px, light and dark, are attached to the pull.).