Skip to content

Commit

Permalink
A bunch of cleanups and a missing ShortcutRegistar in WidgetsApp
Browse files Browse the repository at this point in the history
…(#104560)

A bunch of random cleanup things I found while doing MenuBar development.

Changes an if test to an assert in binding.dart, since the if should always be true.
Adds the default ShortcutRegistrar that should have been in the ShortcutRegistry PR.
Moves a debug message in the FocusManager to print the result after the focus change instead of before.
Reorders the test parameters in theme_data_test.dart to match the order of the theme data fields everywhere else.
  • Loading branch information
gspencergoog authored May 25, 2022
1 parent 19d69a9 commit 0a417c3
Show file tree
Hide file tree
Showing 7 changed files with 295 additions and 182 deletions.
6 changes: 3 additions & 3 deletions packages/flutter/lib/src/gestures/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,9 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H
@pragma('vm:notify-debugger-on-exception')
void dispatchEvent(PointerEvent event, HitTestResult? hitTestResult) {
assert(!locked);
// No hit test information implies that this is a [PointerHoverEvent],
// [PointerAddedEvent], or [PointerRemovedEvent]. These events are specially
// routed here; other events will be routed through the `handleEvent` below.
// No hit test information implies that this is a [PointerAddedEvent] or
// [PointerRemovedEvent]. These events are specially routed here; other
// events will be routed through the `handleEvent` below.
if (hitTestResult == null) {
assert(event is PointerAddedEvent || event is PointerRemovedEvent);
try {
Expand Down
4 changes: 3 additions & 1 deletion packages/flutter/lib/src/widgets/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1730,7 +1730,9 @@ class _WidgetsAppState extends State<WidgetsApp> with WidgetsBindingObserver {
actions: widget.actions ?? WidgetsApp.defaultActions,
child: FocusTraversalGroup(
policy: ReadingOrderTraversalPolicy(),
child: child,
child: ShortcutRegistrar(
child: child,
),
),
),
),
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/lib/src/widgets/focus_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1798,10 +1798,10 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier {
_dirtyNodes.add(_primaryFocus!);
}
}
assert(_focusDebug('Notifying ${_dirtyNodes.length} dirty nodes:', _dirtyNodes.toList().map<String>((FocusNode node) => node.toString())));
for (final FocusNode node in _dirtyNodes) {
node._notify();
}
assert(_focusDebug('Notified ${_dirtyNodes.length} dirty nodes:', _dirtyNodes.toList().map<String>((FocusNode node) => node.toString())));
_dirtyNodes.clear();
if (previousFocus != _primaryFocus) {
notifyListeners();
Expand Down
7 changes: 7 additions & 0 deletions packages/flutter/test/material/debug_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,13 @@ void main() {
' Localizations\n'
' MediaQuery\n'
' _MediaQueryFromWindow\n'
' _ShortcutRegistrarMarker\n'
' _ShortcutsMarker\n'
' Semantics\n'
' _FocusMarker\n'
' Focus\n'
' Shortcuts\n'
' ShortcutRegistrar\n'
' _FocusMarker\n'
' Focus\n'
' _FocusTraversalGroupMarker\n'
Expand Down
411 changes: 235 additions & 176 deletions packages/flutter/test/material/theme_data_test.dart

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/flutter/test/widgets/focus_manager_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ void main() {
final String messagesStr = messages.toString();
expect(messagesStr.split('\n').length, equals(58));
expect(messagesStr, contains(RegExp(r' └─Child 1: FocusScopeNode#[a-f0-9]{5}\(parent1 \[PRIMARY FOCUS\]\)')));
expect(messagesStr, contains('FOCUS: Notifying 2 dirty nodes'));
expect(messagesStr, contains('FOCUS: Notified 2 dirty nodes'));
expect(messagesStr, contains(RegExp(r'FOCUS: Scheduling update, current focus is null, next focus will be FocusScopeNode#.*parent1')));
});
}
45 changes: 45 additions & 0 deletions packages/flutter/test/widgets/shortcuts_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,51 @@ void main() {
expect(invokedB, equals(1));
});

testWidgets('MaterialApp has a ShortcutRegistrar listening', (WidgetTester tester) async {
int invokedA = 0;
int invokedB = 0;
await tester.pumpWidget(
MaterialApp(
home: TestCallbackRegistration(
shortcuts: <ShortcutActivator, Intent>{
const SingleActivator(LogicalKeyboardKey.keyA): VoidCallbackIntent(() {
invokedA += 1;
}),
const SingleActivator(LogicalKeyboardKey.keyB): VoidCallbackIntent(() {
invokedB += 1;
}),
},
child: Actions(
actions: <Type, Action<Intent>>{
VoidCallbackIntent: VoidCallbackAction(),
},
child: const Focus(
autofocus: true,
child: Placeholder(),
),
),
),
),
);
await tester.pump();

await tester.sendKeyDownEvent(LogicalKeyboardKey.keyA);
await tester.pump();
expect(invokedA, equals(1));
expect(invokedB, equals(0));
await tester.sendKeyUpEvent(LogicalKeyboardKey.keyA);
expect(invokedA, equals(1));
expect(invokedB, equals(0));
invokedA = 0;
invokedB = 0;
await tester.sendKeyDownEvent(LogicalKeyboardKey.keyB);
expect(invokedA, equals(0));
expect(invokedB, equals(1));
await tester.sendKeyUpEvent(LogicalKeyboardKey.keyB);
expect(invokedA, equals(0));
expect(invokedB, equals(1));
});

testWidgets("doesn't override text field shortcuts", (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(
Expand Down

0 comments on commit 0a417c3

Please sign in to comment.