From fc3f4ed882cdb5e89039ed5a90899ae0aca502cd Mon Sep 17 00:00:00 2001 From: Luccas Clezar Date: Fri, 2 Feb 2024 17:22:42 -0300 Subject: [PATCH] Fix CupertinoTextSelectionToolbar clipping (#138195) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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)| https://github.com/flutter/flutter/issues/138177 --- .../src/cupertino/text_selection_toolbar.dart | 8 +-- .../text_selection_toolbar_test.dart | 56 +++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/text_selection_toolbar.dart b/packages/flutter/lib/src/cupertino/text_selection_toolbar.dart index 6e89fc6dd5d..0bd6f1db7b0 100644 --- a/packages/flutter/lib/src/cupertino/text_selection_toolbar.dart +++ b/packages/flutter/lib/src/cupertino/text_selection_toolbar.dart @@ -1008,7 +1008,6 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container final double subsequentPageButtonsWidth = _backButton!.size.width + _nextButton!.size.width; double currentButtonPosition = 0.0; late double toolbarWidth; // The width of the whole widget. - late double firstPageWidth; int currentPage = 0; int i = -1; visitChildren((RenderObject renderObjectChild) { @@ -1031,7 +1030,7 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container // The width of the menu is set by the first page. child.layout( BoxConstraints( - maxWidth: (currentPage == 0 ? constraints.maxWidth : firstPageWidth) - paginationButtonsWidth, + maxWidth: constraints.maxWidth - paginationButtonsWidth, minHeight: greatestHeight, maxHeight: greatestHeight, ), @@ -1047,7 +1046,7 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container paginationButtonsWidth = _backButton!.size.width + _nextButton!.size.width; child.layout( BoxConstraints( - maxWidth: firstPageWidth - paginationButtonsWidth, + maxWidth: constraints.maxWidth - paginationButtonsWidth, minHeight: greatestHeight, maxHeight: greatestHeight, ), @@ -1058,9 +1057,6 @@ class _RenderCupertinoTextSelectionToolbarItems extends RenderBox with Container currentButtonPosition += child.size.width + dividerWidth; childParentData.shouldPaint = currentPage == page; - if (currentPage == 0) { - firstPageWidth = currentButtonPosition + _nextButton!.size.width; - } if (currentPage == page) { toolbarWidth = currentButtonPosition; } diff --git a/packages/flutter/test/cupertino/text_selection_toolbar_test.dart b/packages/flutter/test/cupertino/text_selection_toolbar_test.dart index 170d9d556e8..bf46a864b7c 100644 --- a/packages/flutter/test/cupertino/text_selection_toolbar_test.dart +++ b/packages/flutter/test/cupertino/text_selection_toolbar_test.dart @@ -273,6 +273,62 @@ void main() { expect(findOverflowBackButton(), findsNothing); }, skip: kIsWeb); // [intended] We do not use Flutter-rendered context menu on the Web. + testWidgets('correctly sizes large toolbar buttons', (WidgetTester tester) async { + final GlobalKey firstBoxKey = GlobalKey(); + final GlobalKey secondBoxKey = GlobalKey(); + final GlobalKey thirdBoxKey = GlobalKey(); + final GlobalKey fourthBoxKey = GlobalKey(); + + await tester.pumpWidget( + CupertinoApp( + home: Center( + child: SizedBox( + width: 420, + child: CupertinoTextSelectionToolbar( + anchorAbove: const Offset(50.0, 100.0), + anchorBelow: const Offset(50.0, 200.0), + children: [ + SizedBox(key: firstBoxKey, width: 100), + SizedBox(key: secondBoxKey, width: 300), + SizedBox(key: thirdBoxKey, width: 100), + SizedBox(key: fourthBoxKey, width: 100), + ], + ), + ), + ), + ), + ); + + // The first page isn't wide enough to show the second button. + expect(find.byKey(firstBoxKey), findsOneWidget); + expect(find.byKey(secondBoxKey), findsNothing); + expect(find.byKey(thirdBoxKey), findsNothing); + expect(find.byKey(fourthBoxKey), findsNothing); + + // Show the next page. + await tester.tapAt(tester.getCenter(findOverflowNextButton())); + await tester.pumpAndSettle(); + + // The second page should show only the second button. + expect(find.byKey(firstBoxKey), findsNothing); + expect(find.byKey(secondBoxKey), findsOneWidget); + expect(find.byKey(thirdBoxKey), findsNothing); + expect(find.byKey(fourthBoxKey), findsNothing); + + // The button's width shouldn't be limited by the first page's width. + expect(tester.getSize(find.byKey(secondBoxKey)).width, 300); + + // Show the next page. + await tester.tapAt(tester.getCenter(findOverflowNextButton())); + await tester.pumpAndSettle(); + + // The third page should show the last two items. + expect(find.byKey(firstBoxKey), findsNothing); + expect(find.byKey(secondBoxKey), findsNothing); + expect(find.byKey(thirdBoxKey), findsOneWidget); + expect(find.byKey(fourthBoxKey), findsOneWidget); + }, skip: kIsWeb); // [intended] We do not use Flutter-rendered context menu on the Web. + testWidgets('positions itself at anchorAbove if it fits', (WidgetTester tester) async { late StateSetter setState; const double height = 50.0;