problem with showHeadersOnEveryPage

[Sgkhour]

Hi , i am using TPPDF 2.2.0 and integrating in a personal iOS app i am writing . a great library , thank you but i am trying to understand showHeadersOnEveryPage
when showHeadersOnEveryPage is true, the header row overwrites the top row on the next page and ends up hiding it.
i don't see any properties to add space after the header row. being new, i might have overlooked something

here are two screenshots.. without a header and with. the 20:12 entry is overwritten by the header row

thank you
Sami

[philprime]

Hi Sami,

looks like this is the same issue as #205, actually a bug from my side.

Is it possible you can create me a small code sample I can run locally to reproduce your issue?
It would help tremendously.

[Sgkhour]

Philip, thank you for the followup, here is my generateDocument function , trimmed to the bare minimum but with the style parameters i used. only thing you need is a reference to a uiimage inside the row loop. the third column is the row number. and you can see it jumps from 24 to 26 (25 being overwritten by the header. i suspect it is an offset issue since regardless of showheader being true or false, row 25 is always at the same spot on the page. let me know if you need anything else , happy to help. Sami

 func generateDocument() -> [PDFDocument] {
        var timeString: String = ""
        var imgName = UIImage()
        var volume: String = ""
        var description: String = ""
        
        let numberOfRowns = 35
        let document = PDFDocument(format: .usLetter)

        let eventsTable = PDFTable(rows: numberOfRowns + 1, columns: 4)
        eventsTable.widths = [0.1, 0.05, 0.1, 0.75]
        eventsTable.padding = 1.0
        eventsTable.margin = 2.0

        eventsTable.showHeadersOnEveryPage = true

        let eventsTableTopRowStyle = PDFTableCellStyle.init(colors: (fill: Color.systemBlue,
                                                          text: Color.white),
                                                 borders: PDFTableCellBorders(left: PDFLineStyle(type: .none),
                                                                              top: PDFLineStyle(type: .none),
                                                                              right: PDFLineStyle(type: .none),
                                                                              bottom: PDFLineStyle(type: .none)),
                                                 font: Font.boldSystemFont(ofSize: 10.0))

        let eventsTableContentStyle = PDFTableCellStyle.init(colors: (fill: Color.white,
                                                        text: Color.black),
                                               borders: PDFTableCellBorders(left: PDFLineStyle(type: .none),
                                                                            top: PDFLineStyle(type: .none),
                                                                            right: PDFLineStyle(type: .none),
                                                                            bottom: PDFLineStyle(type: .none)),
                                               font: Font.systemFont(ofSize: 10))

        eventsTable.style = PDFTableStyle(rowHeaderCount: 0,
                                          columnHeaderCount: 1,
                                          footerCount: 0,
                                          columnHeaderStyle: eventsTableTopRowStyle,
                                          contentStyle: eventsTableContentStyle,
                                          alternatingContentStyle: eventsTableContentStyle)

        let myrow = eventsTable[row: 0]
        myrow.content = ["Time", nil, "Vol.", "Description"]
        myrow.alignment = [.center, .center, .center, .center]

        let dateFormat = DateFormatter()
        dateFormat.dateFormat = "HH:mm"

        for ndx in 0..<numberOfRowns {
            timeString = dateFormat.string(from: Date())
            description = "this is a description"
            volume = String(ndx)
            imgName = UIImage(named: "stats")!
              
            let myrow = eventsTable[row: ndx+1]
            myrow.content = [timeString, imgName, volume, description]
            myrow.alignment = [.right, .center, . right, .left]
        }
        
        let titleDateFormat = DateFormatter()
        titleDateFormat.dateStyle = .full
        titleDateFormat.timeStyle = .none

        let docTitleStyle = document.add(style: PDFTextStyle(name: "Title",
                                                          font: UIFont.boldSystemFont(ofSize: 24.0),
                                                          color: UIColor(red: 0.171875, green: 0.2421875, blue: 0.3125, alpha: 1.0)))
 
        document.add(textObject: PDFSimpleText(text: titleDateFormat.string(from: Date()), style: docTitleStyle))
        document.add(space: 32.0)

        document.add(table: eventsTable)
        document.add(space: 16.0)

         return [document]
    }

[philprime]

Thanks a lot! I will try to reproduce it in the TPPDF experiment sample, so I can use it for testing and investigating.

[philprime]

Alright, with 2.3.2 it the "shouldSplitCellsOnPageBreak" option should fix at least the half cells.

I am not quite certain that showHeadersOnEveryPage is not working correctly, but that does take more time to analyze (see #250)

[moored]

I am seeing the same behavior with the header overwriting the top cell/cells. Have shouldSplitCellsOnPageBreak = false and showHeadersOnEveryPage = true.

[moored]

Note sure if this is related but the header.spacer is not being enforced on additional pages.

[Sgkhour]

for what its worth, i worked on this issue trying to run it down, created a fork and have coded a workaround. not sure if it is a clean solution but feel free to have a look at it in PDFCalculations.swift and also in PDFTableObject.swift where i added some code to account for handling subsequent pages. i am new to this so if you think it is a solution to the header spacing issue, i could put a pull request...

[moored]

@Sgkhour This does appear to fix the issue, however I don't believe you need line 347, nextPageCells = shiftCellsBy(cells: nextPageCells, shiftValue: table.padding) as it causes additional spacing between the header and the first row on pages > 2. Also, some refactoring is needed since your changes are based on the master branch and not the current develop branch. Very nice work!

[philprime]

I am happy to review a PR. I justed pushed a fix for outdated gem dependencies, so master and develop are on the same commit now. Please rebase any changes before creating a PR

Edit:
Also code-review is easier in a PR as we can comment on specific lines (similar to what @moored just did)

[Sgkhour]

sorry, taking me longer than i hoped to the rebase... new to this and have never done it before so in major learn more today

[philprime]

@Sgkhour is the current PR #264 ready for a review or are you still working on it?

[Sgkhour]

i think it is... i was reviewing @moored comment about cancelling the changes at lines 335-343 in PDFTableObject but according to my tests, they are still needed. i am attaching two PDF with and without . you can see that on the second one, without the changes, row 23 is squished in the header row

Example-with.pdf
Example-without.pdf

i also tested without a header row and i didn't get any negative side effects. hopefully my test cases are exhaustive but feedback welcome as always

[moored]

@Sgkhour It was not 335-343, I was referring to just line 343. If line 343 does not add additional spacing between the header and the first row on pages > 1 for you I say it is good to go.

[philprime]

Looks like the tests are failing, mainly because the table calculations differ. Please check if the tests are still accurate or need to be changed too

[Sgkhour]

@Sgkhour It was not 335-343, I was referring to just line 343. If line 343 does not add additional spacing between the header and the first row on pages > 1 for you I say it is good to go.

thank you @moored .. i did a quick check with and without line 343 and i would suggest we keep it it. When showHeadersOnEveryPage is set to false, this line introduced a tiny padding around the first row of the tables on pages 2 onward. i am attaching a small screenshot (with and without) and aesthetically that tiny space which comes from table.padding helps

[moored]

That looks good, I was seeing a much larger amount of space added after page 1 and is probably something I have in my implementation. How will this look if you have full borders on the cells? Will there be gaps on the second page that are not present on the first page?

[Sgkhour]

@moored , i added a 2pt border all around and reran the previous test and with the table padding (per line 343), there is a gap between the border and the table header. the attached show P1 and P2 .. i tried with a table of 150 rows and page 2-6 are identical and look good. one thing that bugs me is page 1 has a slightly larger gap . i suspect table.padding is accounted for twice . will chase it down and submit a new PR but for now, the rest looks good.

[Sgkhour]

found the culprit :) it was on line 343

@@ -339,7 +339,7 @@ internal class PDFTableObject: PDFRenderObject {
                     headerShift = false
                 }
                 //add table padding around cells
                 nextPageCells = shiftCellsBy(cells: nextPageCells, shiftValue: table.padding)
                 nextPageCells = shiftCellsBy(cells: nextPageCells, shiftValue: table.margin)
             }

             let filterResult = filterCellsOnPage(for: generator,

changed table.padding to table.margin and now the spacing is good on all pages

[philprime]

Hi, I have left you some comments on the pull request a while ago and it seems like the tests are still failing (maybe they are outdated). I am not able to merge your changes until these issues are resolved :)

[Sgkhour]

sorry @philprime , i didn't see your previous comments, i am working on fixing the tests and hopefully will be done this weekend . i might have a clue...

[philprime]

Hi @Sgkhour thanks for the updates and the email you sent me.
I took a shot and cleaned up the project and fixed the test cases in your branch (Sgkhour/TPPDF:master).
I opened up a Pull Request so you can merge my changes into your working branch.
Please take a look that I didn't break anything, I had to change some code and also removed one or the other change (e.g. you added additional margin for header cells).
If you branch works fine afterwards, just push on your working branch, and the Pull Request #264 should update automatically. I will then give it another review and if it is working fine, we are merging into develop and preparing another release :)

Thanks for your contributions!

[Sgkhour]

thank you @philprime ... i merged your latest changes with my fork and all looks good. replaced this version in my app and all my outputs look good . so nothing broken.
all the iOS tests passed but a couple of failed macOS tests in
techprimate/TPPDF/Tests/TPPDFTests/List/PDFListItemObject_Spec.swift
oddly enough, they fail with a similar offset (15) as the previous failed test that you corrected. i will try to give it a shot and see what's the issue but having issues with quick/nimble.

[philprime]

interesting that they fail on macOS but not on iOS.

The "magic" 15 offset is most likely the header space, which should only appear if at least one element is in at least one header container. I'll test your branch later and see what's going on.

I just reviewed the Pull Request and realized you merged gh-pages into your master page.
That branch is only used for automated documentation, so please remove the docs folder

[philprime]

@Sgkhour I checked out your branch and tested it on macOS - it seems that all tests passed there too.

Anyway I merged your changes and will release an update now :) Thank you so much for your contributions!