py-pdf / fpdf2

Simple PDF generation for Python
https://py-pdf.github.io/fpdf2/
GNU Lesser General Public License v3.0
1.11k stars 249 forks source link

Feature request: allow horizontal centering in TextStyle #1282

Open Lucas-C opened 2 weeks ago

Lucas-C commented 2 weeks ago

Proposed solution

  1. Allow Align.C to be passed as a value for .l_margin in TextStyle
  2. When this .l_margin is used in FPDF._use_title_style(), if it is Align.C, then horizontally align the text

The PR implementing this should include unit tests using FPDF.start_section()

Discussed in https://github.com/py-pdf/fpdf2/discussions/1280

Originally posted by **Benoite142** October 10, 2024: > Hi, > I've been trying to center a TitleStyle for quite some time now. I've been using a TitleStyle for the `start_section` function used in the TOC placeholder and I am having a lot of trouble trying to center the TitleStyle. I know we can easily center some cell or text with `align='C'` but I found no way of doing that for the TitleStyle. I know we can put a value to the `l_margin` to make it look like it is centered, but I can't really use that since all of my titles differs in length. > I also tried aligning everything to the center of the page, but that still doesn't work with TitleStyle and the sections. Worst case scenario is that I put everything on the left, but I would really want it to be centered. > If anyone knows how I could do this, please let me know. > Thank you for you time 😊
Lucas-C commented 2 weeks ago

@allcontributors please add @Benoite142 for ideas

allcontributors[bot] commented 2 weeks ago

@Lucas-C

I've put up a pull request to add @Benoite142! :tada:

visheshdvivedi commented 15 hours ago

Hi @Lucas-C ,

I am new to this project and would like to contribute for this feature request.

Based on my understanding, here are the required changes

fpdf/fonts.py (allow to accept Align values in l_margin

class TextStyle(FontFace):
"""
Subclass of `FontFace` that allows to specify vertical & horizontal spacing
"""
def __init__(
    self,
    font_family: Optional[str] = None,  # None means "no override"
    #                                     Whereas "" means "no emphasis"
    font_style: Optional[str] = None,
    font_size_pt: Optional[int] = None,
    color: Union[int, tuple] = None,  # grey scale or (red, green, blue),
    fill_color: Union[int, tuple] = None,  # grey scale or (red, green, blue),
    underline: bool = False,
    t_margin: Optional[int] = None, 
    l_margin: Optional[int] | Optional[Align] = None, # allow to pass Align values in l_margin
    b_margin: Optional[int] = None,
    ):

fonts/fpdf.py

@contextmanager
    def _use_title_style(self, title_style: TextStyle):
        if title_style:
            if title_style.t_margin:
                self.ln(title_style.t_margin)
            if title_style.l_margin:
                if title_style.l_margin == Align.C:
                    self.set_x(self.w/2) # write code to center the title_style horizontally
                    pass
                self.set_x(title_style.l_margin)
        with self.use_font_face(title_style):
            yield
        if title_style and title_style.b_margin:
            self.ln(title_style.b_margin)

Please suggest any other changes required or not (I am not counting the test cases here, just the changes required to introduce the feature)

Lucas-C commented 11 hours ago

Hi @visheshdvivedi

Yes, this looks like a good start! You are very welcome to initiate a Pull Request, and create a dedicated unit test πŸ™‚

You will also find more information about fpdf2 development process on this page: https://py-pdf.github.io/fpdf2/Development.html

There is one thing I would suggest: enums.Align inherits from enums.CoerciveEnum, meaning that the string CENTER or C can be converted to Align.C by simply calling Align.coerce(string_value). For this reason:

  1. TextStyle constructor should also accept string as values for l_margin
  2. Inside TextStyle constructor, self.l_margin = Align.coerce(l_margin) should be called
  3. We should also consider if we support other Align values, and if not, raise an error if invalid Align values are provided for l_margin
visheshdvivedi commented 2 hours ago

Hi @Lucas-C ,

When writing test, something like this

from fpdf import FPDF, Align, TitleStyle

def test_toc_horizontal_alignment(tmp_path):  # issue-1282
    title_styles = [
        "small title style",
        "intentionally large title style",
    ]

    pdf = FPDF()
    pdf.set_font(family="helvetica", size=12)
    pdf.add_page()
    for title in title_styles:
        level0 = TitleStyle("Helvetica", "", 20, (0, 0, 0), l_margin=Align.C)
        pdf.set_section_title_styles(level0)
        pdf.start_section(title)
    pdf.output("test.pdf")

test_toc_horizontal_alignment(tmp_path="")

When I passed the value of Align.C, it gave an error like so

  File "/<path>/fpdf2/fpdf/fpdf.py", line 3818, in multi_cell
    maximum_allowed_width = w = w - padding.right - padding.left
                                ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
TypeError: unsupported operand type(s) for -: 'float' and 'Align'

I looked into it and realized that the start_section has this code here.

page_break_triggered = self.multi_cell(
    w=self.epw,
    h=self.font_size,
    text=name,
    new_x=XPos.LMARGIN,
    new_y=YPos.NEXT,
    dry_run=True,  # => does not produce any output
    output=MethodReturnValue.PAGE_BREAK,
    padding=Padding(
        top=title_style.t_margin or 0,
        left=title_style.l_margin or 0,
        bottom=title_style.b_margin or 0,
    ),
)

to check if any page break will be triggered and this takes in l_margin as input. As a result the code breaks (since no code is present here to allow Align or str values in l_margin.

Should I also update the Padding class and multi_cell function to allow Align and str values ?