SD-2281 - fix: page number on footer not properly aligned#2725
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f92d0f8016
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f92d0f8 to
7b0ba2e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
caio-pizzol
left a comment
There was a problem hiding this comment.
hey @chittolinag, thanks for this! footers look much better. tested against real Word and found one thing, left inline. withdrawing the ?? 0 flag from last round, measured it and no visible bug. two test gaps from round 1 still stand.
side note: Word doesn't render the footer on page 1 of this repro, we do. separate issue (probably anchor or pagination related), worth a follow-up ticket.
|
@chittolinag any updates here? |
- normalize-header-footer-fragments: outer Math.max(0, ...) clamp on band origin matches the renderer's behavior, so malformed footerDistance > pageHeight produces 0 rather than a negative origin. - PresentationEditor: only spread margins.footer when the source defines w:footer. Defaulting to 0 here would defeat the bottom-margin fallback in computeFooterBandOrigin (typeof 0 === 'number' passes the check, returning pageHeight - 0).
4b4d3f1 to
36eedd5
Compare
caio-pizzol
left a comment
There was a problem hiding this comment.
hey @chittolinag! pushed two commits since main had drifted: the merge with conflicts resolved, plus the two small fixes from the prior review.
lgtm.
|
🎉 This PR is included in superdoc v1.30.0-next.8 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.8.0-next.23 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.8 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.4 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.49 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.51 |
|
🎉 This PR is included in superdoc v1.30.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.8.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0 |
|
🎉 This PR is included in @superdoc-dev/react v1.3.0 The release is available on GitHub release |
Issue
Floating page-number text boxes overlapped footer text because page-relative anchors were normalized against the bottom margin instead of the footer distance, with the incorrect origin cascading into DomPainter and cache reuse.
Proposed solution
Thread the footer distance through header/footer constraints, normalize footer anchors using that distance (with a bottom-margin fallback), update cache hashing to include the new geometry, and teach DomPainter to convert page-relative footer anchors using the same origin so floating page numbers sit beneath footer content.