Unverified Commit fc3f4ed8 authored by Luccas Clezar's avatar Luccas Clezar Committed by GitHub

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)|

https://github.com/flutter/flutter/issues/138177
parent 6facb969
......@@ -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;
}
......
......@@ -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: <Widget>[
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;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment