-
Notifications
You must be signed in to change notification settings - Fork 29.7k
DefaultTextEditingShortcuts should use meta-based shortcut for iOS #103077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits 👍. Thanks for catching this.
Edit: Looks like there is a real test failure on a web test.
| expect(MediaQuery.of(capturedContext), isNotNull); | ||
| }); | ||
|
|
||
| testWidgets('WidgetsApp provide meta based shortcut for iOS', (WidgetTester tester) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"provide" => "provides"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"shortcut" => "shortcuts"
| expect(selectAllSpy.invoked, isTrue); | ||
| expect(copySpy.invoked, isTrue); | ||
| expect(pasteSpy.invoked, isTrue); | ||
| }, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This should work identically for macOS, right? Should we just make this test about both platforms or is that misleading?
| break; | ||
|
|
||
| // Mac expands by line. | ||
| // Mac and iOS expands by line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In these comments that you changed here and below, the verb should be changed now that the subject is plural. So "expands" => "expand" in this case, etc.
fixes #103073
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.