Conversation
superdoc-bot
bot
added
risk: critical
risk: sensitive
and removed
risk: critical
labels
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@VladaHarbour looks good overall. left a few small inline comments.
one test comment at column-balancing.test.ts:293 describes a check that doesn't exist — easy fix.
normalizeColumnsForFootnotes and normalizeColumns are mostly the same ~30 lines — a shared helper would keep them in sync.
on tests: widthsEqual and cloneColumnLayout have a few branches but no unit tests. also nothing covers the case where two sections share count/gap but differ only in widths or equalWidth.
packages/layout-engine/layout-engine/src/column-balancing.test.ts
Outdated
Show resolved
Hide resolved
| @@ -128,12 +129,40 @@ const COLUMN_EPSILON = 0.01; | |||
| type NormalizedColumns = ColumnLayout & { width: number }; | |||
| type PageColumns = NormalizedColumns & { left: number; contentWidth: number }; | |||
|
|
|||
There was a problem hiding this comment.
this function is almost copy-pasted from normalizeColumns in index.ts:2567. could they share one helper?
There was a problem hiding this comment.
created shared helper
| type NormalizedColumns = ColumnLayout & { width: number }; | ||
| type PageColumns = NormalizedColumns & { left: number; contentWidth: number }; | ||
|
|
||
| const resolveMaxColumnWidth = (contentWidth: number, columns?: ColumnLayout): number => { |
There was a problem hiding this comment.
footnotes are always measured using the widest column's width. if a footnote lands in a narrower column, its wrapping might be slightly off. maybe add a // TODO so it doesn't catch someone off guard later?
3 participants
Initial issue:
SuperDoc does not match Word layout for early pages in a document that combines: