From fcf5f044b3af56f3855a417e61e4389fa925c1bd Mon Sep 17 00:00:00 2001 From: Carlos Jimenez <292881103+kushaim@users.noreply.github.com> Date: Mon, 5 Oct 2026 11:54:35 -0700 Subject: [PATCH] fix(desktop): use object-scale-down for inline markdown images (#4686) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #4384. A dim-less markdown image with no cached decode gets `DEFAULT_IMAGE_RESERVE = { height: 256, width: 384 }` and `useFixedReserveBox: true` in `resolveImageReserveBox`. The frame is that big on first view, and the inline `` uses `object-contain` which scales UP to fill the frame — so a 46×20 shields badge renders as a giant blurry block. The same message renders at the natural size on the second view because `rememberDecodedImageDimensions` populates the cache, but the inconsistency between first and subsequent view is the bug. Switches `object-contain` → `object-scale-down` on `ProgressiveImage.tsx`'s shared `IMAGE_CLASS`. Both `object-contain` and `object-scale-down` scale down to fit, but only `object-scale-down` also refuses to scale up — a small image now sits centered in the reserve box on first view, never upscaled. The layout-shift guarantee for large images is preserved (they still scale down to fit the reserve), matching the issue's stated constraint. Chose option 2 ("Alternatively/minimally") from the issue over option 1 because: - It's a one-class-name change with zero new state, zero new effects, and zero risk to the existing layout-shift guarantee that the surrounding `useFrozenImageReserve` is explicitly designed to preserve. - Option 1 (collapse the frame on decode) is the issue author's preferred direction but requires deciding whether the layout shift is acceptable *shrink*-only — a direction question that should be confirmed with the maintainer first. The issue explicitly says "Happy to open a PR for option 1 if the direction is acceptable" — that's the sign-off step, not this PR. If maintainers want the full first-view-equals-second-view behavior, option 1 is a follow-up that builds on this base. Not added: a regression test. The current desktop test infra (`desktop/src/shared/ui/markdown/markdown.test.mjs` and siblings) doesn't cover `ProgressiveImage` directly; a DOM-style snapshot would need a new test harness. The change is one CSS class name; the visual regression is plain to eyeball in the screenshots CI already captures. Verified: - `pnpm check` (biome + file-sizes + px-text + pubkey-truncation) — clean - `npx tsc --noEmit` — clean Signed-off-by: kushaim Co-authored-by: kushaim --- desktop/src/shared/ui/markdown/ProgressiveImage.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/desktop/src/shared/ui/markdown/ProgressiveImage.tsx b/desktop/src/shared/ui/markdown/ProgressiveImage.tsx index 1f2cb42706..aca0ee8e12 100644 --- a/desktop/src/shared/ui/markdown/ProgressiveImage.tsx +++ b/desktop/src/shared/ui/markdown/ProgressiveImage.tsx @@ -2,8 +2,11 @@ import * as React from "react"; import { cn } from "@/shared/lib/cn"; +// `object-scale-down` (not `object-contain`) so small dim-less badges don't +// upscale to fill the reserve box on first view — the decoded size only +// stabilizes after the cache hits. See #4384 for the full trade-off. const IMAGE_CLASS = - "absolute inset-0 block h-full w-full rounded-2xl object-contain"; + "absolute inset-0 block h-full w-full rounded-2xl object-scale-down"; function isSameImageSource( left: string | undefined,