Unverified Commit b3c237ce authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

Modernize selection menu appearance (#59115)

parent c79de782
......@@ -872,19 +872,19 @@ class DefaultMaterialLocalizations implements MaterialLocalizations {
String get continueButtonLabel => 'CONTINUE';
@override
String get copyButtonLabel => 'COPY';
String get copyButtonLabel => 'Copy';
@override
String get cutButtonLabel => 'CUT';
String get cutButtonLabel => 'Cut';
@override
String get okButtonLabel => 'OK';
@override
String get pasteButtonLabel => 'PASTE';
String get pasteButtonLabel => 'Paste';
@override
String get selectAllButtonLabel => 'SELECT ALL';
String get selectAllButtonLabel => 'Select all';
@override
String get viewLicensesButtonLabel => 'VIEW LICENSES';
......
......@@ -11,6 +11,9 @@ import 'package:flutter/scheduler.dart';
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';
import 'button_theme.dart';
import 'colors.dart';
import 'constants.dart';
import 'debug.dart';
import 'flat_button.dart';
import 'icon_button.dart';
......@@ -54,6 +57,18 @@ class _TextSelectionToolbar extends StatefulWidget {
_TextSelectionToolbarState createState() => _TextSelectionToolbarState();
}
// Intermediate data used for building menu items with the _getItems method.
class _ItemData {
const _ItemData(
this.onPressed,
this.label,
) : assert(onPressed != null),
assert(label != null);
final VoidCallback onPressed;
final String label;
}
class _TextSelectionToolbarState extends State<_TextSelectionToolbar> with TickerProviderStateMixin {
ClipboardStatusNotifier _clipboardStatus;
......@@ -65,11 +80,25 @@ class _TextSelectionToolbarState extends State<_TextSelectionToolbar> with Ticke
// The key for _TextSelectionToolbarContainer.
UniqueKey _containerKey = UniqueKey();
FlatButton _getItem(VoidCallback onPressed, String label) {
assert(onPressed != null);
return FlatButton(
child: Text(label),
onPressed: onPressed,
Widget _getItem(_ItemData itemData, bool isFirst, bool isLast) {
assert(isFirst != null);
assert(isLast != null);
return ButtonTheme.fromButtonThemeData(
data: ButtonTheme.of(context).copyWith(
height: kMinInteractiveDimension,
minWidth: kMinInteractiveDimension,
),
child: FlatButton(
onPressed: itemData.onPressed,
padding: EdgeInsets.only(
// These values were eyeballed to match the native text selection menu
// on a Pixel 2 running Android 10.
left: 9.5 + (isFirst ? 5.0 : 0.0),
right: 9.5 + (isLast ? 5.0 : 0.0),
),
shape: Border.all(width: 0.0, color: Colors.transparent),
child: Text(itemData.label),
),
);
}
......@@ -153,20 +182,20 @@ class _TextSelectionToolbarState extends State<_TextSelectionToolbar> with Ticke
}
final MaterialLocalizations localizations = MaterialLocalizations.of(context);
final List<Widget> items = <Widget>[
final List<_ItemData> itemDatas = <_ItemData>[
if (widget.handleCut != null)
_getItem(widget.handleCut, localizations.cutButtonLabel),
_ItemData(widget.handleCut, localizations.cutButtonLabel),
if (widget.handleCopy != null)
_getItem(widget.handleCopy, localizations.copyButtonLabel),
_ItemData(widget.handleCopy, localizations.copyButtonLabel),
if (widget.handlePaste != null
&& _clipboardStatus.value == ClipboardStatus.pasteable)
_getItem(widget.handlePaste, localizations.pasteButtonLabel),
_ItemData(widget.handlePaste, localizations.pasteButtonLabel),
if (widget.handleSelectAll != null)
_getItem(widget.handleSelectAll, localizations.selectAllButtonLabel),
_ItemData(widget.handleSelectAll, localizations.selectAllButtonLabel),
];
// If there is no option available, build an empty widget.
if (items.isEmpty) {
if (itemDatas.isEmpty) {
return const SizedBox(width: 0.0, height: 0.0);
}
......@@ -179,7 +208,12 @@ class _TextSelectionToolbarState extends State<_TextSelectionToolbar> with Ticke
// API 28.
duration: const Duration(milliseconds: 140),
child: Material(
// This value was eyeballed to match the native text selection menu on
// a Pixel 2 running Android 10.
borderRadius: const BorderRadius.all(Radius.circular(7.0)),
clipBehavior: Clip.antiAlias,
elevation: 1.0,
type: MaterialType.card,
child: _TextSelectionToolbarItems(
isAbove: widget.isAbove,
overflowOpen: _overflowOpen,
......@@ -187,6 +221,7 @@ class _TextSelectionToolbarState extends State<_TextSelectionToolbar> with Ticke
// The navButton that shows and hides the overflow menu is the
// first child.
Material(
type: MaterialType.card,
child: IconButton(
// TODO(justinmc): This should be an AnimatedIcon, but
// AnimatedIcons doesn't yet support arrow_back to more_vert.
......@@ -202,7 +237,8 @@ class _TextSelectionToolbarState extends State<_TextSelectionToolbar> with Ticke
: localizations.moreButtonTooltip,
),
),
...items,
for (int i = 0; i < itemDatas.length; i++)
_getItem(itemDatas[i], i == 0, i == itemDatas.length - 1)
],
),
),
......
......@@ -175,7 +175,7 @@ void main() {
setUp(() async {
EditableText.debugDeterministicCursor = false;
// Fill the clipboard so that the PASTE option is available in the text
// Fill the clipboard so that the Paste option is available in the text
// selection menu.
await Clipboard.setData(const ClipboardData(text: 'Clipboard data'));
});
......
......@@ -507,7 +507,7 @@ void main() {
);
// Default US "select all" text.
expect(find.text('SELECT ALL'), findsOneWidget);
expect(find.text('Select all'), findsOneWidget);
// Default Cupertino US "select all" text.
expect(find.text('Select All'), findsOneWidget);
});
......
......@@ -73,7 +73,7 @@ void main() {
fieldLabelText = null;
helpText = null;
// Fill the clipboard so that the PASTE option is available in the text
// Fill the clipboard so that the Paste option is available in the text
// selection menu.
SystemChannels.platform.setMockMethodCallHandler(mockClipboard.handleMethodCall);
await Clipboard.setData(const ClipboardData(text: 'Clipboard data'));
......
......@@ -32,7 +32,7 @@ void main() {
final MockClipboard mockClipboard = MockClipboard();
setUp(() async {
// Fill the clipboard so that the PASTE option is available in the text
// Fill the clipboard so that the Paste option is available in the text
// selection menu.
SystemChannels.platform.setMockMethodCallHandler(mockClipboard.handleMethodCall);
await Clipboard.setData(const ClipboardData(text: 'Clipboard data'));
......
......@@ -338,8 +338,8 @@ void main() {
await tester.pump();
// Context menu should not have paste.
expect(find.text('SELECT ALL'), findsOneWidget);
expect(find.text('PASTE'), findsNothing);
expect(find.text('Select all'), findsOneWidget);
expect(find.text('Paste'), findsNothing);
final EditableTextState editableTextState = tester.firstState(find.byType(EditableText));
final RenderEditable renderEditable = editableTextState.renderEditable;
......
......@@ -22,7 +22,7 @@ const Color cursorColor = Color.fromARGB(0xFF, 0xFF, 0x00, 0x00);
void main() {
setUp(() async {
// Fill the clipboard so that the PASTE option is available in the text
// Fill the clipboard so that the Paste option is available in the text
// selection menu.
await Clipboard.setData(const ClipboardData(text: 'Clipboard data'));
});
......@@ -88,7 +88,7 @@ void main() {
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(const Duration(milliseconds: 500));
await tester.tap(find.text('PASTE'));
await tester.tap(find.text('Paste'));
await tester.pump();
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(const Duration(milliseconds: 500));
......@@ -141,7 +141,7 @@ void main() {
tester.state<EditableTextState>(textFinder).showToolbar();
await tester.pumpAndSettle();
await tester.tap(find.text('PASTE'));
await tester.tap(find.text('Paste'));
await tester.pump();
expect(changedValue, clipboardContent);
......@@ -779,7 +779,7 @@ void main() {
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(const Duration(milliseconds: 500));
await tester.tap(find.text('PASTE'));
await tester.tap(find.text('Paste'));
await tester.pump();
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(const Duration(milliseconds: 500));
......
......@@ -54,7 +54,7 @@ void main() {
setUp(() async {
debugResetSemanticsIdCounter();
controller = TextEditingController();
// Fill the clipboard so that the PASTE option is available in the text
// Fill the clipboard so that the Paste option is available in the text
// selection menu.
await Clipboard.setData(const ClipboardData(text: 'Clipboard data'));
});
......@@ -1096,7 +1096,7 @@ void main() {
// Can't show the toolbar when there's no focus.
expect(state.showToolbar(), false);
await tester.pumpAndSettle();
expect(find.text('PASTE'), findsNothing);
expect(find.text('Paste'), findsNothing);
// Can show the toolbar when focused even though there's no text.
state.renderEditable.selectWordsInRange(
......@@ -1106,19 +1106,19 @@ void main() {
await tester.pump();
expect(state.showToolbar(), true);
await tester.pumpAndSettle();
expect(find.text('PASTE'), findsOneWidget);
expect(find.text('Paste'), findsOneWidget);
// Hide the menu again.
state.hideToolbar();
await tester.pump();
expect(find.text('PASTE'), findsNothing);
expect(find.text('Paste'), findsNothing);
// Can show the menu with text and a selection.
controller.text = 'blah';
await tester.pump();
expect(state.showToolbar(), true);
await tester.pumpAndSettle();
expect(find.text('PASTE'), findsOneWidget);
expect(find.text('Paste'), findsOneWidget);
}, skip: isBrowser);
testWidgets('can show the toolbar after clearing all text', (WidgetTester tester) async {
......@@ -1149,7 +1149,7 @@ void main() {
await tester.pump();
// Clear the text and selection.
expect(find.text('PASTE'), findsNothing);
expect(find.text('Paste'), findsNothing);
state.updateEditingValue(const TextEditingValue(
text: '',
));
......@@ -1158,7 +1158,7 @@ void main() {
// Should be able to show the toolbar.
expect(state.showToolbar(), true);
await tester.pumpAndSettle();
expect(find.text('PASTE'), findsOneWidget);
expect(find.text('Paste'), findsOneWidget);
});
testWidgets('can dynamically disable options in toolbar', (WidgetTester tester) async {
......@@ -1190,10 +1190,10 @@ void main() {
await tester.pump();
expect(state.showToolbar(), true);
await tester.pump();
expect(find.text('SELECT ALL'), findsOneWidget);
expect(find.text('COPY'), findsOneWidget);
expect(find.text('PASTE'), findsNothing);
expect(find.text('CUT'), findsNothing);
expect(find.text('Select all'), findsOneWidget);
expect(find.text('Copy'), findsOneWidget);
expect(find.text('Paste'), findsNothing);
expect(find.text('Cut'), findsNothing);
});
testWidgets('can dynamically disable select all option in toolbar - cupertino', (WidgetTester tester) async {
......@@ -1256,10 +1256,10 @@ void main() {
await tester.pump();
expect(state.showToolbar(), true);
await tester.pump();
expect(find.text('SELECT ALL'), findsNothing);
expect(find.text('COPY'), findsOneWidget);
expect(find.text('PASTE'), findsNothing);
expect(find.text('CUT'), findsNothing);
expect(find.text('Select all'), findsNothing);
expect(find.text('Copy'), findsOneWidget);
expect(find.text('Paste'), findsNothing);
expect(find.text('Cut'), findsNothing);
});
testWidgets('cut and paste are disabled in read only mode even if explicit set', (WidgetTester tester) async {
......@@ -1294,10 +1294,10 @@ void main() {
await tester.pump();
expect(state.showToolbar(), true);
await tester.pump();
expect(find.text('SELECT ALL'), findsOneWidget);
expect(find.text('COPY'), findsOneWidget);
expect(find.text('PASTE'), findsNothing);
expect(find.text('CUT'), findsNothing);
expect(find.text('Select all'), findsOneWidget);
expect(find.text('Copy'), findsOneWidget);
expect(find.text('Paste'), findsNothing);
expect(find.text('Cut'), findsNothing);
});
testWidgets('Fires onChanged when text changes via TextSelectionOverlay', (WidgetTester tester) async {
......@@ -1328,7 +1328,7 @@ void main() {
tester.state<EditableTextState>(textFinder).showToolbar();
await tester.pumpAndSettle();
await tester.tap(find.text('PASTE'));
await tester.tap(find.text('Paste'));
await tester.pump();
expect(changedValue, clipboardContent);
......
......@@ -456,7 +456,7 @@ void main() {
await tester.pumpAndSettle();
await tester.pump(const Duration(seconds: 1));
expect(find.text('SELECT ALL'), findsOneWidget);
expect(find.text('Select all'), findsOneWidget);
});
testWidgets('Caret position is updated on tap', (WidgetTester tester) async {
......@@ -633,9 +633,9 @@ void main() {
await tester.pump();
// Context menu should not have paste and cut.
expect(find.text('COPY'), findsOneWidget);
expect(find.text('PASTE'), findsNothing);
expect(find.text('CUT'), findsNothing);
expect(find.text('Copy'), findsOneWidget);
expect(find.text('Paste'), findsNothing);
expect(find.text('Cut'), findsNothing);
});
testWidgets('selectable text can disable toolbar options', (WidgetTester tester) async {
......@@ -655,8 +655,8 @@ void main() {
await tester.longPressAt(dPos);
await tester.pump();
// Context menu should not have copy.
expect(find.text('COPY'), findsNothing);
expect(find.text('SELECT ALL'), findsOneWidget);
expect(find.text('Copy'), findsNothing);
expect(find.text('Select all'), findsOneWidget);
});
testWidgets('Can select text by dragging with a mouse', (WidgetTester tester) async {
......@@ -948,14 +948,14 @@ void main() {
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
// SELECT ALL should select all the text.
await tester.tap(find.text('SELECT ALL'));
// Select all should select all the text.
await tester.tap(find.text('Select all'));
await tester.pump();
expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, testValue.length);
// COPY should reset the selection.
await tester.tap(find.text('COPY'));
// Copy should reset the selection.
await tester.tap(find.text('Copy'));
await skipPastScrollingAnimation(tester);
expect(controller.selection.isCollapsed, true);
});
......@@ -1086,7 +1086,7 @@ void main() {
expect(controller.selection.baseOffset, 5);
expect(controller.selection.extentOffset, 50);
await tester.tap(find.text('COPY'));
await tester.tap(find.text('Copy'));
await tester.pump();
expect(controller.selection.isCollapsed, true);
});
......
......@@ -3855,10 +3855,10 @@ class MaterialLocalizationEn extends GlobalMaterialLocalizations {
String get continueButtonLabel => 'CONTINUE';
@override
String get copyButtonLabel => 'COPY';
String get copyButtonLabel => 'Copy';
@override
String get cutButtonLabel => 'CUT';
String get cutButtonLabel => 'Cut';
@override
String get dateHelpText => 'mm/dd/yyyy';
......@@ -3942,7 +3942,7 @@ class MaterialLocalizationEn extends GlobalMaterialLocalizations {
String get pageRowsInfoTitleApproximateRaw => '\$firstRow\$lastRow of about \$rowCount';
@override
String get pasteButtonLabel => 'PASTE';
String get pasteButtonLabel => 'Paste';
@override
String get popupMenuLabel => 'Popup menu';
......@@ -4008,7 +4008,7 @@ class MaterialLocalizationEn extends GlobalMaterialLocalizations {
String get searchFieldLabel => 'Search';
@override
String get selectAllButtonLabel => 'SELECT ALL';
String get selectAllButtonLabel => 'Select all';
@override
String get selectYearSemanticsLabel => 'Select year';
......@@ -4094,6 +4094,18 @@ class MaterialLocalizationEnAu extends MaterialLocalizationEn {
@override
String get licensesPageTitle => 'Licences';
@override
String get copyButtonLabel => 'COPY';
@override
String get cutButtonLabel => 'CUT';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
......@@ -4142,6 +4154,18 @@ class MaterialLocalizationEnCa extends MaterialLocalizationEn {
@override
String get licensesPageTitle => 'Licences';
@override
String get copyButtonLabel => 'COPY';
@override
String get cutButtonLabel => 'CUT';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
......@@ -4190,12 +4214,24 @@ class MaterialLocalizationEnGb extends MaterialLocalizationEn {
@override
TimeOfDayFormat get timeOfDayFormatRaw => TimeOfDayFormat.HH_colon_mm;
@override
String get copyButtonLabel => 'COPY';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
@override
String get licensesPageTitle => 'Licences';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get cutButtonLabel => 'CUT';
@override
String get popupMenuLabel => 'Pop-up menu';
......@@ -4241,12 +4277,24 @@ class MaterialLocalizationEnIe extends MaterialLocalizationEn {
@override
TimeOfDayFormat get timeOfDayFormatRaw => TimeOfDayFormat.HH_colon_mm;
@override
String get copyButtonLabel => 'COPY';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
@override
String get licensesPageTitle => 'Licences';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get cutButtonLabel => 'CUT';
@override
String get popupMenuLabel => 'Pop-up menu';
......@@ -4292,6 +4340,18 @@ class MaterialLocalizationEnIn extends MaterialLocalizationEn {
@override
String get licensesPageTitle => 'Licences';
@override
String get copyButtonLabel => 'COPY';
@override
String get cutButtonLabel => 'CUT';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
......@@ -4340,6 +4400,18 @@ class MaterialLocalizationEnNz extends MaterialLocalizationEn {
@override
String get licensesPageTitle => 'Licences';
@override
String get copyButtonLabel => 'COPY';
@override
String get cutButtonLabel => 'CUT';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
......@@ -4388,6 +4460,18 @@ class MaterialLocalizationEnSg extends MaterialLocalizationEn {
@override
String get licensesPageTitle => 'Licences';
@override
String get copyButtonLabel => 'COPY';
@override
String get cutButtonLabel => 'CUT';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
......@@ -4436,12 +4520,24 @@ class MaterialLocalizationEnZa extends MaterialLocalizationEn {
@override
TimeOfDayFormat get timeOfDayFormatRaw => TimeOfDayFormat.HH_colon_mm;
@override
String get copyButtonLabel => 'COPY';
@override
String get selectAllButtonLabel => 'SELECT ALL';
@override
String get viewLicensesButtonLabel => 'VIEW LICENCES';
@override
String get licensesPageTitle => 'Licences';
@override
String get pasteButtonLabel => 'PASTE';
@override
String get cutButtonLabel => 'CUT';
@override
String get popupMenuLabel => 'Pop-up menu';
......
......@@ -118,12 +118,12 @@
"description": "The label for continue buttons and menu items."
},
"copyButtonLabel": "COPY",
"copyButtonLabel": "Copy",
"@copyButtonLabel": {
"description": "The label for copy buttons and menu items."
},
"cutButtonLabel": "CUT",
"cutButtonLabel": "Cut",
"@cutButtonLabel": {
"description": "The label for cut buttons and menu items."
},
......@@ -133,12 +133,12 @@
"description": "The label for OK buttons and menu items."
},
"pasteButtonLabel": "PASTE",
"pasteButtonLabel": "Paste",
"@pasteButtonLabel": {
"description": "The label for paste buttons and menu items."
},
"selectAllButtonLabel": "SELECT ALL",
"selectAllButtonLabel": "Select all",
"@selectAllButtonLabel": {
"description": "The label for select-all buttons and menu items."
},
......
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