mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): restore zoom on timeline text via rem tokens
Cmd +/- zoom scales the root <html> font-size (rem-only by design), but PR #891 converted the message-timeline + thread render path from rem tokens to hardcoded px (text-[15px]/text-[13px], font-size: 15px), freezing that text against zoom. Preserve Kenny's 15px chat sizing intent but express it in rem so it scales: add rem-based `text-chat` (0.9375rem === 15px) and `text-code` (0.8125rem === 13px) Tailwind tokens, and swap the px classes over in MessageRow, markdown, mentionChip, and globals.css (.mention-highlight). Codify it: add `pnpm check:px-text` CI guard flagging new px text in the timeline/thread render path, wired into `pnpm check`, plus an AGENTS.md note steering future agents to rem tokens. Scoped to the regression footprint — no app-wide sweep. Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
This commit is contained in:
co-authored by
Taylor Ho
parent
9ee5aeebd9
commit
bfb892c985
@@ -412,6 +412,27 @@ just desktop-dev # web-only dev server (faster iteration)
|
||||
just dev # full Tauri app with native shell
|
||||
```
|
||||
|
||||
### Text sizing & zoom (use rem, never px)
|
||||
|
||||
The desktop app implements Cmd +/- zoom by scaling the root `<html>`
|
||||
font-size (`desktop/src/app/useWebviewZoomShortcuts.ts`) and pinning the native
|
||||
webview zoom. **Only rem-based text scales with zoom — hardcoded px text sizes
|
||||
are frozen.**
|
||||
|
||||
So for any readable text, reach for rem-based Tailwind tokens, never arbitrary
|
||||
px:
|
||||
|
||||
- ✅ `text-chat` (chat body/author, 15px in rem), `text-code` (inline/block
|
||||
code, 13px in rem), or a stock token (`text-sm`, `text-base`, …). Tokens live
|
||||
in `desktop/tailwind.config.js` under `theme.extend.fontSize`.
|
||||
- ❌ `text-[15px]`, `text-[13px]`, or CSS `font-size: 15px`. These opted out of
|
||||
zoom and caused the message-timeline regression (PR #891).
|
||||
|
||||
If a design needs a size between stock tokens (e.g. 15px sits between `text-sm`
|
||||
14px and `text-base` 16px), **add a rem-based token** rather than an arbitrary
|
||||
px value. A CI guard (`pnpm check:px-text`, in `desktop/scripts/check-px-text.mjs`)
|
||||
fails on new px text in the message-timeline / thread render path.
|
||||
|
||||
### Workspace Switching
|
||||
|
||||
The desktop app supports multiple workspaces (each backed by a different relay).
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
# Timeline Zoom Regression — Findings (for Bart)
|
||||
|
||||
## The bug
|
||||
Cmd +/- zoom no longer scales message-timeline & thread text.
|
||||
|
||||
## Why (mechanism)
|
||||
`desktop/src/app/useWebviewZoomShortcuts.ts` scales the ROOT `<html>` font-size
|
||||
(rem-based scaling) and pins native `webview.setZoom(DEFAULT)`. So only **rem**
|
||||
sizes scale; hardcoded **px** sizes are frozen. This is the intended approach
|
||||
("only text should scale" — keeps webview coordinate system stable). Do NOT
|
||||
revert to native webview zoom.
|
||||
|
||||
## Two-layer history
|
||||
1. **#573** (9e76a08a, May 14) — switched zoom native→root-font-size/rem-only.
|
||||
2. **#891** (45f3dfe5, Jun 8, "Tune chat text sizing", klopez4212) — the recent
|
||||
timeline regression. Converted timeline rem→px.
|
||||
|
||||
## Kenny's intent in #891 (PRESERVE THIS)
|
||||
He bumped chat text up from `text-sm` (0.875rem=14px) to **15px** because sm felt
|
||||
too small. Conversions made:
|
||||
- MessageRow author name `<span>` & `<h3>`: `text-sm` → `text-[15px]`
|
||||
- markdown body / mentionChip: `text-sm` → `text-[15px]`
|
||||
- `globals.css` `.mention-highlight`: added `font-size: 15px`, radius 0.375rem→4px
|
||||
- also touched MessageTimeline, SystemMessageRow, MessageThreadSummaryRow
|
||||
|
||||
## The crux
|
||||
Tailwind v4 here uses STOCK text tokens (no `@theme` override). Stock scale:
|
||||
text-sm=14px, text-base=16px. **15px sits between them — no stock token exists.**
|
||||
That's WHY Kenny reached for arbitrary px.
|
||||
|
||||
## tho's directive
|
||||
- Use rem + Tailwind tokens wherever possible.
|
||||
- Preserve Kenny's visual intent (the 15px chat sizing).
|
||||
- Outcome MAY be to define/update a text-size token to yield 15px in rem — but
|
||||
only if stock tokens genuinely can't deliver the look. Take a critical pass:
|
||||
prefer a stock token if it looks right; introduce a custom token only if needed.
|
||||
|
||||
## Scope: timeline + thread render path only
|
||||
MessageRow, markdown.tsx, mentionChip.ts, SystemMessageRow,
|
||||
MessageThreadSummaryRow, MessageTimeline, and the relevant globals.css rules
|
||||
(incl. `.mention-highlight` px font-size). Verify zoom works after.
|
||||
|
||||
## Codify (fix #2)
|
||||
No custom lint exists. Add a guard (Biome rule or CI grep) flagging new
|
||||
`text-[NNpx]` / px `fontSize` in the desktop app + a note in AGENTS.md/CLAUDE.md.
|
||||
@@ -8,8 +8,9 @@
|
||||
"build": "tsc && vite build",
|
||||
"typecheck": "tsc --noEmit",
|
||||
"check:file-sizes": "node ./scripts/check-file-sizes.mjs",
|
||||
"check:px-text": "node ./scripts/check-px-text.mjs",
|
||||
"lint": "biome lint .",
|
||||
"check": "biome check . && pnpm check:file-sizes",
|
||||
"check": "biome check . && pnpm check:file-sizes && pnpm check:px-text",
|
||||
"format": "biome format --write .",
|
||||
"test": "node --import ./test-loader.mjs --experimental-strip-types --test 'src/**/*.test.mjs'",
|
||||
"preview": "vite preview",
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { runPxTextCheck } from "../../scripts/check-px-text-core.mjs";
|
||||
|
||||
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
const projectRoot = path.resolve(__dirname, "..");
|
||||
|
||||
// Scoped to the message-timeline / thread render path — the surface where the
|
||||
// rem→px zoom regression (PR #891) landed. Readable message text here MUST use
|
||||
// rem-based tokens (`text-chat`, `text-code`) so Cmd +/- zoom scales it. We
|
||||
// intentionally do NOT sweep the whole app yet (decorative chrome — avatar
|
||||
// initials, day dividers, diff-viewer labels — still uses px); widen these
|
||||
// roots when that sweep happens.
|
||||
const rules = [
|
||||
{
|
||||
root: "src/shared/ui",
|
||||
extensions: new Set([".ts", ".tsx"]),
|
||||
files: new Set(["markdown.tsx", "mentionChip.ts"]),
|
||||
},
|
||||
{
|
||||
root: "src/features/messages/ui",
|
||||
extensions: new Set([".tsx"]),
|
||||
files: new Set(["MessageRow.tsx"]),
|
||||
},
|
||||
{
|
||||
// `.mention-highlight` lives here and was part of the #891 px regression —
|
||||
// guard the `font-size: NNpx` form too, not just the Tailwind utility.
|
||||
root: "src/shared/styles",
|
||||
extensions: new Set([".css"]),
|
||||
files: new Set(["globals.css"]),
|
||||
},
|
||||
];
|
||||
|
||||
// Decorative / chrome px-text exceptions: `relativePath:lineNumber`. Empty for
|
||||
// now — the regression footprint is fully on rem tokens.
|
||||
const overrides = new Set();
|
||||
|
||||
await runPxTextCheck({
|
||||
projectRoot,
|
||||
rules,
|
||||
overrides,
|
||||
label: "Desktop",
|
||||
scriptPath: "desktop/scripts/check-px-text.mjs",
|
||||
});
|
||||
@@ -188,7 +188,7 @@ export const MessageRow = React.memo(
|
||||
<Markdown
|
||||
channelNames={channelNames}
|
||||
className={cn(
|
||||
"max-w-full text-[15px] leading-6",
|
||||
"max-w-full text-chat",
|
||||
emojiOnly &&
|
||||
"text-4xl leading-tight [&_img[data-custom-emoji]]:h-[1.45em] [&_img[data-custom-emoji]]:align-middle [&_button:has(img[data-custom-emoji])]:align-middle",
|
||||
)}
|
||||
@@ -245,11 +245,11 @@ export const MessageRow = React.memo(
|
||||
);
|
||||
|
||||
const authorNode = message.pubkey ? (
|
||||
<span className="truncate text-[15px] font-semibold leading-none tracking-tight hover:underline">
|
||||
<span className="truncate text-chat font-semibold leading-none tracking-tight hover:underline">
|
||||
{message.author}
|
||||
</span>
|
||||
) : (
|
||||
<h3 className="truncate text-[15px] font-semibold leading-none tracking-tight">
|
||||
<h3 className="truncate text-chat font-semibold leading-none tracking-tight">
|
||||
{message.author}
|
||||
</h3>
|
||||
);
|
||||
|
||||
@@ -615,7 +615,8 @@
|
||||
border-radius: 4px;
|
||||
background: hsl(var(--primary) / 0.15);
|
||||
padding: 3px 4px 2px;
|
||||
font-size: 15px;
|
||||
/* 0.9375rem === 15px; rem so it scales with the root-font-size zoom. */
|
||||
font-size: 0.9375rem;
|
||||
line-height: 1;
|
||||
color: hsl(var(--primary));
|
||||
font-weight: 500;
|
||||
|
||||
@@ -78,7 +78,7 @@ const MAX_CACHE_ENTRIES = 100;
|
||||
const MAX_LOADED_LANGUAGES = 30;
|
||||
const MAX_HIGHLIGHT_LINES = 150;
|
||||
const CODE_BLOCK_CLASS =
|
||||
"code-block-lines block min-w-full whitespace-pre font-mono text-[13px] leading-6 text-foreground";
|
||||
"code-block-lines block min-w-full whitespace-pre font-mono text-code text-foreground";
|
||||
const DIFF_ADD_RE = /\s*\/\/\s*\[!code\s*\+\+\]\s*$/;
|
||||
const DIFF_REMOVE_RE = /\s*\/\/\s*\[!code\s*--\]\s*$/;
|
||||
|
||||
@@ -850,7 +850,7 @@ function createMarkdownComponents(
|
||||
<code
|
||||
{...props}
|
||||
className={cn(
|
||||
"rounded-md bg-muted px-1.5 py-0.5 font-mono text-[13px] text-foreground",
|
||||
"rounded-md bg-muted px-1.5 py-0.5 font-mono text-code text-foreground",
|
||||
className,
|
||||
)}
|
||||
>
|
||||
@@ -1221,7 +1221,7 @@ function MarkdownInner({
|
||||
].join(" ")
|
||||
: compact
|
||||
? [
|
||||
"max-w-none break-words text-[15px] leading-6 text-foreground/90",
|
||||
"max-w-none break-words text-chat text-foreground/90",
|
||||
"[&>*:first-child]:mt-0 [&>*:last-child]:mb-0",
|
||||
"[&>*+*]:mt-2",
|
||||
"[&>*+h1]:mt-3 [&>*+h2]:mt-3 [&>*+h3]:mt-3",
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
export const MENTION_CHIP_BASE_CLASSES =
|
||||
"inline-block rounded-[4px] bg-primary/15 px-1 pt-[3px] pb-[2px] text-[15px] font-medium leading-none text-primary";
|
||||
"inline-block rounded-[4px] bg-primary/15 px-1 pt-[3px] pb-[2px] text-chat font-medium leading-none text-primary";
|
||||
|
||||
export const MENTION_CHIP_HOVER_CLASSES =
|
||||
"transition-colors hover:bg-primary/25 hover:text-primary/90";
|
||||
|
||||
@@ -2,6 +2,15 @@
|
||||
export default {
|
||||
theme: {
|
||||
extend: {
|
||||
fontSize: {
|
||||
// Chat body/author sizing. 15px sits between Tailwind's stock
|
||||
// `text-sm` (14px) and `text-base` (16px), so we express it as a rem
|
||||
// token instead of a hardcoded px value — px would not scale with the
|
||||
// root-font-size zoom (Cmd +/-). 0.9375rem === 15px at the 16px root.
|
||||
chat: ["0.9375rem", { lineHeight: "1.5rem" }],
|
||||
// Inline & block code inside chat messages (13px → 0.8125rem).
|
||||
code: ["0.8125rem", { lineHeight: "1.5rem" }],
|
||||
},
|
||||
borderRadius: {
|
||||
lg: "var(--radius)",
|
||||
md: "calc(var(--radius) - 2px)",
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
import { promises as fs } from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
/**
|
||||
* Shared "no hardcoded px text size" guard.
|
||||
*
|
||||
* Zoom (Cmd +/-) scales the root <html> font-size, so only **rem**-based text
|
||||
* scales. Hardcoded px text sizes (`text-[15px]`, `font-size: 15px`) freeze
|
||||
* against zoom — that's the timeline regression we fixed. This guard stops new
|
||||
* px text sizes from creeping back in. Use a rem-based Tailwind token instead
|
||||
* (e.g. `text-chat`, `text-code`, `text-sm`).
|
||||
*
|
||||
* It flags:
|
||||
* - Tailwind arbitrary px text utilities: `text-[NNpx]`
|
||||
* - CSS px font sizes: `font-size: NNpx`
|
||||
*
|
||||
* Decorative/chrome exceptions (avatar initials sized to a fixed avatar box,
|
||||
* etc.) live in the `overrides` allowlist supplied by each app.
|
||||
*/
|
||||
|
||||
const TEXT_PX_RE = /\btext-\[\d+(?:\.\d+)?px\]/g;
|
||||
// Match the CSS `font-size` property, but NOT custom properties like
|
||||
// `--font-size:` (third-party widget vars) which merely contain the substring.
|
||||
const FONT_SIZE_PX_RE = /(?<!-)\bfont-size:\s*\d+(?:\.\d+)?px/g;
|
||||
|
||||
async function walkFiles(directory) {
|
||||
const entries = await fs.readdir(directory, { withFileTypes: true });
|
||||
const files = await Promise.all(
|
||||
entries.map(async (entry) => {
|
||||
const fullPath = path.join(directory, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
return walkFiles(fullPath);
|
||||
}
|
||||
return [fullPath];
|
||||
}),
|
||||
);
|
||||
return files.flat();
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {object} options
|
||||
* @param {string} options.projectRoot Absolute path the rule roots resolve against.
|
||||
* @param {Array<{root: string, extensions: Set<string>}>} options.rules Where to scan.
|
||||
* @param {string} options.label Human label for the failure header.
|
||||
* @param {Set<string>} [options.overrides] Allowlisted "relativePath:lineNumber" entries.
|
||||
* @param {string} options.scriptPath Path mentioned in the failure hint.
|
||||
*/
|
||||
export async function runPxTextCheck({
|
||||
projectRoot,
|
||||
rules,
|
||||
label,
|
||||
overrides = new Set(),
|
||||
scriptPath,
|
||||
}) {
|
||||
const candidateFiles = (
|
||||
await Promise.all(
|
||||
rules.map((rule) => {
|
||||
const dir = path.join(projectRoot, rule.root);
|
||||
return fs
|
||||
.access(dir)
|
||||
.then(() => walkFiles(dir))
|
||||
.catch(() => []);
|
||||
}),
|
||||
)
|
||||
).flat();
|
||||
|
||||
const violations = [];
|
||||
|
||||
for (const filePath of candidateFiles) {
|
||||
const relativePath = path.relative(projectRoot, filePath);
|
||||
const rule = rules.find((r) =>
|
||||
relativePath.startsWith(`${r.root}${path.sep}`),
|
||||
);
|
||||
if (!rule) {
|
||||
continue;
|
||||
}
|
||||
if (!rule.extensions.has(path.extname(relativePath))) {
|
||||
continue;
|
||||
}
|
||||
// Optional per-rule basename allowlist — scopes the scan to specific files.
|
||||
if (rule.files && !rule.files.has(path.basename(relativePath))) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const content = await fs.readFile(filePath, "utf8");
|
||||
const lines = content.split(/\r?\n/);
|
||||
lines.forEach((line, index) => {
|
||||
const lineNumber = index + 1;
|
||||
const key = `${relativePath}:${lineNumber}`;
|
||||
if (overrides.has(key)) {
|
||||
return;
|
||||
}
|
||||
const matches = [
|
||||
...(line.match(TEXT_PX_RE) ?? []),
|
||||
...(line.match(FONT_SIZE_PX_RE) ?? []),
|
||||
];
|
||||
for (const match of matches) {
|
||||
violations.push({ relativePath, lineNumber, match });
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
if (violations.length > 0) {
|
||||
console.error(`${label} px-text check failed:`);
|
||||
for (const v of violations) {
|
||||
console.error(`- ${v.relativePath}:${v.lineNumber}: ${v.match}`);
|
||||
}
|
||||
console.error(
|
||||
"Use a rem-based Tailwind text token (e.g. `text-chat`, `text-code`, " +
|
||||
"`text-sm`) so the text scales with Cmd +/- zoom. If this px size is " +
|
||||
"genuinely decorative/chrome (not readable message text), add a " +
|
||||
`narrowly scoped \`relativePath:lineNumber\` exception in \`${scriptPath}\`.`,
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user