another fix for showHeaderOnEveryPage #264
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
account for space.header and space.footer in page margins calculations
if table has a header that repeats on everypage, make sure the cells are shifted by headerHeight and adjust the position of the size wrt minOffset. also add table padding to the surrounding cells
moored
reviewed
Jan 12, 2021
| headerShift = false | ||
| } | ||
| //add table padding around cells | ||
| nextPageCells = shiftCellsBy(cells: nextPageCells, shiftValue: table.padding) |
There was a problem hiding this comment.
Line 342 adds additional spacing for me between the header and the first row on pages > 1. If I comment it out it works fine.
philprime
reviewed
Jan 22, 2021
| @@ -450,7 +461,22 @@ internal class PDFTableObject: PDFRenderObject { | |||
| } | |||
| return result | |||
| } | |||
|
|
|||
|
|
|||
| internal typealias ShiftedCells = ([PDFTableCalculatedCell]) | |||
There was a problem hiding this comment.
Suggested change
| internal typealias ShiftedCells = ([PDFTableCalculatedCell]) | |
| internal typealias ShiftedCells = [PDFTableCalculatedCell] |
It is not necessary to wrap the array in parantheses and that might lead to side effects (it would be a single value tuple)
philprime
reviewed
Jan 22, 2021
| @@ -284,6 +284,7 @@ internal class PDFTableObject: PDFRenderObject { | |||
| let startPosition: CGPoint = cells.first?.frames.cell.origin ?? .zero | |||
| var nextPageCells: [PDFTableCalculatedCell] = cells | |||
| var pageEnd = CGPoint.null | |||
| var headerShift = true | |||
There was a problem hiding this comment.
should this be a hardcoded value or should it be connected to showHeaderOnEveryPage or shouldSplitCellsOnPageBreak?
fixed the spacing between the first row and the table header on rest of the pages (after firstpage)
fixed the PDFTableObject test
adjust the offset in TableObjectSpec test
code cleanup as per @philprime Co-Authored-By: Philip Niedertscheider <dev@philpri.me>
adjusted the expected value of the teableobkect test Co-Authored-By: Philip Niedertscheider <dev@philpri.me>
Codecov Report
@@ Coverage Diff @@
## develop #264 +/- ##
===========================================
+ Coverage 56.77% 57.48% +0.70%
===========================================
Files 122 122
Lines 4329 4311 -18
===========================================
+ Hits 2458 2478 +20
+ Misses 1871 1833 -38
Continue to review full report at Codecov.
|
merged with docs directory which i shouldn't :( so removing it from my fork
philprime
pushed a commit
that referenced
this pull request
Aug 26, 2021
* Update PDFCalculations.swift account for space.header and space.footer in page margins calculations * Update PDFTableObject.swift - if table has a header that repeats on everypage, make sure the cells are shifted by headerHeight and adjust the position of the size wrt minOffset. also add table padding to the surrounding cells - fixed the spacing between the first row and the table header on rest of the pages (after firstpage) * Update PDFTableObjectSpec.swift Co-authored-by: Philip Niedertscheider <dev@philpri.me>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[sorry, originally submitted the PR against master not develop and didn't know how to undo this.. so resubmitting against develop]
looping back on the open issue #222 and some code changes i am proposing to address it (in line with #262 by David Moore). code is meant to fix some missing offset to line up headers and table spacing on every page. code can benefit from some refactoring but just tested it in 2.3.3 and looks good .