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

iOS TextSelectionToolbar fidelity (#127757)

CupertinoTextSelectionToolbar is different from the native one, with some UI and UX issues. More details on the linked issue.

https://github.com/flutter/flutter/issues/127756

Currently the only problem that I listed on the linked issue that I couldn't fix was the horizontal scrolling, but to workaround this I added a GestureDetector to change pages when swiping the toolbar. It's not exactly the same as native as there is no scroll animation, but it works.

I'm creating this PR a little early to have some feedback as these changes were more complex than the ones in my last PR. Probably best if @justinmc is involved 😅

|Version|Video|
|-|-|
|Flutter Old|<video src="https://github.com/flutter/flutter/assets/12024080/7cf81075-46ec-4970-b118-cc27b60ddac0"></video>|
|Flutter New|<video src="https://github.com/flutter/flutter/assets/12024080/c9e27a53-f94c-4cb0-9b76-e47b73841dcb"></video>|
|Native|<video src="https://github.com/flutter/flutter/assets/12024080/468c7d5b-ba93-4bd4-8f6e-8ec2644b9866"></video>|
parent 3e66c86a
......@@ -11,30 +11,26 @@ import 'localizations.dart';
const TextStyle _kToolbarButtonFontStyle = TextStyle(
inherit: false,
fontSize: 14.0,
fontSize: 15.0,
letterSpacing: -0.15,
fontWeight: FontWeight.w400,
);
// Colors extracted from https://developer.apple.com/design/resources/.
// TODO(LongCatIsLooong): https://github.com/flutter/flutter/issues/41507.
const CupertinoDynamicColor _kToolbarBackgroundColor = CupertinoDynamicColor.withBrightness(
// This value was extracted from a screenshot of iOS 16.0.3, as light mode
// didn't appear in the Apple design resources assets linked above.
color: Color(0xEBF7F7F7),
darkColor: Color(0xEB202020),
);
const CupertinoDynamicColor _kToolbarTextColor = CupertinoDynamicColor.withBrightness(
color: CupertinoColors.black,
darkColor: CupertinoColors.white,
);
// Eyeballed value.
const EdgeInsets _kToolbarButtonPadding = EdgeInsets.symmetric(vertical: 16.0, horizontal: 18.0);
const CupertinoDynamicColor _kToolbarPressedColor = CupertinoDynamicColor.withBrightness(
color: Color(0x10000000),
darkColor: Color(0x10FFFFFF),
);
// Value measured from screenshot of iOS 16.0.2
const EdgeInsets _kToolbarButtonPadding = EdgeInsets.symmetric(vertical: 18.0, horizontal: 16.0);
/// A button in the style of the iOS text selection toolbar buttons.
class CupertinoTextSelectionToolbarButton extends StatelessWidget {
class CupertinoTextSelectionToolbarButton extends StatefulWidget {
/// Create an instance of [CupertinoTextSelectionToolbarButton].
///
/// [child] cannot be null.
......@@ -113,26 +109,65 @@ class CupertinoTextSelectionToolbarButton extends StatelessWidget {
}
}
@override
State<StatefulWidget> createState() => _CupertinoTextSelectionToolbarButtonState();
}
class _CupertinoTextSelectionToolbarButtonState extends State<CupertinoTextSelectionToolbarButton> {
bool isPressed = false;
void _onTapDown(TapDownDetails details) {
setState(() => isPressed = true);
}
void _onTapUp(TapUpDetails details) {
setState(() => isPressed = false);
widget.onPressed?.call();
}
void _onTapCancel() {
setState(() => isPressed = false);
}
@override
Widget build(BuildContext context) {
final Widget child = this.child ?? Text(
text ?? getButtonLabel(context, buttonItem!),
final Widget child = CupertinoButton(
color: isPressed
? _kToolbarPressedColor.resolveFrom(context)
: const Color(0x00000000),
borderRadius: null,
disabledColor: const Color(0x00000000),
// This CupertinoButton does not actually handle the onPressed callback,
// this is only here to correctly enable/disable the button (see
// GestureDetector comment below).
onPressed: widget.onPressed,
padding: _kToolbarButtonPadding,
// There's no foreground fade on iOS toolbar anymore, just the background
// is darkened.
pressedOpacity: 1.0,
child: widget.child ?? Text(
widget.text ?? CupertinoTextSelectionToolbarButton.getButtonLabel(context, widget.buttonItem!),
overflow: TextOverflow.ellipsis,
style: _kToolbarButtonFontStyle.copyWith(
color: onPressed != null
color: widget.onPressed != null
? _kToolbarTextColor.resolveFrom(context)
: CupertinoColors.inactiveGray,
),
),
);
return CupertinoButton(
borderRadius: null,
color: _kToolbarBackgroundColor,
disabledColor: _kToolbarBackgroundColor,
onPressed: onPressed,
padding: _kToolbarButtonPadding,
pressedOpacity: onPressed == null ? 1.0 : 0.7,
if (widget.onPressed != null) {
// As it's needed to change the CupertinoButton's backgroundColor when
// pressed, not its opacity, this GestureDetector handles both the
// onPressed callback and the backgroundColor change.
return GestureDetector(
onTapDown: _onTapDown,
onTapUp: _onTapUp,
onTapCancel: _onTapCancel,
child: child,
);
} else {
return child;
}
}
}
......@@ -1545,7 +1545,7 @@ void main() {
Text text = tester.widget<Text>(find.text('Paste'));
expect(text.style!.color!.value, CupertinoColors.black.value);
expect(text.style!.fontSize, 14);
expect(text.style!.fontSize, 15);
expect(text.style!.letterSpacing, -0.15);
expect(text.style!.fontWeight, FontWeight.w400);
......@@ -1577,7 +1577,7 @@ void main() {
text = tester.widget<Text>(find.text('Paste'));
// The toolbar buttons' text are still the same style.
expect(text.style!.color!.value, CupertinoColors.white.value);
expect(text.style!.fontSize, 14);
expect(text.style!.fontSize, 15);
expect(text.style!.letterSpacing, -0.15);
expect(text.style!.fontWeight, FontWeight.w400);
}, skip: isContextMenuProvidedByPlatform); // [intended] only applies to platforms where we supply the context menu.
......@@ -6537,7 +6537,7 @@ void main() {
topMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy + 8, epsilon: 0.01),
leftMatcher: moreOrLessEquals(8),
rightMatcher: lessThanOrEqualTo(400 - 8),
bottomMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy + 8 + 43, epsilon: 0.01),
bottomMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy + 8 + 45, epsilon: 0.01),
),
),
);
......@@ -6597,7 +6597,7 @@ void main() {
pathMatcher: PathBoundsMatcher(
topMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy + 8, epsilon: 0.01),
rightMatcher: moreOrLessEquals(400.0 - 8),
bottomMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy + 8 + 43, epsilon: 0.01),
bottomMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy + 8 + 45, epsilon: 0.01),
leftMatcher: greaterThanOrEqualTo(8),
),
),
......@@ -6650,7 +6650,7 @@ void main() {
paints..clipPath(
pathMatcher: PathBoundsMatcher(
bottomMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy - 8 - lineHeight, epsilon: 0.01),
topMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy - 8 - lineHeight - 43, epsilon: 0.01),
topMatcher: moreOrLessEquals(bottomLeftSelectionPosition.dy - 8 - lineHeight - 45, epsilon: 0.01),
rightMatcher: lessThanOrEqualTo(400 - 8),
leftMatcher: greaterThanOrEqualTo(8),
),
......@@ -6719,7 +6719,7 @@ void main() {
paints..clipPath(
pathMatcher: PathBoundsMatcher(
bottomMatcher: moreOrLessEquals(selectionPosition.dy - 8 - lineHeight, epsilon: 0.01),
topMatcher: moreOrLessEquals(selectionPosition.dy - 8 - lineHeight - 43, epsilon: 0.01),
topMatcher: moreOrLessEquals(selectionPosition.dy - 8 - lineHeight - 45, epsilon: 0.01),
rightMatcher: lessThanOrEqualTo(400 - 8),
leftMatcher: greaterThanOrEqualTo(8),
),
......@@ -6792,7 +6792,7 @@ void main() {
paints..clipPath(
pathMatcher: PathBoundsMatcher(
bottomMatcher: moreOrLessEquals(selectionPosition.dy - 8 - lineHeight, epsilon: 0.01),
topMatcher: moreOrLessEquals(selectionPosition.dy - 8 - lineHeight - 43, epsilon: 0.01),
topMatcher: moreOrLessEquals(selectionPosition.dy - 8 - lineHeight - 45, epsilon: 0.01),
rightMatcher: lessThanOrEqualTo(400 - 8),
leftMatcher: greaterThanOrEqualTo(8),
),
......
......@@ -29,7 +29,7 @@ void main() {
expect(pressed, true);
});
testWidgets('pressedOpacity defaults to 0.1', (WidgetTester tester) async {
testWidgets('background darkens when pressed', (WidgetTester tester) async {
await tester.pumpWidget(
CupertinoApp(
home: Center(
......@@ -41,35 +41,38 @@ void main() {
),
);
// Original at full opacity.
FadeTransition opacity = tester.widget(find.descendant(
of: find.byType(CupertinoTextSelectionToolbarButton),
matching: find.byType(FadeTransition),
// Original with transparent background.
DecoratedBox decoratedBox = tester.widget(find.descendant(
of: find.byType(CupertinoButton),
matching: find.byType(DecoratedBox),
));
expect(opacity.opacity.value, 1.0);
BoxDecoration boxDecoration = decoratedBox.decoration as BoxDecoration;
expect(boxDecoration.color, const Color(0x00000000));
// Make a "down" gesture on the button.
final Offset center = tester.getCenter(find.byType(CupertinoTextSelectionToolbarButton));
final TestGesture gesture = await tester.startGesture(center);
await tester.pumpAndSettle();
// Opacity reduces during the down gesture.
opacity = tester.widget(find.descendant(
// When pressed, the background darkens.
decoratedBox = tester.widget(find.descendant(
of: find.byType(CupertinoTextSelectionToolbarButton),
matching: find.byType(FadeTransition),
matching: find.byType(DecoratedBox),
));
expect(opacity.opacity.value, 0.7);
boxDecoration = decoratedBox.decoration as BoxDecoration;
expect(boxDecoration.color!.value, const Color(0x10000000).value);
// Release the down gesture.
await gesture.up();
await tester.pumpAndSettle();
// Opacity is back to normal.
opacity = tester.widget(find.descendant(
// Color is back to transparent.
decoratedBox = tester.widget(find.descendant(
of: find.byType(CupertinoTextSelectionToolbarButton),
matching: find.byType(FadeTransition),
matching: find.byType(DecoratedBox),
));
expect(opacity.opacity.value, 1.0);
boxDecoration = decoratedBox.decoration as BoxDecoration;
expect(boxDecoration.color, const Color(0x00000000));
});
testWidgets('passing null to onPressed disables the button', (WidgetTester tester) async {
......
......@@ -6,12 +6,13 @@ import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import '../rendering/mock_canvas.dart';
import '../widgets/editable_text_utils.dart' show textOffsetToPosition;
// These constants are copied from cupertino/text_selection_toolbar.dart.
const double _kArrowScreenPadding = 26.0;
const double _kToolbarContentDistance = 8.0;
const double _kToolbarHeight = 43.0;
const double _kToolbarHeight = 45.0;
// A custom text selection menu that just displays a single custom button.
class _CustomCupertinoTextSelectionControls extends CupertinoTextSelectionControls {
......@@ -60,9 +61,9 @@ class TestBox extends SizedBox {
static const double itemWidth = 100.0;
}
const CupertinoDynamicColor _kToolbarBackgroundColor = CupertinoDynamicColor.withBrightness(
color: Color(0xEBF7F7F7),
darkColor: Color(0xEB202020),
const CupertinoDynamicColor _kToolbarTextColor = CupertinoDynamicColor.withBrightness(
color: CupertinoColors.black,
darkColor: CupertinoColors.white,
);
void main() {
......@@ -81,8 +82,65 @@ void main() {
// visible part of the toolbar for use in measurements.
Finder findToolbar() => findPrivate('_CupertinoTextSelectionToolbarContent');
Finder findOverflowNextButton() => find.text('▶');
Finder findOverflowBackButton() => find.text('◀');
// Check if the middle point of the chevron is pointing left or right.
//
// Offset.dx: a right or left margin (_kToolbarChevronSize / 4 => 2.5) to center the icon horizontally
// Offset.dy: always in the exact vertical center (_kToolbarChevronSize / 2 => 5)
PaintPattern overflowNextPaintPattern() => paints
..line(p1: const Offset(2.5, 0), p2: const Offset(7.5, 5))
..line(p1: const Offset(7.5, 5), p2: const Offset(2.5, 10));
PaintPattern overflowBackPaintPattern() => paints
..line(p1: const Offset(7.5, 0), p2: const Offset(2.5, 5))
..line(p1: const Offset(2.5, 5), p2: const Offset(7.5, 10));
Finder findOverflowNextButton() => find.byWidgetPredicate((Widget widget) =>
widget is CustomPaint &&
'${widget.painter?.runtimeType}' == '_RightCupertinoChevronPainter',
);
Finder findOverflowBackButton() => find.byWidgetPredicate((Widget widget) =>
widget is CustomPaint &&
'${widget.painter?.runtimeType}' == '_LeftCupertinoChevronPainter',
);
testWidgets('chevrons point to the correct side', (WidgetTester tester) async {
// Add enough TestBoxes to need 3 pages.
final List<Widget> children = List<Widget>.generate(15, (int i) => const TestBox());
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoTextSelectionToolbar(
anchorAbove: const Offset(50.0, 100.0),
anchorBelow: const Offset(50.0, 200.0),
children: children,
),
),
),
);
expect(findOverflowBackButton(), findsNothing);
expect(findOverflowNextButton(), findsOneWidget);
expect(findOverflowNextButton(), overflowNextPaintPattern());
// Tap the overflow next button to show the next page of children.
await tester.tapAt(tester.getCenter(findOverflowNextButton()));
await tester.pumpAndSettle();
expect(findOverflowBackButton(), findsOneWidget);
expect(findOverflowNextButton(), findsOneWidget);
expect(findOverflowBackButton(), overflowBackPaintPattern());
expect(findOverflowNextButton(), overflowNextPaintPattern());
// Tap the overflow next button to show the last page of children.
await tester.tapAt(tester.getCenter(findOverflowNextButton()));
await tester.pumpAndSettle();
expect(findOverflowBackButton(), findsOneWidget);
expect(findOverflowNextButton(), findsNothing);
expect(findOverflowBackButton(), overflowBackPaintPattern());
}, skip: kIsWeb); // Path.combine is not implemented in the HTML backend https://github.com/flutter/flutter/issues/44572
testWidgets('paginates children if they overflow', (WidgetTester tester) async {
late StateSetter setState;
......@@ -121,22 +179,15 @@ void main() {
expect(findOverflowBackButton(), findsNothing);
// Tap the overflow next button to show the next page of children.
await tester.tap(findOverflowNextButton());
await tester.pumpAndSettle();
expect(find.byType(TestBox), findsNWidgets(1));
expect(findOverflowNextButton(), findsOneWidget);
expect(findOverflowBackButton(), findsOneWidget);
// Tapping the overflow next button again does nothing because it is
// disabled and there are no more children to display.
await tester.tap(findOverflowNextButton());
// The next button is hidden as there's no next page.
await tester.tapAt(tester.getCenter(findOverflowNextButton()));
await tester.pumpAndSettle();
expect(find.byType(TestBox), findsNWidgets(1));
expect(findOverflowNextButton(), findsOneWidget);
expect(findOverflowNextButton(), findsNothing);
expect(findOverflowBackButton(), findsOneWidget);
// Tap the overflow back button to go back to the first page.
await tester.tap(findOverflowBackButton());
await tester.tapAt(tester.getCenter(findOverflowBackButton()));
await tester.pumpAndSettle();
expect(find.byType(TestBox), findsNWidgets(7));
expect(findOverflowNextButton(), findsOneWidget);
......@@ -157,7 +208,7 @@ void main() {
expect(findOverflowBackButton(), findsNothing);
// Tap the overflow next button to show the second page of children.
await tester.tap(findOverflowNextButton());
await tester.tapAt(tester.getCenter(findOverflowNextButton()));
await tester.pumpAndSettle();
// With the back button, only six children fit on this page.
expect(find.byType(TestBox), findsNWidgets(6));
......@@ -165,21 +216,21 @@ void main() {
expect(findOverflowBackButton(), findsOneWidget);
// Tap the overflow next button again to show the third page of children.
await tester.tap(findOverflowNextButton());
await tester.tapAt(tester.getCenter(findOverflowNextButton()));
await tester.pumpAndSettle();
expect(find.byType(TestBox), findsNWidgets(1));
expect(findOverflowNextButton(), findsOneWidget);
expect(findOverflowNextButton(), findsNothing);
expect(findOverflowBackButton(), findsOneWidget);
// Tap the overflow back button to go back to the second page.
await tester.tap(findOverflowBackButton());
await tester.tapAt(tester.getCenter(findOverflowBackButton()));
await tester.pumpAndSettle();
expect(find.byType(TestBox), findsNWidgets(6));
expect(findOverflowNextButton(), findsOneWidget);
expect(findOverflowBackButton(), findsOneWidget);
// Tap the overflow back button to go back to the first page.
await tester.tap(findOverflowBackButton());
await tester.tapAt(tester.getCenter(findOverflowBackButton()));
await tester.pumpAndSettle();
expect(find.byType(TestBox), findsNWidgets(7));
expect(findOverflowNextButton(), findsOneWidget);
......@@ -345,13 +396,12 @@ void main() {
final Finder buttonFinder = find.byType(CupertinoButton);
expect(buttonFinder, findsOneWidget);
final Finder decorationFinder = find.descendant(
final Finder textFinder = find.descendant(
of: find.byType(CupertinoButton),
matching: find.byType(DecoratedBox)
matching: find.byType(Text)
);
expect(decorationFinder, findsOneWidget);
final DecoratedBox decoratedBox = tester.widget(decorationFinder);
final BoxDecoration boxDecoration = decoratedBox.decoration as BoxDecoration;
expect(textFinder, findsOneWidget);
final Text text = tester.widget(textFinder);
// Theme brightness is preferred, otherwise MediaQuery brightness is
// used. If both are null, defaults to light.
......@@ -363,10 +413,10 @@ void main() {
}
expect(
boxDecoration.color!.value,
text.style!.color!.value,
effectiveBrightness == Brightness.dark
? _kToolbarBackgroundColor.darkColor.value
: _kToolbarBackgroundColor.color.value,
? _kToolbarTextColor.darkColor.value
: _kToolbarTextColor.color.value,
);
}, skip: kIsWeb); // [intended] We do not use Flutter-rendered context menu on the Web.
}
......@@ -419,7 +469,7 @@ void main() {
of: find.byType(CupertinoTextSelectionToolbar),
matching: find.byType(DecoratedBox),
);
expect(finder, findsNWidgets(2));
expect(finder, findsOneWidget);
DecoratedBox decoratedBox = tester.widget(finder.first);
BoxDecoration boxDecoration = decoratedBox.decoration as BoxDecoration;
List<BoxShadow>? shadows = boxDecoration.boxShadow;
......
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