Skip to content

Conversation

@gmischler
Copy link

Fixes a lot of stuff

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.
  • A unit test is covering the code added / modified by this PR
  • This PR is ready to be merged
  • In case of a new feature, docstrings have been added, with also some documentation in the docs/ folder
  • A mention of the change is present in CHANGELOG.md

This is going to be a rather substantial refactor of the Table code. Be patient...
Based on the concepts outlined in #339 .

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

@gmischler
Copy link
Author

gmischler commented Jun 30, 2024

Necessary breaking changes:

Just a heads-up for @Lucas-C and @andersonhc and anyone else interested:
Converting table cells to text regions will make tables a lot more flexible and useful:

  • Freely format text within cells
  • Combine text and images in cells in arbitrary sequence
  • Allow to split cells for page breaks

But there's also stuff that needs to change in a backwards incompatible manner:

  • No "data driven" spans in the core library. Explicit colspan and rowspan arguments competing with "special content" in the actual cell data is bad design. It results in hard to predict behaviour and makes things unnecessarily complicated, because the code needs to rearrange the internal structure of the table in several iterations after the data has been collected. Besides, I thought there was consensus that any "data driven" formatting should be relegated to data adapters, and this functionality is trivial to implement that way.

  • Unfortunately, the original implementation declared line_height to be an absolute size in document units. With the possibility to arbitrarily change the font size within the text, this doesn't make any practical sense. Because of that, HTML defines line_height as a factor relative to the font height, and I've adapted the same strategy in the text regions. Tables should not be an exception here. This will most likely be the most annoying change for end users, but that can't be helped.

Maybe we should add notices to the documentation in advance, to give our users a chance to prepare for those changes.

@jbwexler
Copy link

This will solve an issue I'm having. Any idea when it will be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants