Skip to content

Commit fc3f4ed

Browse files
authored
Fix CupertinoTextSelectionToolbar clipping (#138195)
The CupertinoTextSelectionToolbar sets the maxWidth of the whole toolbar to the width of the first page. This ends up clipping other pages from the toolbar. This PR just removes this limitation. It was easy enough that I thought there was a catch, but I ran the tests locally and they all passed. |Before|After| |-|-| |![Simulator Screenshot - iPhone X� - 2023-11-09 at 19 45 29](https://github.com/flutter/flutter/assets/12024080/c84c40b9-3b02-48bf-9e87-17a9e4cfb461)|![Simulator Screenshot - iPhone X� - 2023-11-09 at 19 44 30](https://github.com/flutter/flutter/assets/12024080/0c3d829b-952e-462b-9f02-8a2833c6f65d)| flutter/flutter#138177
1 parent 6facb96 commit fc3f4ed

File tree

2 files changed

+58
-6
lines changed

2 files changed

+58
-6
lines changed

packages/flutter/lib/src/cupertino/text_selection_toolbar.dart

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,6 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container
10081008
final double subsequentPageButtonsWidth = _backButton!.size.width + _nextButton!.size.width;
10091009
double currentButtonPosition = 0.0;
10101010
late double toolbarWidth; // The width of the whole widget.
1011-
late double firstPageWidth;
10121011
int currentPage = 0;
10131012
int i = -1;
10141013
visitChildren((RenderObject renderObjectChild) {
@@ -1031,7 +1030,7 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container
10311030
// The width of the menu is set by the first page.
10321031
child.layout(
10331032
BoxConstraints(
1034-
maxWidth: (currentPage == 0 ? constraints.maxWidth : firstPageWidth) - paginationButtonsWidth,
1033+
maxWidth: constraints.maxWidth - paginationButtonsWidth,
10351034
minHeight: greatestHeight,
10361035
maxHeight: greatestHeight,
10371036
),
@@ -1047,7 +1046,7 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container
10471046
paginationButtonsWidth = _backButton!.size.width + _nextButton!.size.width;
10481047
child.layout(
10491048
BoxConstraints(
1050-
maxWidth: firstPageWidth - paginationButtonsWidth,
1049+
maxWidth: constraints.maxWidth - paginationButtonsWidth,
10511050
minHeight: greatestHeight,
10521051
maxHeight: greatestHeight,
10531052
),
@@ -1058,9 +1057,6 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container
10581057
currentButtonPosition += child.size.width + dividerWidth;
10591058
childParentData.shouldPaint = currentPage == page;
10601059

1061-
if (currentPage == 0) {
1062-
firstPageWidth = currentButtonPosition + _nextButton!.size.width;
1063-
}
10641060
if (currentPage == page) {
10651061
toolbarWidth = currentButtonPosition;
10661062
}

packages/flutter/test/cupertino/text_selection_toolbar_test.dart

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,62 @@ void main() {
273273
expect(findOverflowBackButton(), findsNothing);
274274
}, skip: kIsWeb); // [intended] We do not use Flutter-rendered context menu on the Web.
275275

276+
testWidgets('correctly sizes large toolbar buttons', (WidgetTester tester) async {
277+
final GlobalKey firstBoxKey = GlobalKey();
278+
final GlobalKey secondBoxKey = GlobalKey();
279+
final GlobalKey thirdBoxKey = GlobalKey();
280+
final GlobalKey fourthBoxKey = GlobalKey();
281+
282+
await tester.pumpWidget(
283+
CupertinoApp(
284+
home: Center(
285+
child: SizedBox(
286+
width: 420,
287+
child: CupertinoTextSelectionToolbar(
288+
anchorAbove: const Offset(50.0, 100.0),
289+
anchorBelow: const Offset(50.0, 200.0),
290+
children: <Widget>[
291+
SizedBox(key: firstBoxKey, width: 100),
292+
SizedBox(key: secondBoxKey, width: 300),
293+
SizedBox(key: thirdBoxKey, width: 100),
294+
SizedBox(key: fourthBoxKey, width: 100),
295+
],
296+
),
297+
),
298+
),
299+
),
300+
);
301+
302+
// The first page isn't wide enough to show the second button.
303+
expect(find.byKey(firstBoxKey), findsOneWidget);
304+
expect(find.byKey(secondBoxKey), findsNothing);
305+
expect(find.byKey(thirdBoxKey), findsNothing);
306+
expect(find.byKey(fourthBoxKey), findsNothing);
307+
308+
// Show the next page.
309+
await tester.tapAt(tester.getCenter(findOverflowNextButton()));
310+
await tester.pumpAndSettle();
311+
312+
// The second page should show only the second button.
313+
expect(find.byKey(firstBoxKey), findsNothing);
314+
expect(find.byKey(secondBoxKey), findsOneWidget);
315+
expect(find.byKey(thirdBoxKey), findsNothing);
316+
expect(find.byKey(fourthBoxKey), findsNothing);
317+
318+
// The button's width shouldn't be limited by the first page's width.
319+
expect(tester.getSize(find.byKey(secondBoxKey)).width, 300);
320+
321+
// Show the next page.
322+
await tester.tapAt(tester.getCenter(findOverflowNextButton()));
323+
await tester.pumpAndSettle();
324+
325+
// The third page should show the last two items.
326+
expect(find.byKey(firstBoxKey), findsNothing);
327+
expect(find.byKey(secondBoxKey), findsNothing);
328+
expect(find.byKey(thirdBoxKey), findsOneWidget);
329+
expect(find.byKey(fourthBoxKey), findsOneWidget);
330+
}, skip: kIsWeb); // [intended] We do not use Flutter-rendered context menu on the Web.
331+
276332
testWidgets('positions itself at anchorAbove if it fits', (WidgetTester tester) async {
277333
late StateSetter setState;
278334
const double height = 50.0;

0 commit comments

Comments
 (0)