Skip to content

Commit

Permalink
FPDF.table() now raises an error when a single row is too high to be …
Browse files Browse the repository at this point in the history
…rendered on a single page
  • Loading branch information
Lucas-C committed May 27, 2024
1 parent 10e529d commit 9f0445f
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ This can also be enabled programmatically with `warnings.simplefilter('default',
* [`fpdf.drawing.DeviceCMYK`](https://py-pdf.github.io/fpdf2/fpdf/drawing.html#fpdf.drawing.DeviceCMYK) objects can now be passed to [`FPDF.set_draw_color()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.set_draw_color), [`FPDF.set_fill_color()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.set_fill_color) and [`FPDF.set_text_color()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.set_text_color) without raising a `ValueError`: [documentation](https://py-pdf.github.io/fpdf2/Text.html#text-formatting).

### Changed
* [`FPDF.table()`](https://py-pdf.github.io/fpdf2/Tables.html) now raises an error when a single row is too high to be rendered on a single page

### Deprecated

Expand Down
2 changes: 1 addition & 1 deletion fpdf/fpdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -3402,7 +3402,7 @@ def will_page_break(self, height):
"""
return (
# ensure that there is already some content on the page:
self.y > self.t_margin
self.y >= self.t_margin
and self.y + height > self.page_break_trigger
and not self.in_footer
and self.accept_page_break
Expand Down
29 changes: 20 additions & 9 deletions fpdf/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def render(self):
)

# Defining table global horizontal position:
prev_l_margin = self._fpdf.l_margin
prev_x, prev_y, prev_l_margin = self._fpdf.x, self._fpdf.y, self._fpdf.l_margin
if table_align == Align.C:
self._fpdf.l_margin = (self._fpdf.w - self._width) / 2
self._fpdf.x = self._fpdf.l_margin
Expand Down Expand Up @@ -220,10 +220,20 @@ def render(self):
)
self._fpdf.y += self._outer_border_margin[1]
for i, row in enumerate(self.rows):
pagebreak_height = row_info[i].pagebreak_height
# pylint: disable=protected-access
page_break = self._fpdf._perform_page_break_if_need_be(
row_info[i].pagebreak_height
)
page_break = self._fpdf._perform_page_break_if_need_be(pagebreak_height)
if (
page_break
and self._fpdf.y + pagebreak_height > self._fpdf.page_break_trigger
):
# Restoring original position on page:
self._fpdf.x = prev_x
self._fpdf.y = prev_y
self._fpdf.l_margin = prev_l_margin
raise ValueError(
f"The row with index {i} is too high and cannot be rendered on a single page"
)
if page_break and repeat_headings and i >= self._num_heading_rows:
# repeat headings on top:
self._fpdf.y += self._outer_border_margin[1]
Expand Down Expand Up @@ -372,9 +382,8 @@ def _render_table_cell(

# place cursor (required for images after images)

if (
height_query_only
): # not rendering, cell_x_positions is not relevant (and probably not provided)
# not rendering, cell_x_positions is not relevant (and probably not provided):
if height_query_only:
cell_x = 0
else:
cell_x = cell_x_positions[j]
Expand Down Expand Up @@ -483,7 +492,7 @@ def _render_table_cell(
dy = 0

if cell_height is not None:
actual_text_height = cell_height_info.rendered_height[j]
actual_text_height = cell_height_info.rendered_heights[j]

if v_align == VAlign.M:
dy = (cell_height - actual_text_height) / 2
Expand Down Expand Up @@ -848,8 +857,10 @@ def write(self, text, align=None):
@dataclass(frozen=True)
class RowLayoutInfo:
height: float
# accumulated rowspans to take in account when considering page breaks:
pagebreak_height: float
rendered_height: dict
# heights of every cell in the row:
rendered_heights: dict
merged_heights: list


Expand Down
Binary file modified test/table/table_with_rowspan_and_pgbreak.pdf
Binary file not shown.
27 changes: 27 additions & 0 deletions test/table/test_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -802,3 +802,30 @@ def test_table_cell_fill_mode(tmp_path):
pass
pdf.ln()
assert_pdf_equal(pdf, HERE / "table_cell_fill_mode.pdf", tmp_path)


def test_table_with_very_long_text():
pdf = FPDF()
pdf.add_page()
pdf.set_font("Helvetica")
with pytest.raises(ValueError) as error:
with pdf.table() as table:
row = table.row()
row.cell(LOREM_IPSUM)
# Adding columns to have the content of the 1st cell to overflow:
row.cell("")
row.cell("")
assert (
str(error.value)
== "The row with index 0 is too high and cannot be rendered on a single page"
)
with pytest.raises(ValueError) as error:
with pdf.table() as table:
row = table.row()
row.cell("")
row.cell("")
row.cell(LOREM_IPSUM)
assert (
str(error.value)
== "The row with index 0 is too high and cannot be rendered on a single page"
)
5 changes: 2 additions & 3 deletions test/table/test_table_rowspan.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,6 @@ def test_table_with_rowspan_and_pgbreak(tmp_path):
]

pdf.add_page()
y0 = pdf.h - pdf.b_margin
with pdf.local_context(**line_opts):
pdf.line(0, y0, pdf.w, y0)

# simple table
# with pdf.table(TABLE_DATA, **table_opts):
Expand All @@ -181,7 +178,9 @@ def test_table_with_rowspan_and_pgbreak(tmp_path):
# -- verify break occurs before the offending rowspan
# -- verify header reproduction
pdf.set_y(pdf.h - 85)
y0 = pdf.h - pdf.b_margin
with pdf.local_context(**line_opts):
pdf.line(0, y0, pdf.w, y0)
pdf.line(0, pdf.y, pdf.w, pdf.y)
with pdf.table(TABLE_DATA, **table_opts):
pass
Expand Down

0 comments on commit 9f0445f

Please sign in to comment.