flutter icon indicating copy to clipboard operation
flutter copied to clipboard

feat(table): add support for colSpan and rowSpan in TableCell

Open hm21 opened this issue 3 months ago • 11 comments

Description

This PR adds support for colSpan and rowSpan to the TableCell widget.
With this change, each TableCell can now span multiple columns and/or rows, similar to how HTML tables work.

This allows developers to create more flexible and complex table layouts without manually merging cells or resorting to nested tables.

Examples

Example 1 image
Table(
  border: TableBorder.all(color: Colors.grey),
  columnWidths: const <int, TableColumnWidth>{
    0: FixedColumnWidth(120),
    1: FlexColumnWidth(),
    2: FlexColumnWidth(),
  },
  children: <TableRow>[
    const TableRow(
      decoration: BoxDecoration(color: Color(0xFFE0E0E0)),
      children: <Widget>[
        TableCell(
          colSpan: 3,
          child: Padding(
            padding: EdgeInsets.all(8),
            child: Text(
              'Invoice Summary',
              style: TextStyle(fontSize: 18, fontWeight: FontWeight.bold),
              textAlign: TextAlign.center,
            ),
          ),
        ),
        TableCell.none,
        TableCell.none,
      ],
    ),
    const TableRow(
      decoration: BoxDecoration(color: Color(0xFFF5F5F5)),
      children: <Widget>[
        Padding(padding: EdgeInsets.all(8), child: Text('Customer')),
        TableCell(
          colSpan: 2,
          child: Padding(padding: EdgeInsets.all(8), child: Text('Alex Frei')),
        ),
        TableCell.none,
      ],
    ),
    const TableRow(
      children: <Widget>[
        Padding(padding: EdgeInsets.all(8), child: Text('Date')),
        Padding(padding: EdgeInsets.all(8), child: Text('16 Oct 2025')),
        Padding(padding: EdgeInsets.all(8), child: Text('Invoice #1024')),
      ],
    ),
    const TableRow(
      decoration: BoxDecoration(color: Color(0xFFE0E0E0)),
      children: <Widget>[
        TableCell(
          colSpan: 3,
          child: Padding(
            padding: EdgeInsets.all(8),
            child: Text('Items', style: TextStyle(fontWeight: FontWeight.bold)),
          ),
        ),
        TableCell.none,
        TableCell.none,
      ],
    ),
    const TableRow(
      children: <Widget>[
        Padding(padding: EdgeInsets.all(8), child: Text('Development Work')),
        Padding(padding: EdgeInsets.all(8), child: Text('10 hours')),
        Padding(padding: EdgeInsets.all(8), child: Text(r'$500')),
      ],
    ),
    const TableRow(
      children: <Widget>[
        Padding(padding: EdgeInsets.all(8), child: Text('Design Work')),
        Padding(padding: EdgeInsets.all(8), child: Text('5 hours')),
        Padding(padding: EdgeInsets.all(8), child: Text(r'$250')),
      ],
    ),
    TableRow(
      children: <Widget>[
        TableCell(
          rowSpan: 2,
          verticalAlignment: TableCellVerticalAlignment.fill,
          child: Container(
            color: const Color(0xFFF5F5F5),
            padding: const EdgeInsets.all(8),
            child: const Text('Notes'),
          ),
        ),
        const TableCell(
          colSpan: 2,
          child: Padding(
            padding: EdgeInsets.all(8),
            child: Text('Thank you for your business!'),
          ),
        ),
        TableCell.none,
      ],
    ),
    const TableRow(
      children: <Widget>[
        TableCell.none,
        TableCell(
          colSpan: 2,
          child: Padding(padding: EdgeInsets.all(8), child: Text('Payment due in 14 days.')),
        ),
        TableCell.none,
      ],
    ),
  ],
)
Example 2 image
Table(
  border: TableBorder.all(),
  children: <TableRow>[
    TableRow(
      children: <Widget>[
        TableCell(
          rowSpan: 3,
          verticalAlignment: TableCellVerticalAlignment.fill,
          child: Container(color: Colors.blue),
        ),
        TableCell(colSpan: 2, child: Container(color: Colors.pink, height: 50)),
        TableCell.none,
      ],
    ),
    TableRow(
      children: <Widget>[
        TableCell.none,
        TableCell(child: Container(color: Colors.green, height: 50)),
        TableCell(
          verticalAlignment: TableCellVerticalAlignment.bottom,
          rowSpan: 2,
          child: Container(color: Colors.orange, height: 75),
        ),
      ],
    ),
    TableRow(
      children: <Widget>[
        TableCell.none,
        TableCell(child: Container(color: Colors.purple, height: 50)),
        TableCell.none,
      ],
    ),
  ],
);

Issues Fixed

Fixes #21594

Questions for Reviewers

  1. This PR introduces the static constant TableCell.none to represent a skipped cell (for when another cell uses rowSpan or colSpan). I’m not sure if none is the best name, alternatives like placeholder or skipped might also fit. I avoided empty since DataCell.empty already exists with different semantics.
  2. Should this feature also be considered for DataTable? I initially didn’t include it cuz the Material Design table doesn’t normally support rowSpan or colSpan.

Pre-launch Checklist

hm21 avatar Oct 16 '25 15:10 hm21

I am not sure what the implications might be on performance here, have you see the TableView widget that supports cell merging with lazy loading for children?

Piinks avatar Oct 24 '25 22:10 Piinks

Hey @Piinks, thanks for the fast response to that PR, really appreciate it cuz I thought it would take way longer for someone to take a look, as my other older PR #176393 is still waiting, but maybe that one’s not very interesting for you guys😅🙈

...would you be willing to draft an outline of your changes and the motivations behind the decisions you made here?

Sure, I can fill it out but not sure I’ll have time this weekend, but definitely next week I should find a bit of time.

I am not sure what the implications might be on performance here, have you see the TableView widget that supports cell merging with lazy loading for children?

I'm not sure, but I guess you're referring to the TableView from the two_dimensional_scrollables package, right? If that's the case, then yes, I know that package. However, I still think there are a few reasons why it makes sense to implement it the way it's done in the PR.

Reasons for that PR
  1. Dependencies: two_dimensional_scrollables is a package on pub.dev and not directly part of Flutter. I think new Flutter devs will expect the Table widget to support that out of the box, without needing to search for an extra package. As seen in #21594, a few hundred people gave a thumbs up to that feature, so it seems like expected behavior should be internal. Also, when we compare it to other popular frameworks like MAUI, Ionic, or NativeScript, that kind of functionality is built-in without any extra dependencies.
  2. Complexity: I think for simple tables, most devs will expect a structure like Table > TableRow > TableCell, since that’s exactly how it works in HTML. In contrast, TableView works differently, the rowBuilder is separated from the cells, which could makes the widget tree harder to understand. Also, in TableViewCell, you have to set both columnMergeSpan and columnMergeStart. That makes sense when using the builder, but for a simple table, it feels a bit off or unnecessarily complicated. In most other frameworks, and even in standard HTML tables, you just set colSpan and rowSpan without needing to define a start position.
  3. Built-in RTL: I'm not 100% sure, but it seems that TableView.builder doesn't handle RTL correctly, or at least in my example, switching to RTL still keeps cell 0,0 on the top-left. Btw the existing Table widget already supports RTL, and the PR just extends that to also respect it for cell spanning.
  4. TableCellVerticalAlignment: Also, not 100% sure, but it seems like verticalAlignment isn’t supported in TableView. In this PR, verticalAlignment still works as expected and it also works correctly with colSpan and rowSpan for each individual cell.
  5. Performance: That's a good point you mentioned. For large tables with many offscreen cells that can be lazy-loaded, TableView.builder will definitely perform better. But for small tables where all cells are visible, I’m pretty sure it’s faster than TableView, though I doubt users would notice any difference. Comparing the original Table code to my changes, there’s a minimal impact on performance, but I don’t think that’s something end users would feel, especially since the Table widget isn’t designed for massive cell counts anyway. I tested it with 10k cells, and it was about 16ms slower, details are in the “Compare-Performance” section below. FYI: if performance is your top priority and readability is less important, I could also rewrite _computeSpanInformation to run directly within the same loop in performLayout. And also the loop inside table_border.dart could I merge.

Compare-Performance

To measure the average render duration, I repeatedly called setState() 100 times.

Test 1: Very Large (10,000 cells: 500 rows × 20 columns)

Version Avg (ms) Min (ms) Max (ms)
Original 351.09 275.40 469.32
PR 367.22 289.07 452.91

Test 2: Large (500 cells: 50 rows × 10 columns)

Version Avg (ms) Min (ms) Max (ms)
Original 42.18 20.29 96.12
PR 49.89 21.50 80.72

Btw, I hope you didn’t get me wrong TableView from two_dimensional_scrollables is definitely an awesome package, and I saw in the commits that you’re also working on it. Still, as I mentioned above, I believe there are valid reasons why it could make sense to support colSpan/rowSpan in the default Table as well. Anyway, if the points I mentioned aren’t relevant for you and the way to go for devs is to use packages for this kind of core functionality, just let me know, then we can close the PR 😉 But in that case, I’d also suggest closing #21594 and mentioning that the recommended solution is to use the two_dimensional_scrollables package.

Thanks again for the fast response to that PR and I'll fill out the document you mentioned as soon as possible.

hm21 avatar Oct 25 '25 11:10 hm21

as my other older PR https://github.com/flutter/flutter/pull/176393 is still waiting, but maybe that one’s not very interesting for you guys

Actually, we use labels to route PRs to the right sub-team for review. I noticed that PR did not have a team label on it, so I applied one and reached out to the team to confirm. I hope you will hear back soon on it, and thanks again for contributing! We have a Discord channel for reaching out to folks directly if a PR goes unattended, see here for docs:

  • https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#who
  • https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md

Thanks as well for your detailed response. If you are able to fill out the design dic template (the various sections are suggestive, not all required) it would be helpful for review. Otherwise I can start digging into this, it just might take a minute to get through it all. :)

Piinks avatar Oct 27 '25 21:10 Piinks

Hey @Piinks, thanks for sharing those two links and my bad for not reading carefully enough that I was supposed to mention the open PR in the Discord channel after two weeks🙈 Still, really appreciate your help with tagging the team on the other PR.

The link to the design-document is https://flutter.dev/go/table-cell-span.

Also, sorry again, I missed that the usual process is to first create an issue referencing the design doc before opening a PR. If you'd still prefer I create a new issue for the design doc but I just thought it might be a bit pointless now that the PR is already there😅

hm21 avatar Oct 28 '25 12:10 hm21

Also, FYI it looks like there are some failing tests here.

Piinks avatar Nov 17 '25 23:11 Piinks

Hey @Piinks, so sorry for my late response, I was on vacation in Iceland, and afterwards got a bit caught up with some personal work projects🙈

Couple tests I want to make sure we've covered.. From your doc (thanks again for providing!): "If a developer forgets to include the required TableCell.none entries, assertions during layout report the conflict and specify which row and column overlap occurred." Is that covered here? I might have missed it.

Currently it detects two different behaviors:

  1. If the user forgets to add a TableCell.none or any other widget in a row, it will assert here, indicating exactly which row has the incorrect number of children.
  2. If the number of children is correct but the colSpan or rowSpan exceeds the available columns or rows, it will assert here which throws an error with the exact row and column number where the problem occurred.

Also, what happens if the configuration of spans causes overlapping merged cells? How is that handled?

At the moment, there's no assert that throws an error in that case, so if it happens, the merged cells will overlap the other cells in reverse order (the last widget ends up on top of the others). I guess it would make sense to implement an assert for that too, right? Or how do you handle that case in TableView?


Also, FYI it looks like there are some failing tests here.

Oh, I thought it was cuz of the infrastructure issue with the Linux tests that were failing around the time I created that PR. In another PR, I had the same problem and was told here that it was related to that. But my bad for not double-checking also here more carefully.

~~Some tests might fail again now, since I’m pretty sure I didn’t implement the golden tests correctly but I’ll fix them ASAP ;)~~

EDIT: The Flutter Dashboard bot here mentions reviewing the golden tests. I opened the link and tried it (logged in with the same email as on GitHub), but got this error message: "Unexpected error triaging: 401 (Are you logged in with the right account?)"

So I’m guessing that part is only available to Flutter team members, or in this case, to “you,” right? Or is there something else I need to do first?

hm21 avatar Nov 28 '25 15:11 hm21

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #177102 at sha 0dfad75645b2306ae383de5407c06ac361cf4bfd

flutter-dashboard[bot] avatar Nov 28 '25 17:11 flutter-dashboard[bot]

Thanks for the updates! I hope you had a nice time in Iceland. Let me check how TableView handles overlapping merged cells. I do not recall, and there are probably some fundamental differences here since it lays out lazily.

Piinks avatar Dec 09 '25 21:12 Piinks

Thanks for the updates! I hope you had a nice time in Iceland. Let me check how TableView handles overlapping merged cells. I do not recall, and there are probably some fundamental differences here since it lays out lazily.

Looks like the TableView will not complain about overlapping merged cells. So maybe that's alright here too. :)

Piinks avatar Dec 09 '25 21:12 Piinks

The Flutter Dashboard bot https://github.com/flutter/flutter/pull/177102#issuecomment-3590004149 mentions reviewing the golden tests. I opened the link and tried it (logged in with the same email as on GitHub), but got this error message: "Unexpected error triaging: 401 (Are you logged in with the right account?)"

Yes, we have an allow list to manage write access to images. I can approve them, looking now.

Piinks avatar Dec 09 '25 21:12 Piinks

Delightful. 🙂 image

Piinks avatar Dec 09 '25 21:12 Piinks

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #177102 at sha ec0c44f632ebaac69dcc74e577e29e732470173c

flutter-dashboard[bot] avatar Dec 10 '25 10:12 flutter-dashboard[bot]

Hey @Piinks, thanks again for your review :)

Looks like the TableView will not complain about overlapping merged cells. So maybe that's alright here too. :)

Okay, that's good to know but if you still prefer me to add it here, just let me know :)

hm21 avatar Dec 10 '25 10:12 hm21