feat: support rendering of line between columns#2743
feat: support rendering of line between columns#2743Abdeltoto wants to merge 1 commit intosuperdoc-dev:mainfrom
Conversation
Parse the w:sep attribute and child element on w:cols in section properties to detect the "line between" column setting. Carry the lineBetween flag through ColumnLayout, SectionBreakBlock, and Page types so the DomPainter can render vertical separator lines in the inter-column gaps. Closes superdoc-dev#2067 Made-with: Cursor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb1d82966f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (activeColumns && activeColumns.count > 1) { | ||
| page.columns = activeColumns; |
There was a problem hiding this comment.
page.columns = activeColumns assumes the new lineBetween flag survives section-state cloning, but cloneColumnLayout currently drops that field (packages/layout-engine/contracts/src/column-layout.ts, function cloneColumnLayout). As a result, page.columns.lineBetween is typically undefined even when <w:cols w:sep="1"> is parsed, so the separator rendering path never activates. Please propagate lineBetween through the column clone/state pipeline before assigning it to the page.
Useful? React with 👍 / 👎.
| // Render vertical separator lines between columns when lineBetween is enabled | ||
| if (page.columns?.lineBetween && page.columns.count > 1 && page.margins && this.doc) { | ||
| const cols = page.columns; |
There was a problem hiding this comment.
This separator logic was added only to renderPage, but the default vertical/non-book flow renders pages via fullRender/patchLayout/virtualization, which build DOM through createPageState instead of renderPage (same file). In those modes, separators are never created, so users in standard vertical mode won’t see the new line-between-columns feature. The separator rendering needs to be shared with or duplicated in the createPageState path.
Useful? React with 👍 / 👎.
2 participants
Summary
w:sepattribute and child element onw:colsin section properties to detect the line-between column settinglineBetweenflag toColumnLayout,SectionBreakBlock.columns, andPagetypes in contractslineBetweenthrough the pm-adapter extraction and layout engine page creationlineBetweenis enabledFiles changed
contracts/src/index.ts--lineBetweenonColumnLayout,SectionBreakBlock,Pagesuper-converter/section-properties.js-- parsew:sepattribute and child elementpm-adapter/src/sections/extraction.ts-- extractlineBetweenfromw:colslayout-engine/src/index.ts-- carryactiveColumnsto pagepainters/dom/src/renderer.ts-- render vertical lines in inter-column gapsTest plan
Closes #2067