Skip to content

feat: Table cell spans #1736

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

feat: Table cell spans #1736

wants to merge 12 commits into from

Conversation

Gustl22
Copy link

@Gustl22 Gustl22 commented Sep 8, 2024

Add's the col and rowspan feature for tables.

Closes #989, #983, #896, #645, #313, #256, #167

@Gustl22 Gustl22 marked this pull request as ready for review September 8, 2024 15:01
@Gustl22
Copy link
Author

Gustl22 commented Sep 8, 2024

IntrinsicColumnWidth is not yet tested. Also I didn't get the whole concept, so there might be some edge cases, which I haven't considered (especially in the paint method, where _widths are consumed).

@DavBfr
Copy link
Owner

DavBfr commented Sep 9, 2024

Can you add a test?

@Gustl22
Copy link
Author

Gustl22 commented Sep 9, 2024

Sure, I just have limited resources and vacation soon, so if someone wants to finish up, I don't mind.
I'll get back to it in a few weeks.

@Gustl22 Gustl22 changed the title feat: Column spans (closes #313) feat: Column spans Sep 9, 2024
@Gustl22
Copy link
Author

Gustl22 commented Sep 14, 2024

I added a test, but it does not precisely check e.g. the different box properties.
If this should be also done, let me know and how to best approach it.

@Gustl22
Copy link
Author

Gustl22 commented Sep 19, 2024

I have a small problem: the tests now complain, that there's a difference (which is indeed in the renders).
But if I inspect this in a pdf viewer / firefox / chrome etc. There isn't any difference visisble:

diff-3
image

I suspect the test renderer interprets this as small gap and therefore outputs the mean as grey?
widgets-table.pdf

Should we ignore this or solve it another way?

@DavBfr
Copy link
Owner

DavBfr commented Sep 19, 2024

Yes, that's possible. Seems to be at the intersection of each cell.

@Gustl22
Copy link
Author

Gustl22 commented Sep 19, 2024

To allow col spans the border must be calculated for each cell and not for the whole width / height of the table any more. So there now exist infinite small fake gaps, which in theory should not be visible, and also aren't in my inspection in the pdf readers, just the pdftocairo does this I think...

@DavBfr
Copy link
Owner

DavBfr commented Sep 19, 2024

That's fine then.

@Gustl22
Copy link
Author

Gustl22 commented Sep 20, 2024

I now wonder, if it would make sense to introduce a new widget, like TableCell where one can define the col and row span. Otherwise I don't see a good way to support row spans in the future. Also this would remove potential errors in defining the col spans on the Row level.

See also:
flutter/packages#5917
https://www.w3.org/TR/css-tables-3/#height-distribution

@Gustl22 Gustl22 changed the title feat: Column spans feat: Table cell spans Sep 30, 2024
@Gustl22
Copy link
Author

Gustl22 commented Sep 30, 2024

Row span is now also supported via TableCell:

image

I updated the goldens and saved the diffs here:
diff.zip

@Gustl22
Copy link
Author

Gustl22 commented Oct 5, 2024

I now used an improved algorithm for rowspanning. There's a special case where the remaining space it not quite optimized, when multiple cells with rowpans overlap and in one row and no cell (without rowspan) with height is given to that row.
This is the case, as there's no way to know beforehand how much space a second cell with rowspan would occupy in the current row, as it could use its space on the next row also. So we would need a third round of calculation who minimizes these gaps, or an optimization function in general.
I found this quite hard to solve, but it occurs in only a very small amount of cases (I think). So I would leave it like that for now and if this needs improving, do it afterwards (?).

@Gustl22 Gustl22 mentioned this pull request Nov 14, 2024
@sabin26
Copy link
Contributor

sabin26 commented Nov 25, 2024

Hello @Gustl22, I tried your changes, I think this is a bug.

Code

pw.Widget buildSpanCell({
      final int columnSpan = 1,
      final int rowSpan = 1,
      final PdfColor? color,
      final double? height,
    }) {
      return pw.TableCell(
        child: pw.Container(
          color: color,
          height: height,
          child: pw.Text('colSpan: $columnSpan, rowSpan: $rowSpan, height: $height'),
        ),
        columnSpan: columnSpan,
        rowSpan: rowSpan,
      );
    }

pdf.addPage(pw.Page(
      build: (final pw.Context context) => pw.Table(
        children: [
          pw.TableRow(
            children: [
              buildSpanCell(rowSpan: 2, color: PdfColors.red),
              buildSpanCell(rowSpan: 2, color: PdfColors.blue),
              buildSpanCell(color: PdfColors.green),
            ],
          ),
          pw.TableRow(
            children: [
              buildSpanCell(color: PdfColors.green),
            ],
          ),
        ],
        border: pw.TableBorder.all(),
      ),
    ));

Result

table_span_pdf

Expectation

The first and second column should have their rows spanned

Actual

The first and third column have their rows spanned instead of the second column (notice the border in the second column)

@fchamone
Copy link

Hi @Gustl22 , I'm hitting the same bug as @sabin26 . From my tests I could see that when we have two consecutive rowspanned cells, it bugs. Here are some images:

1 - Perfect table with first row colspanned (4):
image

2 - Perfect table with first row colspanned (4) and second row colspanned(2) in two columns:
image

3 - Perfect table now with three different rowspans. As they are not consecutive, the table is fine (cells 2 & 2a merged, cells 3a & 3b merged and cells 4b and 4c merged)
image

4 - Now comes the problem. When trying to merge cells 2 & 2a and also 3 & 3a, the bug happens. It merges correctly the 2 & 2A, but when merging the 3 & 3a, the 4a cell gets pulled into the 3a position (that should be rowspanned inside the 3 cell)
image

@Gustl22
Copy link
Author

Gustl22 commented Feb 25, 2025

Current performance decrease from about 30% for tables, but considering the fact, that every cell is drawn by its own instead of whole rows and columns.

image

to

image

@Gustl22

This comment has been minimized.

@Gustl22
Copy link
Author

Gustl22 commented Feb 26, 2025

@sabin26 @fchamone I fixed the issue.

@DavBfr can you have a look? If the performance decrease is to high, we can think about switching the calculation mode, if no TableCell is present.

More: are you still interested in this feature?
Thank you for providing and maintaining this package, its a lot of work!

@matteo-convert
Copy link

matteo-convert commented Mar 7, 2025

Any update on this please ? it will be very usefull to have this feature

@Gustl22
Copy link
Author

Gustl22 commented Mar 7, 2025

You can always checkout this branch as git dependency, I'll try to keep it up to date. And also an thumbs up in the PR description may would help to get this in.

@matteo-convert
Copy link

matteo-convert commented Mar 7, 2025

You can always checkout this branch as git dependency, I'll try to keep it up to date. And also an thumbs up in the PR description may would help to get this in.

can you explain how i can achieve this in flutter, i'm new idk all the stuff ?

Edited :
Got it for who search

dependency_overrides:

  pdf:
    git:
      url: https://github.com/Gustl22/dart_pdf.git
      path: pdf/
      ref: 313-column-span

@matteo-convert
Copy link

@DavBfr Hello,
Can we got a progress on this pull request please ?

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.

Please show interest in achieving row span and colspan like html in pdf table
5 participants