-
Notifications
You must be signed in to change notification settings - Fork 99
Better spacing for paragraphs with borders/shades #2554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ import type { | |
| ImageFragmentMetadata, | ||
| DrawingBlock, | ||
| DrawingMeasure, | ||
| DrawingFragment, | ||
| DrawingFragment, ParagraphBorders | ||
| } from '@superdoc/contracts'; | ||
| import { | ||
| computeFragmentPmRange, | ||
|
|
@@ -23,6 +23,7 @@ import { | |
| } from './layout-utils.js'; | ||
| import { computeAnchorX } from './floating-objects.js'; | ||
| import { getFragmentZIndex } from '@superdoc/pm-adapter/utilities.js'; | ||
| import { PX_PER_PT } from '@superdoc/pm-adapter/constants.js'; | ||
|
|
||
| const spacingDebugEnabled = false; | ||
| /** | ||
|
|
@@ -89,6 +90,8 @@ type ParagraphBlockAttrs = { | |
| floatAlignment?: unknown; | ||
| /** Keep all lines of the paragraph on the same page */ | ||
| keepLines?: boolean; | ||
| /** Border attributes for the paragraph */ | ||
| borders?: ParagraphBorders; | ||
| }; | ||
|
|
||
| const spacingDebugLog = (..._args: unknown[]): void => { | ||
|
|
@@ -162,6 +165,28 @@ const asSafeNumber = (value: unknown): number => { | |
| return value; | ||
| }; | ||
|
|
||
| /** | ||
| * Computes the vertical border expansion for a paragraph fragment. | ||
| * The border's `space` attribute (in points) plus the border width extends | ||
| * the visual box beyond the content area. This ensures cursorY accounts | ||
| * for the full visual height when paragraphs have borders with space. | ||
| */ | ||
| const computeBorderVerticalExpansion = (borders?: ParagraphBorders): { top: number; bottom: number } => { | ||
| if (!borders) return { top: 0, bottom: 0 }; | ||
|
|
||
| // Top border: space (pts) + width (px) | ||
| const topSpace = (borders.top?.space ?? 0) * PX_PER_PT; | ||
| const topWidth = borders.top?.width ?? 0; | ||
| const top = topSpace + topWidth; | ||
|
|
||
| // Bottom border: space (pts) + width (px) | ||
| const bottomSpace = (borders.bottom?.space ?? 0) * PX_PER_PT; | ||
| const bottomWidth = borders.bottom?.width ?? 0; | ||
| const bottom = bottomSpace + bottomWidth; | ||
|
|
||
| return { top, bottom }; | ||
| }; | ||
|
|
||
| /** | ||
| * Calculates the first line indent for list markers when remeasuring paragraphs. | ||
| * | ||
|
|
@@ -785,14 +810,15 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para | |
| // Expand width: negative indents on both sides expand the fragment width | ||
| // (e.g., -48px left + -72px right = 120px wider) | ||
| const adjustedWidth = effectiveColumnWidth - negativeLeftIndent - negativeRightIndent; | ||
|
|
||
| // Account for border space + width that extends the visual box | ||
| const borderExpansion = computeBorderVerticalExpansion(attrs?.borders); | ||
| const fragment: ParaFragment = { | ||
| kind: 'para', | ||
| blockId: block.id, | ||
| fromLine, | ||
| toLine: slice.toLine, | ||
| x: adjustedX, | ||
| y: state.cursorY, | ||
| y: state.cursorY + borderExpansion.top, | ||
| width: adjustedWidth, | ||
| ...computeFragmentPmRange(block, lines, fromLine, slice.toLine), | ||
| }; | ||
|
|
@@ -838,9 +864,9 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para | |
| fragment.x = columnX(state.columnIndex) + offsetX + (effectiveColumnWidth - maxLineWidth) / 2; | ||
| } | ||
| } | ||
|
|
||
| state.page.fragments.push(fragment); | ||
| state.cursorY += fragmentHeight; | ||
|
|
||
| state.cursorY += fragmentHeight + borderExpansion.top + borderExpansion.bottom; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. border space and paragraph spacing are stacking here — but Word treats them as overlapping. the spec says border space is measured from the text, ignoring paragraph spacing (ECMA-376 §17.3.1.42). tested with a doc that has borders (space=4) + 10pt spacing: SuperDoc adds ~6px extra per side vs Word. that adds up to the ~13px gap you mentioned in the PR description. fix direction: make them overlap (use the bigger one) instead of adding both.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @caiopizzol , thanks for the thorough review. I actually read through quite a bit of ECMA-376 as part of this. Really complex issue!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, for your work on this! |
||
| lastState = state; | ||
| fromLine = slice.toLine; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
layoutParagraphBlocknow offsets paragraph fragments byborderExpansion.top, but the slice fit still usesstate.contentBottom - state.cursorY(text-only space). When a bordered paragraph starts near the page bottom,sliceLinescan accept lines that fit only without the new top offset, so the rendered text is pushed below the page’s content area and gets clipped (overflow: hiddenon pages). The same mismatch also allowscursorYto advance pastcontentBottomafter placing a “fitting” slice. Reserve top/bottom border expansion before slicing (or include it in the fit checks) so the fragment only takes lines that truly fit.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johanneswilm real issue — bordered paragraphs near the page bottom can overflow since the border size isn't subtracted from the available space before fitting lines.