Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FPDF.table() now raises an error when a single row is too high to be rendered on a single page #1143

Merged
merged 1 commit into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
4 changes: 1 addition & 3 deletions fpdf/fpdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -3401,9 +3401,7 @@ def will_page_break(self, height):
Returns: a boolean indicating if a page break would occur
"""
return (
# ensure that there is already some content on the page:
self.y > self.t_margin
and self.y + height > self.page_break_trigger
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/image/full_pdf_width_image.pdf
Binary file not shown.
Binary file modified test/image/svg_image_no_dimensions.pdf
Binary file not shown.
2 changes: 1 addition & 1 deletion test/image/test_full_page_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_full_pdf_width_image(tmp_path): # issue-528
pdf = FPDF()
pdf.set_margin(0)
pdf.add_page()
pdf.image(HERE / "png_images/51a4d21670dc8dfa8ffc9e54afd62f5f.png", w=pdf.epw)
pdf.image(HERE / "png_images/ba2b2b6e72ca0e4683bb640e2d5572f8.png", w=pdf.epw)
assert_pdf_equal(pdf, HERE / "full_pdf_width_image.pdf", tmp_path)


Expand Down
2 changes: 1 addition & 1 deletion test/image/test_vector_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_svg_image_fixed_dimensions(tmp_path):


def test_svg_image_no_dimensions(tmp_path):
pdf = fpdf.FPDF()
pdf = fpdf.FPDF(format=(350, 350))
pdf.add_page()
# This image has a 300x300 viewbox but no width/height:
pdf.image(SVG_SRCDIR / "SVG_logo_no_dimensions.svg")
Expand Down
6 changes: 3 additions & 3 deletions test/layout/test_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
HERE = Path(__file__).resolve().parent


def add_cell_page(pdf, cell_text):
def add_cell_page(pdf, cell_text, h=None):
"""Add a page with with some text to the PDF"""
pdf.set_font("Helvetica")
pdf.add_page()
pdf.cell(w=10, h=10, text=str(cell_text))
pdf.cell(w=10, h=h or 10, text=str(cell_text))


def test_unit_default(tmp_path):
Expand All @@ -33,7 +33,7 @@ def test_unit_str(tmp_path, unit):
def test_unit_int(tmp_path):
"""Test creating a PDF with an int unit"""
pdf = FPDF(unit=144)
add_cell_page(pdf, "int = 2 in")
add_cell_page(pdf, "int = 2 in", h=5)
assert_pdf_equal(pdf, HERE / "unit_int.pdf", tmp_path)


Expand Down
Binary file modified test/layout/unit_int.pdf
Binary file not shown.
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
Loading