Skip to content

Commit c3360ac

Browse files
authored
[two_dimensional_scrollables] Fix mouse event loop when calling setState in TableSpan.onEnter (#11606)
This PR addresses a bug in TableView where calling setState (or any action that triggers a delegate rebuild) inside a TableSpan.onEnter callback would cause an infinite loop of onExit and onEnter events. The RenderTableViewport was clearing its cached _columnMetrics and _rowMetrics (_Span objects) on every delegate rebuild. Since _Span implements MouseTrackerAnnotation, and the MouseTracker identifies regions by object identity, re-creating these objects every frame caused the MouseTracker to: 1. Trigger onExit for the old _Span object. 2. Trigger onEnter for the new _Span object. If onEnter contained a setState call, this cycle would repeat indefinitely. Fixes flutter/flutter#147614 Fix: - Refactored _updateRowMetrics and _updateColumnMetrics to reuse existing _Span objects when available. - Updated _Span.update to only refresh the internal configuration and layout metrics while maintaining the same object instance. - Modified layoutChildSequence to avoid clearing the metrics maps, instead allowing the update methods to handle stale entries. - Added logic to properly dispose and remove spans when the row or column count is reduced. ## 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.
1 parent 3b8c4cb commit c3360ac

4 files changed

Lines changed: 107 additions & 13 deletions

File tree

packages/two_dimensional_scrollables/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.5.1
2+
3+
* Fixes an infinite loop of onExit/onEnter events when setState is called within onEnter in a TableSpan.
4+
15
## 0.5.0
26

37
* Adds support for trailing pinned columns and rows in TableView.

packages/two_dimensional_scrollables/lib/src/table_view/table.dart

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -687,18 +687,25 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
687687
: startOfTrailingPinnedColumn)
688688
: startOfRegularColumn;
689689
_Span? span = _columnMetrics.remove(column);
690-
final TableSpan? configuration =
691-
span?.configuration ?? delegate.buildColumn(column);
690+
final TableSpan? configuration = (needsDelegateRebuild || span == null)
691+
? delegate.buildColumn(column)
692+
: span.configuration;
692693
if (configuration == null) {
693694
// We have reached the end of columns based on a null termination. This
694695
// This happens when a column count has not been specified.
695696
assert(_columnsAreInfinite);
696697
_lastNonPinnedColumn ??= column - 1;
697698
_columnNullTerminatedIndex = column;
699+
// If we are starting from 0, we should dispose of any metrics that are
700+
// no longer in use. This happens when the number of columns is reduced.
701+
if (!appendColumns) {
702+
_disposeTrailingSpans(_columnMetrics, column);
703+
}
698704
final bool acceptedDimension = _updateHorizontalScrollBounds();
699705
if (!acceptedDimension) {
700706
_updateFirstAndLastVisibleCell();
701707
}
708+
span?.dispose();
702709
break;
703710
}
704711
span ??= _Span();
@@ -732,6 +739,10 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
732739
column++;
733740
}
734741

742+
if (!appendColumns) {
743+
_disposeTrailingSpans(_columnMetrics, column);
744+
}
745+
735746
assert(_columnMetrics.length >= delegate.pinnedColumnCount);
736747
if (_firstNonPinnedColumn != null) {
737748
_lastNonPinnedColumn ??= _columnNullTerminatedIndex != null
@@ -804,19 +815,26 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
804815
: startOfTrailingPinnedRow)
805816
: startOfRegularRow;
806817
_Span? span = _rowMetrics.remove(row);
807-
final TableSpan? configuration =
808-
span?.configuration ?? delegate.buildRow(row);
818+
final TableSpan? configuration = (needsDelegateRebuild || span == null)
819+
? delegate.buildRow(row)
820+
: span.configuration;
809821
if (configuration == null) {
810822
// We have reached the end of rows based on a null termination. This
811823
// This happens when a row count has not been specified, but we have
812824
// reached the end.
813825
assert(_rowsAreInfinite);
814826
_lastNonPinnedRow ??= row - 1;
815827
_rowNullTerminatedIndex = row;
828+
// If we are starting from 0, we should dispose of any metrics that are
829+
// no longer in use. This happens when the number of rows is reduced.
830+
if (!appendRows) {
831+
_disposeTrailingSpans(_rowMetrics, row);
832+
}
816833
final bool acceptedDimension = _updateVerticalScrollBounds();
817834
if (!acceptedDimension) {
818835
_updateFirstAndLastVisibleCell();
819836
}
837+
span?.dispose();
820838
break;
821839
}
822840
span ??= _Span();
@@ -850,6 +868,10 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
850868
row++;
851869
}
852870

871+
if (!appendRows) {
872+
_disposeTrailingSpans(_rowMetrics, row);
873+
}
874+
853875
assert(_rowMetrics.length >= delegate.pinnedRowCount);
854876
if (_firstNonPinnedRow != null) {
855877
_lastNonPinnedRow ??= _rowNullTerminatedIndex != null
@@ -872,6 +894,16 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
872894
: null
873895
: delegate.rowCount! - delegate.trailingPinnedRowCount - 1;
874896

897+
void _disposeTrailingSpans(Map<int, _Span> metrics, int startIndex) {
898+
metrics.removeWhere((int key, _Span span) {
899+
if (key >= startIndex) {
900+
span.dispose();
901+
return true;
902+
}
903+
return false;
904+
});
905+
}
906+
875907
void _updateScrollBounds() {
876908
final bool acceptedDimension =
877909
_updateHorizontalScrollBounds() && _updateVerticalScrollBounds();
@@ -1041,14 +1073,6 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
10411073

10421074
if (needsDelegateRebuild || didResize) {
10431075
// Recomputes the table metrics, invalidates any cached information.
1044-
for (final _Span span in _columnMetrics.values) {
1045-
span.dispose();
1046-
}
1047-
_columnMetrics.clear();
1048-
for (final _Span span in _rowMetrics.values) {
1049-
span.dispose();
1050-
}
1051-
_rowMetrics.clear();
10521076
_updateColumnMetrics();
10531077
_updateRowMetrics();
10541078
_updateScrollBounds();

packages/two_dimensional_scrollables/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name: two_dimensional_scrollables
22
description: Widgets that scroll using the two dimensional scrolling foundation.
3-
version: 0.5.0
3+
version: 0.5.1
44
repository: https://github.com/flutter/packages/tree/main/packages/two_dimensional_scrollables
55
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+two_dimensional_scrollables%22+
66

packages/two_dimensional_scrollables/test/table_view/table_test.dart

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3630,8 +3630,74 @@ void main() {
36303630
RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
36313631
SystemMouseCursors.basic,
36323632
);
3633+
await gesture.removePointer();
36333634
});
36343635

3636+
testWidgets(
3637+
'Calling setState within onEnter does not cause a loop of onExit/onEnter',
3638+
(WidgetTester tester) async {
3639+
// Regression test for https://github.com/flutter/flutter/issues/147614
3640+
var enterCounter = 0;
3641+
var exitCounter = 0;
3642+
3643+
await tester.pumpWidget(
3644+
MaterialApp(
3645+
home: Scaffold(
3646+
body: StatefulBuilder(
3647+
builder: (BuildContext context, StateSetter setState) {
3648+
return TableView.builder(
3649+
columnCount: 1,
3650+
rowCount: 1,
3651+
columnBuilder: (int index) =>
3652+
const TableSpan(extent: FixedTableSpanExtent(100)),
3653+
rowBuilder: (int index) => TableSpan(
3654+
extent: const FixedTableSpanExtent(100),
3655+
onEnter: (_) {
3656+
enterCounter++;
3657+
setState(() {});
3658+
},
3659+
onExit: (_) {
3660+
exitCounter++;
3661+
},
3662+
),
3663+
cellBuilder:
3664+
(BuildContext context, TableVicinity vicinity) {
3665+
return const TableViewCell(
3666+
child: SizedBox.square(dimension: 100),
3667+
);
3668+
},
3669+
);
3670+
},
3671+
),
3672+
),
3673+
),
3674+
);
3675+
3676+
// Initial state
3677+
expect(enterCounter, 0);
3678+
expect(exitCounter, 0);
3679+
3680+
// Move mouse to the center of the first row (0,0)
3681+
final TestGesture gesture = await tester.createGesture(
3682+
kind: PointerDeviceKind.mouse,
3683+
);
3684+
await gesture.addPointer(location: const Offset(50, 50));
3685+
await tester.pump();
3686+
3687+
// Should have entered once
3688+
expect(enterCounter, 1);
3689+
expect(exitCounter, 0);
3690+
3691+
// Pump again to see if it triggers again
3692+
await tester.pump();
3693+
3694+
expect(exitCounter, 0, reason: 'Should not have exited');
3695+
expect(enterCounter, 1, reason: 'Should not have re-entered');
3696+
3697+
await gesture.removePointer();
3698+
},
3699+
);
3700+
36353701
group('Merged pinned cells layout', () {
36363702
// Regression tests for https://github.com/flutter/flutter/issues/143526
36373703
// These tests all use the same collection of merged pinned cells in a

0 commit comments

Comments
 (0)