[two_dimensional_scrollables] Fix pinned cells hit test by beroso · Pull Request #11777 · flutter/packages

beroso · 2026-05-25T13:50:39Z

This fixes a bug where TableView trailing pinned cells lose hit tests to underlying cells.
That glitch makes one of the main use cases unworkable (action columns at the right edge of the data grid).

Fix flutter/flutter#186876

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

google-cla · 2026-05-25T13:50:49Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request modifies RenderTableViewport to ensure hit testing correctly accounts for pinned areas by intersecting cell rectangles with section-specific clip rectangles. It introduces a _sectionClipRectFor method to calculate these bounds and adds regression tests to verify hit-test precedence for pinned cells. Review feedback suggests optimizing hit testing by using dual contains checks to avoid unnecessary object allocations and caching viewportDimension in _sectionClipRectFor to improve readability and performance.

gemini-code-assist · 2026-05-25T13:52:52Z

+      final Rect clippedCellRect = cellRect.intersect(
+        _sectionClipRectFor(cellParentData.tableVicinity),
+      );
+      if (clippedCellRect.contains(position)) {


To avoid unnecessary object allocation during hit testing, you can replace the intersect call with a combined contains check. Rect.intersect creates a new Rect object, which is not needed here since you only need to verify if the position is within both rectangles. This is a common optimization in hit testing paths to reduce pressure on the garbage collector.

if (cellRect.contains(position) && _sectionClipRectFor(cellParentData.tableVicinity).contains(position)) {

gemini-code-assist · 2026-05-25T13:52:52Z

+  // a cell belongs to (leading-pinned, non-pinned, or trailing-pinned for each
+  // axis). Intersecting the raw cell rect with this rect in hitTestChildren
+  // prevents scrollable cells from capturing taps inside pinned areas.
+  Rect _sectionClipRectFor(TableVicinity vicinity) {


In _sectionClipRectFor, viewportDimension is accessed many times across different branches. Consider caching it in a local variable (e.g., final Size viewportSize = viewportDimension;) at the start of the method to improve readability and avoid redundant property accesses.

Piinks

Hey @beroso thanks so much for the fix!
It looks like there may be an edge case this does not account for. The implementation of _sectionClipRectFor calculates clip boundaries without factoring in the viewport alignment offsets (_hAlignmentOffset and _vAlignmentOffset).

When the table is smaller than the viewport and configured with a custom alignment (e.g., alignment: Alignment.center), cells and clip rects are shifted by the alignment offsets. For hit testing, cells are shifted (via their paintOffset), but _sectionClipRectFor returns an un-shifted clip rect. This means hit testing on pinned cells in aligned tables fails, as the tap position falls outside of the un-shifted clip rect.

I wrote a regression test to include and verify:

Details

Add the new regression test verifying hit-testing on aligned pinned columns:

    testWidgets('Tapping on a pinned column in an aligned table (with _hAlignmentOffset) registers correctly', (
      WidgetTester tester,
    ) async {
      TableVicinity? lastTapped;

      await tester.pumpWidget(
        MaterialApp(
          home: Scaffold(
            body: SizedBox(
              height: 400,
              width: 400,
              child: TableView.builder(
                cacheExtent: 0.0,
                columnCount: 3,
                rowCount: 1,
                pinnedColumnCount: 1,
                alignment: Alignment.center,
                columnBuilder: (_) => const TableSpan(extent: FixedTableSpanExtent(100)),
                rowBuilder: (_) => const TableSpan(extent: FixedTableSpanExtent(100)),
                cellBuilder: (_, TableVicinity vicinity) {
                  return TableViewCell(
                    child: GestureDetector(
                      behavior: HitTestBehavior.opaque,
                      onTap: () => lastTapped = vicinity,
                      child: const SizedBox.expand(),
                    ),
                  );
                },
              ),
            ),
          ),
        ),
      );

      // Pinned column 0 is at x = 50..150 due to Alignment.center. Tap at (100, 200).
      await tester.tapAt(const Offset(100, 200));
      await tester.pumpAndSettle();

      expect(
        lastTapped,
        TableVicinity.zero,
        reason: 'Tapping at x=100 (inside aligned pinned column 0) should register a tap on column 0.',
      );
    });

beroso · 2026-06-04T20:03:42Z

Hi @Piinks thanks for the feedback!
Landed a fix about the viewport alignment offsets and add some regression tests, including yours.

Piinks

Thank you for the updates, I think we just need a bit more test coverage and this is good to go! 🚀

Piinks · 2026-06-23T20:44:11Z

+## 0.5.3

 * Updates minimum supported SDK version to Flutter 3.38/Dart 3.10.
+* Fixes an issue where pinned cells were losing hit tests to underlying non-pinned cells.


Suggested change

* Fixes an issue where pinned cells were losing hit tests to underlying non-pinned cells.

* Fixes an issue where trailing pinned cells in TableView were losing hit tests to underlying non-pinned cells.

Piinks · 2026-06-23T20:48:34Z

+                rowCount: 1,
+                pinnedColumnCount: 1,
+                alignment: Alignment.center,
+                horizontalDetails: const ScrollableDetails.horizontal(reverse: true),


There should be test coverage for when the vertical axis is reversed, and when both axes are reversed in addition to the horizontal reverse case here.

Piinks · 2026-06-23T20:51:53Z

I might also consider test coverage for when the TextDirection is RTL. :)

justinmc · 2026-07-14T21:26:04Z

@beroso Are you still planning to land this PR? Looks like it's close.

Piinks · 2026-08-25T21:22:50Z

Thank you for your contribution. I'm going to close this PR for now since there are outstanding comments, just to get this off our PR review queue. Please don't hesitate to re-open or submit a new PR if you have the time to address the review comments. Thanks!

@beroso

This picks up #11777 by @beroso, which was closed while review comments were still open. It keeps the original commits and adds one on top. `TableView` hit-tested each cell against its full rect, so a regular cell scrolled under a trailing pinned row or column could take taps meant for the pinned cell, like a menu button in a pinned last column. The change from #11777 limits each cell's hit test to the viewport band of its own section (leading pinned, regular or trailing pinned) and accounts for the alignment offsets. My commit adds the tests asked for in review: reversed vertical axis, both axes reversed, a vertically reversed table with alignment, and RTL with directional alignment. It also disposes the scroll controllers in the hit test tests, which failed leak tracking once it was enabled for this package in #11653. The CHANGELOG entry uses the suggested wording. Without the change in `table.dart`, six of the twelve hit test tests fail, including the two new reversed-axis ones. The alignment and RTL tests pass either way, since they cover the alignment offset case raised in review. On Flutter 3.47.6 stable, `flutter test` passes for the package (182 tests) and the hit test group also passes with `--platform chrome`. `flutter analyze` and the repo tool's `validate` report no issues. Fixes flutter/flutter#186876 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

github-actions Bot added p: two_dimensional_scrollables Issues pertaining to the two_dimensional_scrollables package triage-framework Should be looked at in framework triage labels May 25, 2026

gemini-code-assist Bot reviewed May 25, 2026

View reviewed changes

beroso added 2 commits May 25, 2026 10:59

Fix hit test of pinned cells

90f14e4

Add tests for pinned cells hit test.

782e898

beroso force-pushed the fix/186876-pinned-cells-hit-test branch from f3a106c to 782e898 Compare May 25, 2026 14:02

beroso added 2 commits May 25, 2026 11:32

Apply code review suggestions

c0f53f0

Format

ee38453

beroso force-pushed the fix/186876-pinned-cells-hit-test branch from b77e6e5 to ee38453 Compare May 25, 2026 14:32

Add changelog entry

b5e2143

stuartmorgan-g requested a review from Piinks May 26, 2026 18:28

beroso added 2 commits June 3, 2026 17:50

Merge branch 'flutter:main' into fix/186876-pinned-cells-hit-test

86c3865

Format code using updated analysis_options

125c780

Piinks added the CICD Run CI/CD label Jun 4, 2026

Piinks reviewed Jun 4, 2026

View reviewed changes

Consider alignment offsets and add regression tests

79c1b8c

github-actions Bot removed the CICD Run CI/CD label Jun 4, 2026

beroso requested a review from Piinks June 17, 2026 18:10

Piinks reviewed Jun 23, 2026

View reviewed changes

Piinks closed this Aug 25, 2026

Piinks mentioned this pull request Aug 25, 2026

[two_dimensional_scrollables] TableView trailing pinned columns lose hit tests to underlying cells flutter/flutter#186876

Closed

nurullahturkoglu mentioned this pull request Oct 22, 2026

[two_dimensional_scrollables] Fix pinned cells hit test #13376

Merged

11 tasks done

	* Fixes an issue where pinned cells were losing hit tests to underlying non-pinned cells.
	* Fixes an issue where trailing pinned cells in TableView were losing hit tests to underlying non-pinned cells.

Conversation

beroso commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

google-cla Bot commented May 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

beroso May 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

beroso May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

beroso commented Jun 4, 2026

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks commented Jun 23, 2026

Uh oh!

justinmc commented Jul 14, 2026

Uh oh!

Piinks commented Aug 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

beroso commented May 25, 2026 •

edited

Loading