rkusa / pdfjs

A Portable Document Format (PDF) generation library targeting both the server- and client-side.
MIT License
774 stars 142 forks source link

Support adding additional table headers #293

Closed KindaOK closed 1 year ago

KindaOK commented 1 year ago

Resolves #272

Adds the option to create multiple table headers provided that no rows have been created yet.

Currently defers header rendering until the first row is placed. This does change existing behavior in that respect.

Passes all tests locally.

KindaOK commented 1 year ago

Fixed the issue. Also added some code to render the bottom border of a header along with unit tests.

rkusa commented 1 year ago

While preparing a release I've just noticed the following warnings in the simple-multiple-headers test:

# table/simple-multiple-headers
set cursor.y to negative value
set cursor.y to negative value
set cursor.y to negative value
set cursor.y to negative value
set cursor.y to negative value
set cursor.y to negative value
ok 116 simple-multiple-headers

Any idea what could cause that? If not, don't worry about it, I can also look into it

KindaOK commented 1 year ago

It has to do with the size of the headers and fitting them on a page. Some experimentation on the test is below.

Num Headers (i < x) Vertical Padding Warning Messages
3 10 None
3 20 6
3 30 9
3 40 None
4 20 3
8 10 None
9 10 9
10 10 None

Error is reached here. https://github.com/rkusa/pdfjs/blob/55a662f4bc40109d49ffee0b4bd3379bbf486006/lib/cell.js#L248

rkusa commented 1 year ago

I removed the warning altogether. It was also logged in other tests and there are cases where a negative y isn't an issue (as content is rendered to a "content chunk" that is later placed e.g. on the next page with another total vertical offset).