vandeseer / easytable

Small table drawing library built upon Apache PDFBox
MIT License
239 stars 91 forks source link

Repeat headers not displayed properly across multiple pages #111

Open grasshopper7 opened 3 years ago

grasshopper7 commented 3 years ago

Thanks for this artifact.

I kind of had an issue with displaying headers across pages when the space available for the table in the first page is less than the height of the headers. The first two rows are set to repeat.

  1. Below is the pic of a proper display. proper table ystart 200

  2. This is the case when only the repeat headers fit in the available space. Just the headers are drawn in the initial page without any rows of data. headers without row ystart 100

  3. This is the case when only the first header fits. The second header row is printed in the following page but the order is mixed up. split header ystart 70

  4. This is the case when none of the headers fit. Then the table is printed on the following page but it is pushed down by the height of the headers. no fit new page extra space ystart 50

Is there some setting that can be used to fix this? I was unable to find it so I modified the RepeatedHeaderTableDrawer to check if the height of the repeat headers and the first data row will fit in the initial page. If yes then no problem else set the startY value to top of new page. And a change in the draw method to create a new page when fit fails in the above case. If it makes sense will send in a PR.

To replicate this modify the startY variable in this line RepeatedHeaderTableDrawer tableDrawer = RepeatedHeaderTableDrawer.builder().table(tableBuilder.build()) .startX(50).startY(100F).endY(50F).numberOfRowsToRepeat(2).build();

Case 1 - 200F Case 2 - 100F Case 3 - 70 F Case 4 - 50F

public class PaginateTable {

    public static void main(String[] args) throws Exception {

        // Creating PDF document object
        PDDocument document = new PDDocument();
        PDPage my_page = new PDPage(PDRectangle.A4);
        document.addPage(my_page);

        PDPageContentStream content = new PDPageContentStream(document, my_page, APPEND, true);
        content.beginText();
        content.setFont(PDType1Font.TIMES_ROMAN, 20);
        content.newLineAtOffset(50, 500);
        content.showText("This is the sample document and we are adding content to it.");
        content.endText();
        content.close();

        TableBuilder tableBuilder = Table.builder().addColumnsOfWidth(100, 400).borderColor(Color.LIGHT_GRAY)
                .borderWidth(1).font(PDType1Font.TIMES_ROMAN).fontSize(13).verticalAlignment(VerticalAlignment.TOP)
                .horizontalAlignment(HorizontalAlignment.LEFT).padding(5)

                .addRow(Row.builder().add(TextCell.builder().text("SYSTEM").borderWidth(0).colSpan(2).build()).build())
                .addRow(Row.builder().add(TextCell.builder().text("Name").build())
                        .add(TextCell.builder().text("Value").build()).build());

        for (int i = 1; i < 36; i++) {
            Row row = Row.builder().add(TextCell.builder().text(String.valueOf(i)).build()).add(TextCell.builder().text(
                    "Hello there, EASYTABLE PDFBoX EASYTABLE PDFBoX Hello there, EASYTABLE PDFBoX EASYTABLE PDFBoX"
                            + System.getProperty("line.separator")
                            + "   Hello there, EASYTABLE PDFBoX EASYTABLE PDFBoX")
                    .lineSpacing(1).build()).build();
            tableBuilder.addRow(row);
        }

        RepeatedHeaderTableDrawer tableDrawer = RepeatedHeaderTableDrawer.builder().table(tableBuilder.build())
                .startX(50).startY(100F).endY(50F).numberOfRowsToRepeat(2).build();
        tableDrawer.draw(() -> document, () -> new PDPage(PDRectangle.A4), 50f);

        // Saving the document
        document.save("page_table.pdf");
        // Closing the document
        document.close();
    }
}
vandeseer commented 3 years ago

Hi @grasshopper7

Thanks for your detailed analysis. It definitely makes sense to open a PR. I would be more than happy to review it. Ideally with a test case such that we don't ever run in a regression later.

Best regards Stefan

grasshopper7 commented 3 years ago

Hi, Will create a pull request surely next week. Had gone down the rabbit hole dealing with pdfbox for the last 3 weeks.

Had a couple of other requests for which I will create new issues. First was for returning the start page of a table for creating a destination, required if the drawer pushes the table to a new page. Second to allow cells to contain an internal table, using the code u have already written for a Stack post - https://gist.github.com/vandeseer/bb86e12d01ca18b9306901ae2bf73cf9

Thanks.

vandeseer commented 3 years ago

Thanks for your PR!

I will release a new minor/bugfix version soon.

grasshopper7 commented 3 years ago

Will wait for the release. Thx.

Had implemented the use case of returning the PDPage instance for the table start, which I explained in an earlier comment above. It worked for RepeatedHeaderTableDrawer but when testing with a TableDrawer it is returning the initial (wrong) PDPage instance when the table is moved to a new page. This I will try and fix.

I also noticed that only a header without any data rows is displayed if space not available. Similar to case 2 that is described in the this issue for RepeatedHeader. Is this by design? Or at least space for header and one data row should be available?

grasshopper7 commented 3 years ago

Hi @vandeseer, Can you mention your thoughts on the second point in the comment above. - https://github.com/vandeseer/easytable/issues/111#issuecomment-766156908.

Sorry to be a bother. Thanks.

vandeseer commented 3 years ago

Hey @grasshopper7, so what you are saying is, that the library should rather check for the header rows and at least one data row, right?

From my point of view that would make a good candidate for a configuration flag, e.g. allowBlankHeaders(true) or maybe separateDataFromHeader(false) or sth. like this. I guess it shouldn't be too hard to implement this by subclassing the RepeatedHeaderTableDrawer either. I would prefer a configuration flag though.

grasshopper7 commented 3 years ago

Can this work if an additional method is introduced, addHeader() which makes it explicit that needs a minimum of one data row to follow for display?