Skip to content

Add support for cell margins (w:tcMar) in Cell - #81

Merged
thebassplayer1234 merged 2 commits into
developfrom
support-cell-margins
Jul 15, 2026
Merged

Add support for cell margins (w:tcMar) in Cell#81
thebassplayer1234 merged 2 commits into
developfrom
support-cell-margins

Conversation

@thebassplayer1234

@thebassplayer1234 thebassplayer1234 commented Jul 7, 2026

Copy link
Copy Markdown

This pr add support for table cell prop <w:tcMar>.

@thebassplayer1234
thebassplayer1234 marked this pull request as ready for review July 9, 2026 07:10
@thebassplayer1234 thebassplayer1234 self-assigned this Jul 9, 2026

@luiisgallego luiisgallego left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Take a look to my comments :)

Comment thread lib/components/document/src/Cell.ts Outdated
"insideV": docxml:ct-border(${QNS.w}insideV)
},
"margin": ./${QNS.w}tcPr/${QNS.w}tcMar/map {
"top": if (${QNS.w}top/@${QNS.w}w)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is the condition really needed? I think you can call 'docxml:length' directly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

: null,
margin:
tcpr.margin &&
(tcpr.margin.top ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This code is strange, follow "change" prop idea.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Improved

"insideV": docxml:ct-border(${QNS.w}insideV)
},
"margin": ./${QNS.w}tcMar/map {
"top": if (${QNS.w}top/@${QNS.w}w)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove conditions if they are not fully required

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

* Each side is a {@link Length}. Omitted sides fall back to the table default.
*/
margin?: null | {
top?: null | Length;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Documentation?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The documentation for this prop is in line 48 to 53

* both `w:start` and the legacy `w:left` are accepted when reading.
*/
start?: null | Length;
bottom?: null | Length;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Documentation?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The documentation for this prop is in line 48 to 53

@harvestcore harvestcore left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@thebassplayer1234
thebassplayer1234 merged commit f5f0e5b into develop Jul 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants