diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 0b1efec9e9e4..eae3dbfaa970 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,6 +1,7 @@ -## NEXT +## 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. ## 0.5.2 diff --git a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart index dab40443d8ea..c0e281b0cd4d 100644 --- a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart +++ b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart @@ -551,7 +551,11 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { continue; } final Rect cellRect = cellParentData.paintOffset! & cell.size; - if (cellRect.contains(position)) { + // Intersect with the section clip rect for this cell to prevent + // scrollable cells from stealing hits inside the trailing pinned area. + // This mirrors the pushClipRect applied to each section during painting. + if (cellRect.contains(position) && + _sectionClipRectFor(cellParentData.tableVicinity).contains(position)) { result.addWithPaintOffset( offset: cellParentData.paintOffset, position: position, @@ -577,6 +581,89 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { return false; } + // Returns the viewport-space clip rect that painting applies to the section + // 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. + // + // Alignment offsets must be factored in: + // * Leading-pinned cells are laid out with _hAlignmentOffset / + // _vAlignmentOffset applied, so their paint position (and section + // boundary) shifts by those values. + // * Trailing-pinned cells are always placed at the absolute viewport edge + // via -(viewportDimension - trailingExtent), independent of alignment. + // * Non-pinned boundaries that border the leading-pinned section inherit + // the same shift; boundaries that border the trailing-pinned section + // do not. + Rect _sectionClipRectFor(TableVicinity vicinity) { + final bool reversedH = axisDirectionIsReversed(horizontalAxisDirection); + final bool reversedV = axisDirectionIsReversed(verticalAxisDirection); + final Size viewportSize = viewportDimension; + + final double colLeft, colRight; + if (vicinity.column < delegate.pinnedColumnCount) { + // Leading pinned column. + if (reversedH) { + colLeft = viewportSize.width - _leadingPinnedColumnsExtent - _hAlignmentOffset; + colRight = viewportSize.width - _hAlignmentOffset; + } else { + colLeft = _hAlignmentOffset; + colRight = _hAlignmentOffset + _leadingPinnedColumnsExtent; + } + } else if (_firstTrailingPinnedColumn != null && + vicinity.column >= _firstTrailingPinnedColumn!) { + // Trailing pinned column. + if (reversedH) { + colLeft = 0.0; + colRight = _trailingPinnedColumnsExtent; + } else { + colLeft = viewportSize.width - _trailingPinnedColumnsExtent; + colRight = viewportSize.width; + } + } else { + // Non-pinned column. + if (reversedH) { + colLeft = _trailingPinnedColumnsExtent; + colRight = viewportSize.width - _leadingPinnedColumnsExtent - _hAlignmentOffset; + } else { + colLeft = _leadingPinnedColumnsExtent + _hAlignmentOffset; + colRight = viewportSize.width - _trailingPinnedColumnsExtent; + } + } + + final double rowTop, rowBottom; + if (vicinity.row < delegate.pinnedRowCount) { + // Leading pinned row. + if (reversedV) { + rowTop = viewportSize.height - _leadingPinnedRowsExtent - _vAlignmentOffset; + rowBottom = viewportSize.height - _vAlignmentOffset; + } else { + rowTop = _vAlignmentOffset; + rowBottom = _vAlignmentOffset + _leadingPinnedRowsExtent; + } + } else if (_firstTrailingPinnedRow != null && vicinity.row >= _firstTrailingPinnedRow!) { + // Trailing pinned row. + if (reversedV) { + rowTop = 0.0; + rowBottom = _trailingPinnedRowsExtent; + } else { + rowTop = viewportSize.height - _trailingPinnedRowsExtent; + rowBottom = viewportSize.height; + } + } else { + // Non-pinned row. + if (reversedV) { + rowTop = _trailingPinnedRowsExtent; + rowBottom = viewportSize.height - _leadingPinnedRowsExtent - _vAlignmentOffset; + } else { + rowTop = _leadingPinnedRowsExtent + _vAlignmentOffset; + rowBottom = viewportSize.height - _trailingPinnedRowsExtent; + } + } + + return Rect.fromLTRB(colLeft, rowTop, colRight, rowBottom); + } + // Updates the cached column metrics for the table. // // By default, existing column metrics will be updated if they have changed. diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index 45fb03ee642a..a90e6dd31bbb 100644 --- a/packages/two_dimensional_scrollables/pubspec.yaml +++ b/packages/two_dimensional_scrollables/pubspec.yaml @@ -1,6 +1,6 @@ name: two_dimensional_scrollables description: Widgets that scroll using the two dimensional scrolling foundation. -version: 0.5.2 +version: 0.5.3 repository: https://github.com/flutter/packages/tree/main/packages/two_dimensional_scrollables issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+two_dimensional_scrollables%22+ diff --git a/packages/two_dimensional_scrollables/test/table_view/table_test.dart b/packages/two_dimensional_scrollables/test/table_view/table_test.dart index 63b8189b6aaf..7fc79421482e 100644 --- a/packages/two_dimensional_scrollables/test/table_view/table_test.dart +++ b/packages/two_dimensional_scrollables/test/table_view/table_test.dart @@ -4018,6 +4018,487 @@ void main() { expect(mergedRect.top, 200); expect(mergedRect.bottom, 400); }); + + group('Table pinned cells hit test', () { + // Regression tests for https://github.com/flutter/flutter/issues/186876 + // Trailing pinned cells must take precedence over underlying scrollable cells + // that have scrolled into the same screen rect. + // + // Geometry shared by all four tests (10 columns × 10 rows, 100 × 100 px, + // viewport 400 × 400 px): + // + // paintOffset.dx(col k) = k × 100 − scroll (for non-pinned columns) + // + // At scroll = 525: + // • _targetTrailingColumnPixel = 825, so col 8 (trailingOffset = 900) is + // the last non-pinned column built (900 ≥ 825). + // • col 8 lands at x = 8×100 − 525 = 275 → paint rect x = 275..375. + // • Trailing pinned col 9 always sits at x = 300..400. + // • Both rects contain x = 350, so without the _sectionClipRectFor fix + // the forward child-list scan hits col 8 first and swallows the tap. + // + // At scroll = 500 the bug is NOT triggered: _targetTrailingColumnPixel = 800 + // causes col 7 (trailingOffset = 800) to be the last non-pinned column built + // (col 8 is never added to the child list), so there is no overlap. + // The same argument applies symmetrically for rows. + + testWidgets('Trailing pinned column takes precedence over scrolled-under regular cell', ( + WidgetTester tester, + ) async { + final horizontalController = ScrollController(); + TableVicinity? lastTapped; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 400, + child: TableView.builder( + cacheExtent: 0.0, + columnCount: 10, + rowCount: 1, + trailingPinnedColumnCount: 1, + horizontalDetails: ScrollableDetails.horizontal(controller: horizontalController), + 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(), + ), + ); + }, + ), + ), + ), + ), + ); + + // col 8 (regular) lands at x = 275..375, overlapping the trailing + // pinned col 9 (x = 300..400). A tap at x = 350 hits both; col 9 wins. + horizontalController.jumpTo(525); + await tester.pump(); + + await tester.tapAt(const Offset(350, 50)); + await tester.pumpAndSettle(); + + expect( + lastTapped?.column, + 9, + reason: + 'Trailing pinned column 9 must receive the tap; column 8 (scrolled underneath) must not.', + ); + }); + + testWidgets('Trailing pinned row takes precedence over scrolled-under regular cell', ( + WidgetTester tester, + ) async { + final verticalController = ScrollController(); + TableVicinity? lastTapped; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 400, + child: TableView.builder( + cacheExtent: 0.0, + columnCount: 1, + rowCount: 10, + trailingPinnedRowCount: 1, + verticalDetails: ScrollableDetails.vertical(controller: verticalController), + 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(), + ), + ); + }, + ), + ), + ), + ), + ); + + // row 8 (regular) lands at y = 275..375, overlapping the trailing + // pinned row 9 (y = 300..400). A tap at y = 350 hits both; row 9 wins. + verticalController.jumpTo(525); + await tester.pump(); + + await tester.tapAt(const Offset(50, 350)); + await tester.pumpAndSettle(); + + expect( + lastTapped?.row, + 9, + reason: 'Trailing pinned row 9 must receive the tap; row 8 (scrolled underneath) must not.', + ); + }); + + testWidgets('Trailing pinned corner cell takes precedence over scrolled-under regular cells', ( + WidgetTester tester, + ) async { + final horizontalController = ScrollController(); + final verticalController = ScrollController(); + TableVicinity? lastTapped; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 400, + child: TableView.builder( + cacheExtent: 0.0, + columnCount: 10, + rowCount: 10, + trailingPinnedColumnCount: 1, + trailingPinnedRowCount: 1, + horizontalDetails: ScrollableDetails.horizontal(controller: horizontalController), + verticalDetails: ScrollableDetails.vertical(controller: verticalController), + 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(), + ), + ); + }, + ), + ), + ), + ), + ); + + // Regular cell (row 8, col 8) lands at x = 275..375, y = 275..375, + // overlapping the trailing corner cell (row 9, col 9) at x = 300..400, + // y = 300..400. A tap at (350, 350) hits both; (row 9, col 9) wins. + horizontalController.jumpTo(525); + verticalController.jumpTo(525); + await tester.pump(); + + await tester.tapAt(const Offset(350, 350)); + await tester.pumpAndSettle(); + + expect( + lastTapped, + const TableVicinity(column: 9, row: 9), + reason: + 'Trailing pinned corner cell (row 9, col 9) must receive the tap; the regular cell (row 8, col 8) scrolled underneath must not.', + ); + }); + + testWidgets('Leading and trailing pinned columns each clip to their own viewport band', ( + WidgetTester tester, + ) async { + final horizontalController = ScrollController(); + final tappedColumns = []; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 100, + width: 400, + child: TableView.builder( + cacheExtent: 0.0, + columnCount: 10, + rowCount: 1, + // col 0 → always at x = 0..100 + pinnedColumnCount: 1, + // col 9 → always at x = 300..400 + trailingPinnedColumnCount: 1, + horizontalDetails: ScrollableDetails.horizontal(controller: horizontalController), + columnBuilder: (_) => const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (_) => const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + child: GestureDetector( + behavior: HitTestBehavior.opaque, + onTap: () => tappedColumns.add(vicinity.column), + child: const SizedBox.expand(), + ), + ); + }, + ), + ), + ), + ), + ); + + // With leading pinned extent = 100 and trailing pinned extent = 100 the + // non-pinned band is x = 100..300 (200 px wide). The formula becomes + // paintOffset.dx(col k) = k×100 − scroll + // so col 8 lands at x = 275..375 at scroll = 525, overlapping trailing + // col 9 (x = 300..400) at x = 300..375. + horizontalController.jumpTo(525); + await tester.pump(); + + // 1. Tap the trailing band (x = 350) → col 9, NOT the underlying col 8. + await tester.tapAt(const Offset(350, 50)); + await tester.pumpAndSettle(); + expect( + tappedColumns.last, + 9, + reason: 'Tap at x=350 should hit trailing pinned col 9, not col 8.', + ); + + // 2. Tap the leading band (x = 50) → col 0. + await tester.tapAt(const Offset(50, 50)); + await tester.pumpAndSettle(); + expect(tappedColumns.last, 0, reason: 'Tap at x=50 should hit leading pinned col 0.'); + + // 3. Tap the middle of the non-pinned band (x = 200) → neither pinned col. + await tester.tapAt(const Offset(200, 50)); + await tester.pumpAndSettle(); + expect( + tappedColumns.last, + isNot(anyOf(0, 9)), + reason: 'Tap at x=200 should hit a non-pinned column.', + ); + }); + + 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.', + ); + }, + ); + + testWidgets( + 'Tapping on a pinned row in an aligned table (with _vAlignmentOffset) registers correctly', + (WidgetTester tester) async { + TableVicinity? lastTapped; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 600, + width: 100, + child: TableView.builder( + columnCount: 1, + rowCount: 3, + pinnedRowCount: 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(), + ), + ); + }, + ), + ), + ), + ), + ); + + // Row 0 (leading pinned) is at y = 150..250. + await tester.tapAt(const Offset(50, 200)); + await tester.pumpAndSettle(); + expect( + lastTapped?.row, + 0, + reason: + 'Leading pinned row 0 shifted to y = 150..250 by the alignment offset must receive a tap at y = 200.', + ); + + // Row 1 (non-pinned) is at y = 250..350. + await tester.tapAt(const Offset(50, 300)); + await tester.pumpAndSettle(); + expect( + lastTapped?.row, + 1, + reason: 'Non-pinned row 1 at y = 250..350 must receive a tap at y = 300.', + ); + }, + ); + + testWidgets( + 'Tapping on pinned cells in an aligned table with both _hAlignmentOffset and _vAlignmentOffset non-zero', + (WidgetTester tester) async { + TableVicinity? lastTapped; + + // 2 columns (col 0 leading pinned) x 2 rows (row 0 leading pinned), + // each 100 px. Viewport 600 x 600 px. Table = 200 x 200 px. + // Alignment.center: + // _hAlignmentOffset = (600 − 200) × 0.5 = 200 + // _vAlignmentOffset = (600 − 200) × 0.5 = 200 + // + // Cell positions (paintOffset): + // (col 0, row 0) — (L,L) corner -> [200, 300) x [200, 300) + // (col 0, row 1) — (L,N) -> [200, 300) x [300, 400) + // (col 1, row 0) — (N,L) -> [300, 400) x [200, 300) + // (col 1, row 1) — (N,N) -> [300, 400) x [300, 400) + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 600, + width: 600, + child: TableView.builder( + columnCount: 2, + rowCount: 2, + pinnedColumnCount: 1, + pinnedRowCount: 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(), + ), + ); + }, + ), + ), + ), + ), + ); + + // (L,L) corner — both column AND row clip must be shifted. + await tester.tapAt(const Offset(250, 250)); + await tester.pumpAndSettle(); + expect( + lastTapped, + TableVicinity.zero, + reason: '(L,L) corner cell at [200,300)x[200,300) must receive tap at (250,250).', + ); + + // (L,N) — only the column clip needs shifting. + await tester.tapAt(const Offset(250, 350)); + await tester.pumpAndSettle(); + expect( + lastTapped, + const TableVicinity(column: 0, row: 1), + reason: '(L,N) cell at [200,300)x[300,400) must receive tap at (250,350).', + ); + + // (N,L) — only the row clip needs shifting. + await tester.tapAt(const Offset(350, 250)); + await tester.pumpAndSettle(); + expect( + lastTapped, + const TableVicinity(column: 1, row: 0), + reason: '(N,L) cell at [300,400)x[200,300) must receive tap at (350,250).', + ); + + // (N,N) — neither clip needs shifting; verifies no regression. + await tester.tapAt(const Offset(350, 350)); + await tester.pumpAndSettle(); + expect( + lastTapped, + const TableVicinity(column: 1, row: 1), + reason: '(N,N) cell at [300,400)x[300,400) must receive tap at (350,350).', + ); + }, + ); + + testWidgets('Tapping on a pinned column in a reversed + aligned table registers correctly', ( + WidgetTester tester, + ) async { + TableVicinity? lastTapped; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 100, + width: 600, + child: TableView.builder( + columnCount: 3, + rowCount: 1, + pinnedColumnCount: 1, + alignment: Alignment.center, + horizontalDetails: const ScrollableDetails.horizontal(reverse: true), + 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(), + ), + ); + }, + ), + ), + ), + ), + ); + + // Leading pinned col 0 is at x = 350..450 (shifted left by alignment + // in the reversed direction). + await tester.tapAt(const Offset(400, 50)); + await tester.pumpAndSettle(); + expect( + lastTapped?.column, + 0, + reason: + 'Leading pinned col 0 is at x = 350..450 in a reversed + aligned table; a tap at x = 400 must reach it.', + ); + }); + }); } class _NullBuildContext implements BuildContext, TwoDimensionalChildManager {