Skip to content

Commit

Permalink
Fix Linux numpad shortcuts on web (#148988)
Browse files Browse the repository at this point in the history
## Description

This PRs fixes a Web issue on Linux related to numpad keys.
In #145464, I introduced numpad shortcuts for Linux. These shortcuts work well on a desktop Linux application but they broke the Linux+Web numpad logic.

When I added these shortcuts, I expected them to not be active on Web (because I knew that on Web, those shortcuts are handled by the browser). But there is a trick: text editing shortcuts are still defined on Web but they are disabled at the editable text level so one can use them in components that are not `EditableText` (see #103377).
In this PR, I used the same approach than for other text editing shortcuts: when on web associate those shortcuts to the `DoNothingAndStopPropagationTextIntent` intent.

## Related Issue

Fixes #148447.

## Tests

Updates 2 tests.
Adds 2 tests.
  • Loading branch information
bleroux committed May 25, 2024
1 parent cb26a01 commit 0f3882e
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,20 @@ class DefaultTextEditingShortcuts extends StatelessWidget {

Map<ShortcutActivator, Intent>? _getDisablingShortcut() {
if (kIsWeb) {
return _webDisablingTextShortcuts;
switch (defaultTargetPlatform) {
case TargetPlatform.linux:
return <ShortcutActivator, Intent>{
..._webDisablingTextShortcuts,
for (final ShortcutActivator activator in _linuxNumpadShortcuts.keys)
activator as SingleActivator: const DoNothingAndStopPropagationTextIntent(),
};
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.windows:
case TargetPlatform.iOS:
case TargetPlatform.macOS:
return _webDisablingTextShortcuts;
}
}
switch (defaultTargetPlatform) {
case TargetPlatform.android:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,8 @@ void main() {
}, variant: macOSOnly);
}, skip: kIsWeb); // [intended] specific tests target non-web.

group('Linux does accept numpad shortcuts', () {
testWidgets('when numlock is locked', (WidgetTester tester) async {
group('Linux numpad shortcuts', () {
testWidgets('are triggered when numlock is locked', (WidgetTester tester) async {
final FocusNode editable = FocusNode();
addTearDown(editable.dispose);
final FocusNode spy = FocusNode();
Expand Down Expand Up @@ -577,7 +577,7 @@ void main() {
expect((state.lastIntent! as DeleteToNextWordBoundaryIntent).forward, true);
}, variant: TargetPlatformVariant.only(TargetPlatform.linux));

testWidgets('when numlock is unlocked', (WidgetTester tester) async {
testWidgets('are triggered when numlock is unlocked', (WidgetTester tester) async {
final FocusNode editable = FocusNode();
addTearDown(editable.dispose);
final FocusNode spy = FocusNode();
Expand Down Expand Up @@ -664,7 +664,79 @@ void main() {
expect(state.lastIntent, isA<DeleteToNextWordBoundaryIntent>());
expect((state.lastIntent! as DeleteToNextWordBoundaryIntent).forward, true);
}, variant: TargetPlatformVariant.only(TargetPlatform.linux));
}, skip: kIsWeb); // [intended] specific tests target non-web.

testWidgets('update the editable text content when triggered on non-web', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode();
addTearDown(focusNode.dispose);
final TextEditingController controller = TextEditingController(text: 'Flutter');
addTearDown(controller.dispose);

await tester.pumpWidget(MaterialApp(
home: Align(
alignment: Alignment.topLeft,
child: EditableText(
controller: controller,
autofocus: true,
focusNode: focusNode,
style: const TextStyle(fontSize: 10.0),
cursorColor: Colors.blue,
backgroundCursorColor: Colors.grey,
),
),
));

// Verify that NumLock is unlocked.
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isFalse);

await tester.enterText(find.byType(EditableText), 'Flutter');
expect(controller.selection.end, 7);

await sendKeyCombination(tester, const SingleActivator(LogicalKeyboardKey.numpad4));
// Verify the cursor moved to the left (numpad4).
expect(controller.selection.end, 6);
},
variant: TargetPlatformVariant.only(TargetPlatform.linux),
skip: kIsWeb, // [intended] Non-web test.
);

testWidgets('do not update the editable text content when triggered on web', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode();
addTearDown(focusNode.dispose);
final TextEditingController controller = TextEditingController(text: 'Flutter');
addTearDown(controller.dispose);

await tester.pumpWidget(MaterialApp(
home: Align(
alignment: Alignment.topLeft,
child: EditableText(
controller: controller,
autofocus: true,
focusNode: focusNode,
style: const TextStyle(fontSize: 10.0),
cursorColor: Colors.blue,
backgroundCursorColor: Colors.grey,
),
),
));

// Verify that NumLock is unlocked.
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isFalse);

await tester.enterText(find.byType(EditableText), 'Flutter');
expect(controller.selection.end, 7);

await sendKeyCombination(tester, const SingleActivator(LogicalKeyboardKey.numpad4));
// On web, the editable text would have been updated by the browser.
// In the flutter test environment, the browser logic is not called
// so the editable content is not updated when a shortcut is triggered.
// This is the intended result, this test is checking that numpad shortcuts
// have no effect on the web (their intent is set to DoNothingAndStopPropagationTextIntent).
expect(controller.selection.end, 7);
},
variant: TargetPlatformVariant.only(TargetPlatform.linux),
skip: !kIsWeb, // [intended] Web only.
);
});
}

class ActionSpy extends StatefulWidget {
Expand Down

0 comments on commit 0f3882e

Please sign in to comment.