From 5755c8cd8835952939bdb4bb2ce060a0eda2e67a Mon Sep 17 00:00:00 2001 From: Fini Date: Mon, 11 May 2026 00:23:08 +0800 Subject: [PATCH] fix(pen-core): preserve cornerRadius on media-clipping frames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex stop-hook on the prior strip-nested-card-decoration commit caught a regression: cornerRadius on a media-clipping frame (`clipContent: true` wrapping an image / video, or roles like `image-placeholder` / `thumbnail` / `cover-image`) is doing the rounding work for the photo, not stacking card decoration. Blanket stripping un-rounded the media against the user's clear intent — typical pattern is card { cornerRadius: 16, clipContent: true } └─ image-placeholder { cornerRadius: 12, clipContent: true } └─ image where the inner cornerRadius rounds the photo and the outer rounds the card frame around it. After the prior pass the inner radius got stripped (ancestor had cornerRadius too) → square corners on the photo. New `MEDIA_CLIP_ROLES` set + `isMediaClipper(node)` helper: - role match: image, image-card, image-placeholder, video, video-placeholder, media, media-thumbnail, thumbnail, cover, cover-image, gallery-item - shape match: clipContent: true AND has a direct image / video / media-roled child Either signal preserves cornerRadius. Other decorations (stroke, shadow) still get stripped — those ARE redundant card decoration even on a media wrapper, since the photo's own outline + the ancestor card already provide the visual frame. Tests: 2 new cases — clipContent + image, and the role-only path covering image-placeholder / thumbnail / cover-image / gallery-item. --- .../strip-nested-card-decoration.test.ts | 56 +++++++++++++++++++ .../layout/strip-nested-card-decoration.ts | 50 ++++++++++++++++- 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/packages/pen-core/src/__tests__/strip-nested-card-decoration.test.ts b/packages/pen-core/src/__tests__/strip-nested-card-decoration.test.ts index 2b8e91a0b..b7203a680 100644 --- a/packages/pen-core/src/__tests__/strip-nested-card-decoration.test.ts +++ b/packages/pen-core/src/__tests__/strip-nested-card-decoration.test.ts @@ -189,6 +189,62 @@ describe('stripNestedCardDecoration', () => { expect((inner as PenNode & { cornerRadius?: unknown }).cornerRadius).toBeUndefined(); }); + // 2026-05-11 Codex stop-hook caught: cornerRadius on a media-clipping + // frame (clipContent: true wrapping an image / video) is doing the + // rounding work, not stacking card decoration. Stripping it would + // un-round the photo against the user's intent. + + it('preserves cornerRadius on a clipContent frame wrapping an image', () => { + const image = frame({ id: 'img', type: 'image' as const } as never); + const clipper = frame({ + id: 'clipper', + cornerRadius: 12, + clipContent: true, + stroke: stroke(), + effects: shadow(), + children: [image], + } as never); + const card = frame({ + id: 'card', + role: 'card', + cornerRadius: 16, + stroke: stroke(), + effects: shadow(), + children: [clipper], + } as never); + + stripNestedCardDecoration(card); + + // cornerRadius preserved (media clip role). + expect((clipper as PenNode & { cornerRadius?: unknown }).cornerRadius).toBe(12); + // stroke + shadow still get stripped (those are pure decoration). + expect((clipper as PenNode & { stroke?: unknown }).stroke).toBeUndefined(); + expect((clipper as PenNode & { effects?: unknown }).effects).toBeUndefined(); + }); + + it('preserves cornerRadius on frames with media role (image-placeholder, thumbnail, cover)', () => { + for (const role of ['image-placeholder', 'thumbnail', 'cover-image', 'gallery-item']) { + const inner = frame({ + id: `inner-${role}`, + role, + cornerRadius: 8, + } as never); + const card = frame({ + id: 'card', + role: 'card', + cornerRadius: 16, + children: [inner], + } as never); + + stripNestedCardDecoration(card); + + expect( + (inner as PenNode & { cornerRadius?: unknown }).cornerRadius, + `${role} should retain cornerRadius`, + ).toBe(8); + } + }); + it('returns false when nothing was modified', () => { const card = frame({ id: 'plain-card', diff --git a/packages/pen-core/src/layout/strip-nested-card-decoration.ts b/packages/pen-core/src/layout/strip-nested-card-decoration.ts index 03a0276e4..fa1f567cc 100644 --- a/packages/pen-core/src/layout/strip-nested-card-decoration.ts +++ b/packages/pen-core/src/layout/strip-nested-card-decoration.ts @@ -56,6 +56,30 @@ const KEEP_DECORATION_ROLES = new Set([ 'segmented-control', ]); +/** + * Roles that semantically carry MEDIA — their cornerRadius is doing + * clipping work (rounded photo / video / thumbnail) rather than + * card-style decoration. Stripping the radius would un-round the + * media against the user's clear intent. Codex 2026-05-11 stop-hook + * caught the regression: a card with cornerRadius wrapping an + * image-placeholder would have the placeholder's own cornerRadius + * (used for `clipContent: true` rounding) stripped → square corners + * on the photo even though the surrounding card was rounded. + */ +const MEDIA_CLIP_ROLES = new Set([ + 'image', + 'image-card', + 'image-placeholder', + 'video', + 'video-placeholder', + 'media', + 'media-thumbnail', + 'thumbnail', + 'cover', + 'cover-image', + 'gallery-item', +]); + interface DecoFlags { hasStroke: boolean; hasCornerRadius: boolean; @@ -82,6 +106,26 @@ function isRoleProtected(node: PenNode): boolean { return KEEP_DECORATION_ROLES.has(role); } +/** + * Frames acting as media-clipping containers — `clipContent: true` plus + * a direct image / video child mean the cornerRadius is rounding the + * photo, not stacking card decoration. Detector preserves cornerRadius + * on these (other decorations still get stripped if redundant). + */ +function isMediaClipper(node: PenNode): boolean { + const role = ((node as { role?: string }).role ?? '').toLowerCase(); + if (MEDIA_CLIP_ROLES.has(role)) return true; + const clipContent = (node as PenNode & { clipContent?: unknown }).clipContent; + if (clipContent !== true) return false; + if (!('children' in node) || !Array.isArray(node.children)) return false; + return node.children.some((c) => { + if (!c) return false; + if (c.type === 'image') return true; + const childRole = ((c as { role?: string }).role ?? '').toLowerCase(); + return MEDIA_CLIP_ROLES.has(childRole); + }); +} + /** * Returns true if any node was modified. */ @@ -105,6 +149,7 @@ export function stripNestedCardDecoration(root: PenNode): boolean { ); const own = readDecoration(node); const isProtected = isRoleProtected(node); + const mediaClipper = isMediaClipper(node); if (!isProtected) { const n = node as PenNode & { stroke?: unknown; @@ -115,7 +160,10 @@ export function stripNestedCardDecoration(root: PenNode): boolean { delete n.stroke; changed = true; } - if (own.hasCornerRadius && ancestorDeco.hasCornerRadius) { + // Preserve cornerRadius on media-clipping frames — the radius is + // rounding the photo, not stacking card decoration. Other + // decorations (stroke / shadow) still get stripped if redundant. + if (own.hasCornerRadius && ancestorDeco.hasCornerRadius && !mediaClipper) { delete n.cornerRadius; changed = true; }