[go: nahoru, domu]

Skip to content
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

Activate shortcuts based on NumLock state #145146

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Activate shortcuts based on NumLock state
  • Loading branch information
bleroux committed Mar 14, 2024
commit 1424fe04ad800d866ba8bd1407e4ff269317f6a0
35 changes: 34 additions & 1 deletion packages/flutter/lib/src/widgets/shortcuts.dart
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@ class KeySet<T extends KeyboardKey> {
}
}

/// Determines how the state of the NumLock key is used to accept a shortcut.
enum NumLockPolicy {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call this LockPolicy or LockState, so that it can be used with other kinds of locks (e.g. Scroll Lock, Shift Lock, Caps Lock), if we decide that we want to add them in the future. I don't really see a huge reason to add most of them at the moment, but there are some locks on other keyboards (e.g. the Hiragana/Katakana ひらがな key on a Japanese keyboard), that we might want to add later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion! And very interesting to learn about this japanese key 👍
Do you have a preference between LockPolicy and LockState?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted for 'LockState', it sounds great to me.

/// The NumLock key state is not used to determine [SingleActivator.accepts] result.
ignored,
/// The NumLock key must be locked to trigger the shortcut.
locked,
/// The NumLock key must be unlocked to trigger the shortcut.
unlocked,
}

/// An interface to define the keyboard key combination to trigger a shortcut.
///
/// [ShortcutActivator]s are used by [Shortcuts] widgets, and are mapped to
Expand Down Expand Up @@ -430,6 +440,7 @@ class SingleActivator with Diagnosticable, MenuSerializableShortcut implements S
this.shift = false,
this.alt = false,
this.meta = false,
this.numLockPolicy = NumLockPolicy.ignored,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can just be called numLock, to be consistent with the other arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i was reluctant because other fields are boolean but it is way more elegant.

this.includeRepeats = true,
}) : // The enumerated check with `identical` is cumbersome but the only way
// since const constructors can not call functions such as `==` or
Expand Down Expand Up @@ -505,6 +516,19 @@ class SingleActivator with Diagnosticable, MenuSerializableShortcut implements S
/// * [LogicalKeyboardKey.metaLeft], [LogicalKeyboardKey.metaRight].
final bool meta;

/// Whether the NumLock key state should be checked for [trigger] to activate
/// the shortcut.
///
/// It defaults to [NumLockPolicy.ignored], meaning the NumLock state is ignored
/// when the event is received in order to activate the shortcut.
/// If it's [NumLockPolicy.locked], then the NumLock key must be locked.
/// If it's [NumLockPolicy.unlocked], then the NumLock key must be unlocked.
///
/// See also:
///
/// * [LogicalKeyboardKey.numLock].
final NumLockPolicy numLockPolicy;

/// Whether this activator accepts repeat events of the [trigger] key.
///
/// If [includeRepeats] is true, the activator is checked on all
Expand All @@ -525,11 +549,20 @@ class SingleActivator with Diagnosticable, MenuSerializableShortcut implements S
&& meta == pressed.intersection(_metaSynonyms).isNotEmpty;
}

bool _shouldAcceptNumLock(HardwareKeyboard state) {
return switch (numLockPolicy) {
NumLockPolicy.ignored => true,
NumLockPolicy.locked => state.lockModesEnabled.contains(KeyboardLockMode.numLock),
NumLockPolicy.unlocked => !state.lockModesEnabled.contains(KeyboardLockMode.numLock),
};
}

@override
bool accepts(KeyEvent event, HardwareKeyboard state) {
return (event is KeyDownEvent || (includeRepeats && event is KeyRepeatEvent))
&& triggers.contains(event.logicalKey)
&& _shouldAcceptModifiers(state.logicalKeysPressed);
&& _shouldAcceptModifiers(state.logicalKeysPressed)
&& _shouldAcceptNumLock(state);
}

@override
Expand Down
107 changes: 106 additions & 1 deletion packages/flutter/test/widgets/shortcuts_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ void main() {

const SingleActivator singleActivator = SingleActivator(LogicalKeyboardKey.keyA, control: true);

await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft);
await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft);
await tester.sendKeyDownEvent(LogicalKeyboardKey.keyA);
expect(ShortcutActivator.isActivatedBy(singleActivator, events.last), isTrue);
await tester.sendKeyRepeatEvent(LogicalKeyboardKey.keyA);
Expand All @@ -497,6 +497,111 @@ void main() {
expect(ShortcutActivator.isActivatedBy(noRepeatSingleActivator, events.last), isFalse);
});

testWidgets('NumLockPolicy.locked works as expected', (WidgetTester tester) async {
// Collect some key events to use for testing.
final List<KeyEvent> events = <KeyEvent>[];
await tester.pumpWidget(
Focus(
autofocus: true,
onKeyEvent: (FocusNode node, KeyEvent event) {
events.add(event);
return KeyEventResult.ignored;
},
child: const SizedBox(),
),
);

const SingleActivator singleActivator = SingleActivator(LogicalKeyboardKey.numpad4, numLockPolicy: NumLockPolicy.locked);

// Lock NumLock.
await tester.sendKeyEvent(LogicalKeyboardKey.numLock);
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isTrue);

await tester.sendKeyDownEvent(LogicalKeyboardKey.numpad4);
expect(ShortcutActivator.isActivatedBy(singleActivator, events.last), isTrue);

await tester.sendKeyUpEvent(LogicalKeyboardKey.numpad4);

// Unlock NumLock.
await tester.sendKeyEvent(LogicalKeyboardKey.numLock);
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isFalse);

await tester.sendKeyDownEvent(LogicalKeyboardKey.numpad4);
expect(ShortcutActivator.isActivatedBy(singleActivator, events.last), isFalse);

await tester.sendKeyUpEvent(LogicalKeyboardKey.numpad4);
});

testWidgets('NumLockPolicy.unlocked works as expected', (WidgetTester tester) async {
// Collect some key events to use for testing.
final List<KeyEvent> events = <KeyEvent>[];
await tester.pumpWidget(
Focus(
autofocus: true,
onKeyEvent: (FocusNode node, KeyEvent event) {
events.add(event);
return KeyEventResult.ignored;
},
child: const SizedBox(),
),
);

const SingleActivator singleActivator = SingleActivator(LogicalKeyboardKey.numpad4, numLockPolicy: NumLockPolicy.unlocked);

// Lock NumLock.
await tester.sendKeyEvent(LogicalKeyboardKey.numLock);
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isTrue);

await tester.sendKeyDownEvent(LogicalKeyboardKey.numpad4);
expect(ShortcutActivator.isActivatedBy(singleActivator, events.last), isFalse);

await tester.sendKeyUpEvent(LogicalKeyboardKey.numpad4);

// Unlock NumLock.
await tester.sendKeyEvent(LogicalKeyboardKey.numLock);
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isFalse);

await tester.sendKeyDownEvent(LogicalKeyboardKey.numpad4);
expect(ShortcutActivator.isActivatedBy(singleActivator, events.last), isTrue);

await tester.sendKeyUpEvent(LogicalKeyboardKey.numpad4);
});

testWidgets('NumLockPolicy.ignored works as expected', (WidgetTester tester) async {
// Collect some key events to use for testing.
final List<KeyEvent> events = <KeyEvent>[];
await tester.pumpWidget(
Focus(
autofocus: true,
onKeyEvent: (FocusNode node, KeyEvent event) {
events.add(event);
return KeyEventResult.ignored;
},
child: const SizedBox(),
),
);

const SingleActivator singleActivator = SingleActivator(LogicalKeyboardKey.numpad4);

// Lock NumLock.
await tester.sendKeyEvent(LogicalKeyboardKey.numLock);
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isTrue);

await tester.sendKeyDownEvent(LogicalKeyboardKey.numpad4);
expect(ShortcutActivator.isActivatedBy(singleActivator, events.last), isTrue);

await tester.sendKeyUpEvent(LogicalKeyboardKey.numpad4);

// Unlock NumLock.
await tester.sendKeyEvent(LogicalKeyboardKey.numLock);
expect(HardwareKeyboard.instance.lockModesEnabled.contains(KeyboardLockMode.numLock), isFalse);

await tester.sendKeyDownEvent(LogicalKeyboardKey.numpad4);
expect(ShortcutActivator.isActivatedBy(singleActivator, events.last), isTrue);

await tester.sendKeyUpEvent(LogicalKeyboardKey.numpad4);
});

group('diagnostics.', () {
test('single key', () {
final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder();
Expand Down