fpdf2 icon indicating copy to clipboard operation
fpdf2 copied to clipboard

Increase flexibility of TableBordersLayout

Open TedBrookings opened this issue 1 year ago • 7 comments

Add capability to precisely control thickness, color, and dash of table cell borders.

TableBordersLayout is changed from an enum to a more complex class with helper style classes that allow more complex custom style setting. The old enum is maintained as static members of the new class, so the previous invocation style works the same, with the added option to specify cell border styles by passing a custom function. Fixes #957

Checklist:

  • [ ] The GitHub pipeline is OK (green), meaning that both pylint (static code analyzer) and black (code formatter) are happy with the changes of this PR.

  • [x] A unit test is covering the code added / modified by this PR

  • [ ] This PR is ready to be merged

  • [x] In case of a new feature, docstrings have been added, with also some documentation in the docs/ folder

  • [x] A mention of the change is present in CHANGELOG.md

I'm making a draft PR. There are many unit tests that fail. The reason is partly down to strategy, and rather than plowing ahead and changing all the reference PDFs for the tests, I wanted to first get agreement that the overall approach I've used is good. The tests fail for one of two reasons

  1. The reference implementation draws two identical lines where mine draws one. This is easily fixable by just double-drawing the lines, and I've started doing that, but wanted agreement on this decision.
  2. More critically, since the new class manages color, thickness, and dash qualities, I didn't want to managing setting and restoring all of these, so instead I wrap the drawing commands inside a local context (q [set thickness/color/dash] [draw element] Q). The result is visually identical and generally more concise, but it (correctly) fails assert_pdf_equal. My preferred solution would be to just update the test PDFs, but if this is not desired, it would be possible (if tedious) to adopt a set and reset approach that should yield identical outputs.

It also occurred to me that you might not like the class structure I created, so again I'd rather fix that and deal with the implications now, rather than plow ahead.

By submitting this pull request, I confirm that my contribution is made under the terms of the GNU LGPL 3.0 license.

TedBrookings avatar Nov 06 '23 15:11 TedBrookings