Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion packages/marknative/src/layout/block/layout-document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,25 @@ function layoutList(node: ListNode, context: LayoutContext): LayoutResult<ListFr
}
}

function parseWidth(width: number | string, containerWidth: number): number {
if (typeof width === 'number') {
return width > 0 ? width : containerWidth
}
Comment on lines +229 to +231
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseWidth currently allows numeric widths greater than containerWidth (and percentages > 100%) to pass through unchanged, which can cause code blocks to render outside the page content area. If overflow isn’t intended, consider clamping parsed widths to [1, containerWidth] (and similarly clamping computed percentages).

Copilot uses AI. Check for mistakes.
if (width === 'fit-content') {
return -1
Comment on lines +228 to +233
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseWidth uses -1 as a sentinel for 'fit-content' but the function is typed to return number, making the contract implicit and easy to misuse later. Consider returning null/undefined (and updating the return type) or a tagged union so callers are forced to handle the 'fit-content' case explicitly.

Suggested change
function parseWidth(width: number | string, containerWidth: number): number {
if (typeof width === 'number') {
return width > 0 ? width : containerWidth
}
if (width === 'fit-content') {
return -1
function parseWidth(width: number | string, containerWidth: number): number | undefined {
if (typeof width === 'number') {
return width > 0 ? width : containerWidth
}
if (width === 'fit-content') {
return undefined

Copilot uses AI. Check for mistakes.
}
// percentage, e.g. "50%", "80%"
const match = width.match(/^(\d+(?:\.\d+)?)%$/)
if (match && match[1]) {
const percentage = parseFloat(match[1])
if (!isNaN(percentage) && percentage > 0) {
return (containerWidth * percentage) / 100
}
}

return containerWidth
}

function layoutBlockquote(node: BlockquoteNode, context: LayoutContext): LayoutResult<BlockquoteFragment> {
const padding = context.theme.blocks.quote.padding
// The line box has ~32% of lineHeight as dead space above the first visible glyph
Expand Down Expand Up @@ -261,6 +280,7 @@ function layoutBlockquote(node: BlockquoteNode, context: LayoutContext): LayoutR

function layoutCode(node: CodeBlockNode, context: LayoutContext): LayoutResult<CodeFragment> {
const padding = context.theme.blocks.code.padding
const width = context.theme.blocks.code.width
const lineHeight = context.theme.typography.code.lineHeight
const lineWidth = Math.max(1, context.width - padding * 2)
const codeTheme = withBodyTypography(context.theme, context.theme.typography.code)
Comment on lines +283 to 286
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

layoutCode calculates lineWidth from context.width before applying the new theme.blocks.code.width. If a theme sets code width smaller than the container (e.g. 50% or a number), the code lines will be laid out with a wider max width than the final box, so lines can extend outside box.width (background/outline) and violate the existing invariant that line.width <= box.width - padding*2. Consider computing the desired box width first (for non-fit-content values) and deriving lineWidth from that width (fall back to container width only for fit-content).

Copilot uses AI. Check for mistakes.
Expand All @@ -269,6 +289,7 @@ function layoutCode(node: CodeBlockNode, context: LayoutContext): LayoutResult<C
const lines: LineBox[] = []
const lineSourceMap: number[] = []
let cursorY = context.y + padding
let boxWidth = 0

sourceLines.forEach((sourceLine, sourceLineIndex) => {
const hlLine = highlighted?.lines[sourceLineIndex]
Expand All @@ -281,6 +302,7 @@ function layoutCode(node: CodeBlockNode, context: LayoutContext): LayoutResult<C

if (laidOut.length > 0) {
for (const line of laidOut) {
if (line.width > boxWidth) boxWidth = line.width
lines.push(offsetLineBox(line, context.x + padding, cursorY - line.y))
lineSourceMap.push(sourceLineIndex)
cursorY += line.height
Expand All @@ -293,10 +315,13 @@ function layoutCode(node: CodeBlockNode, context: LayoutContext): LayoutResult<C
cursorY += lineHeight
})

const parsedWidth = parseWidth(width, context.width)
const finalWidth = parsedWidth === -1 ? boxWidth + padding * 2 : parsedWidth

const box = {
x: context.x,
y: context.y,
width: context.width,
width: finalWidth,
height: cursorY - context.y + padding,
}

Expand Down
4 changes: 2 additions & 2 deletions packages/marknative/src/theme/default-theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export type Theme = {
paragraph: { marginBottom: number }
heading: { marginTop: number; marginBottom: number }
list: { marginBottom: number; itemGap: number; indent: number }
code: { marginBottom: number; padding: number }
code: { marginBottom: number; padding: number, width: number | string }
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding blocks.code.width as a required property in the exported Theme type is a breaking change for downstream consumers that construct Theme objects directly. Consider making it optional (width?: …) with a default fallback to full container width in layout, and keep the type literal formatting consistent (use ; between properties rather than ,).

Suggested change
code: { marginBottom: number; padding: number, width: number | string }
code: { marginBottom: number; padding: number; width?: number | string }

Copilot uses AI. Check for mistakes.
quote: { marginBottom: number; padding: number }
table: { marginBottom: number; cellPadding: number }
image: { marginBottom: number }
Expand Down Expand Up @@ -143,7 +143,7 @@ export const defaultTheme: Theme = {
paragraph: { marginBottom: 24 },
heading: { marginTop: 40, marginBottom: 12 },
list: { marginBottom: 24, itemGap: 8, indent: 36 },
code: { marginBottom: 24, padding: 24 },
code: { marginBottom: 24, padding: 24, width: '100%' },
quote: { marginBottom: 16, padding: 12 },
table: { marginBottom: 24, cellPadding: 16 },
image: { marginBottom: 24 },
Expand Down
53 changes: 53 additions & 0 deletions packages/marknative/tests/layout/blocks/parseWidth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { test, expect } from "bun:test";
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new test doesn’t follow the established test style in this package (single quotes and no trailing semicolons, e.g. tests/layout/blocks/layout-blocks.test.ts:1). To keep formatting consistent (and avoid formatter churn), update imports and test names to use single quotes and drop semicolons.

Copilot uses AI. Check for mistakes.

// Copy parseWidth function for testing
function parseWidth(width: number | string, containerWidth: number): number {
if (typeof width === 'number') {
return width > 0 ? width : containerWidth
}
if (width === 'fit-content') {
return -1 // Special marker for caller to handle
}
// Support percentage format like "50%", "80%"
const match = width.match(/^(\d+(?:\.\d+)?)%$/)
if (match && match[1]) {
const percentage = parseFloat(match[1])
if (!isNaN(percentage) && percentage > 0) {
return (containerWidth * percentage) / 100
}
}
// Default to container width
return containerWidth
}

test("parseWidth: number > 0 returns the number itself", () => {
expect(parseWidth(100, 500)).toBe(100)
expect(parseWidth(300, 500)).toBe(300)
})

test("parseWidth: number <= 0 returns container width", () => {
expect(parseWidth(0, 500)).toBe(500)
expect(parseWidth(-10, 500)).toBe(500)
})

test("parseWidth: fit-content returns -1", () => {
expect(parseWidth('fit-content', 500)).toBe(-1)
})

test("parseWidth: percentage strings", () => {
expect(parseWidth('50%', 500)).toBe(250)
expect(parseWidth('100%', 500)).toBe(500)
expect(parseWidth('25%', 400)).toBe(100)
})

test("parseWidth: decimal percentages", () => {
expect(parseWidth('33.33%', 300)).toBeCloseTo(99.99)
expect(parseWidth('66.66%', 300)).toBeCloseTo(199.98)
})

test("parseWidth: invalid percentages return container width", () => {
expect(parseWidth('abc', 500)).toBe(500)
expect(parseWidth('50', 500)).toBe(500)
expect(parseWidth('50 px', 500)).toBe(500)
expect(parseWidth('', 500)).toBe(500)
})
Comment on lines +1 to +53
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test file copies parseWidth instead of testing the implementation used by layoutDocument/layoutCode, so it can pass even if the production logic regresses or diverges. Consider moving parseWidth into a shared exported utility (or testing via layoutDocument with a theme override like blocks.code.width: 'fit-content' | '50%') so the tests validate the actual behavior change end-to-end.

Suggested change
import { test, expect } from "bun:test";
// Copy parseWidth function for testing
function parseWidth(width: number | string, containerWidth: number): number {
if (typeof width === 'number') {
return width > 0 ? width : containerWidth
}
if (width === 'fit-content') {
return -1 // Special marker for caller to handle
}
// Support percentage format like "50%", "80%"
const match = width.match(/^(\d+(?:\.\d+)?)%$/)
if (match && match[1]) {
const percentage = parseFloat(match[1])
if (!isNaN(percentage) && percentage > 0) {
return (containerWidth * percentage) / 100
}
}
// Default to container width
return containerWidth
}
test("parseWidth: number > 0 returns the number itself", () => {
expect(parseWidth(100, 500)).toBe(100)
expect(parseWidth(300, 500)).toBe(300)
})
test("parseWidth: number <= 0 returns container width", () => {
expect(parseWidth(0, 500)).toBe(500)
expect(parseWidth(-10, 500)).toBe(500)
})
test("parseWidth: fit-content returns -1", () => {
expect(parseWidth('fit-content', 500)).toBe(-1)
})
test("parseWidth: percentage strings", () => {
expect(parseWidth('50%', 500)).toBe(250)
expect(parseWidth('100%', 500)).toBe(500)
expect(parseWidth('25%', 400)).toBe(100)
})
test("parseWidth: decimal percentages", () => {
expect(parseWidth('33.33%', 300)).toBeCloseTo(99.99)
expect(parseWidth('66.66%', 300)).toBeCloseTo(199.98)
})
test("parseWidth: invalid percentages return container width", () => {
expect(parseWidth('abc', 500)).toBe(500)
expect(parseWidth('50', 500)).toBe(500)
expect(parseWidth('50 px', 500)).toBe(500)
expect(parseWidth('', 500)).toBe(500)
})
import { test } from "bun:test";
test.todo(
"layoutDocument/layoutCode applies a positive numeric blocks.code.width as an explicit width",
);
test.todo(
"layoutDocument/layoutCode falls back to the container width when blocks.code.width is 0 or negative",
);
test.todo(
"layoutDocument/layoutCode preserves fit-content behavior when blocks.code.width is 'fit-content'",
);
test.todo(
"layoutDocument/layoutCode supports percentage widths such as '50%' and '100%' via the production parser",
);
test.todo(
"layoutDocument/layoutCode supports decimal percentage widths such as '33.33%' via the production parser",
);
test.todo(
"layoutDocument/layoutCode falls back to the container width for invalid width strings",
);

Copilot uses AI. Check for mistakes.
Loading