Delay focus trap unfocus until post-frame by gspencergoog · Pull Request #101847 · flutter/flutter

gspencergoog · 2022-04-13T17:47:51Z

Description

This changes the behavior of FocusTrap so that it doesn't unfocus if the focus has changed between the time it detected the click and the end of the frame. That way, widgets that are clicked on have a chance to capture the currently focused widget when they are clicked, instead of only seeing the focus scope, and it also avoids some focus churn if the focus changes during the build.

Tests

Added both a test for this change, and a generic test of FocusTrap, which was missing tests.

jonahwilliams

LGTM

…1847)

chunhtai · 2022-04-22T21:39:36Z

I ran into issue when running the following test that i think is related to this change

  testWidgets('mouse selection sends correct events', (WidgetTester tester) async {
    final FocusNode node = FocusNode();
    await tester.pumpWidget(
        MaterialApp(
          home: Scaffold(
            body: TextField(focusNode: node,),
          ),
        )
    );
    node.requestFocus();
    final TestGesture gesture = await tester.startGesture(const Offset(200.0, 200.0), kind: PointerDeviceKind.mouse);
    addTearDown(gesture.removePointer);
    await gesture.moveTo(const Offset(200.0, 100.0));
    await gesture.up();
  });

If you run the test with flutter test --platform=chrome, it will fail with the following

Pending timers:
Timer (duration: 0:00:00.000000, periodic: false), created:
../packages/fake_async/fake_async.dart.js 354:80                        __
../packages/fake_async/fake_async.dart.js 211:19                        [_createTimer]
../packages/fake_async/fake_async.dart.js 153:38                        <fn>
../dart-sdk/lib/async/zone.dart 1388:19                                 createTimer
../dart-sdk/lib/async/timer.dart 54:10                                  new
../dart-sdk/lib/async/timer.dart 110:9                                  run
../packages/flutter/src/scheduler/binding.dart.js 492:19                [_ensureEventLoopCallback]
../packages/flutter/src/scheduler/binding.dart.js 480:92                scheduleTask
../packages/flutter/src/widgets/title.dart.js 16737:43                  handleEvent
../packages/flutter/src/gestures/binding.dart.js 383:24                 dispatchEvent
../packages/flutter/src/rendering/layer.dart.js 5830:13                 dispatchEvent
../packages/flutter/src/gestures/binding.dart.js 356:14                 [_handlePointerEventImmediately]
../packages/flutter/src/gestures/binding.dart.js 329:43                 handlePointerEvent
../packages/flutter_test/src/deprecated.dart.js 1172:62                 <fn>
../packages/flutter_test/src/deprecated.dart.js 1180:9                  withPointerEventSource
../packages/flutter_test/src/deprecated.dart.js 1172:12                 handlePointerEventForSource
../packages/flutter_test/src/matchers.dart.js 4721:22                   <fn>
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54   runBody
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 123:5   _async
../packages/flutter_test/src/matchers.dart.js 4720:83                   <fn>
../dart-sdk/lib/async/zone.dart 1426:13                                 _rootRun
../dart-sdk/lib/async/zone.dart 1328:19                                 run
../packages/flutter_test/src/test_async_utils.dart.js 77:31             guard
../packages/flutter_test/src/matchers.dart.js 4720:46                   sendEventToBinding
../packages/flutter_test/src/test_pointer.dart.js 378:35                <fn>
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54   runBody
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 123:5   _async
../packages/flutter_test/src/test_pointer.dart.js 375:85                <fn>
../dart-sdk/lib/async/zone.dart 1426:13                                 _rootRun
../dart-sdk/lib/async/zone.dart 1328:19                                 run
../packages/flutter_test/src/test_async_utils.dart.js 77:31             guard
../packages/flutter_test/src/test_pointer.dart.js 375:48                down
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54   runBody
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 123:5   _async
../packages/flutter_test/src/test_pointer.dart.js 374:20                down
../packages/flutter_test/src/matchers.dart.js 3928:22                   startGesture
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 45:50   <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 179:98     <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 247:16     [_run]
../packages/stack_trace/src/stack_zone_specification.dart.js 179:80     <fn>
../dart-sdk/lib/async/zone.dart 1434:47                                 _rootRunUnary
../dart-sdk/lib/async/zone.dart 1335:19                                 runUnary
../dart-sdk/lib/async/future_impl.dart 147:18                           handleValue
../dart-sdk/lib/async/future_impl.dart 766:44                           handleValueCallback
../dart-sdk/lib/async/future_impl.dart 795:13                           _propagateToListeners
../dart-sdk/lib/async/future_impl.dart 566:5                            [_completeWithValue]
../dart-sdk/lib/async/future_impl.dart 639:7                            <fn>
../packages/fake_async/fake_async.dart.js 171:40                        flushMicrotasks
../packages/flutter_test/src/deprecated.dart.js 1767:19                 <fn>
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54   runBody
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 123:5   _async
../packages/flutter_test/src/deprecated.dart.js 1764:62                 <fn>
../dart-sdk/lib/async/future.dart 276:37                                <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 247:16     [_run]
../packages/stack_trace/src/stack_zone_specification.dart.js 170:71     <fn>
../dart-sdk/lib/async/zone.dart 1418:47                                 _rootRun
../dart-sdk/lib/async/zone.dart 1328:19                                 run
../dart-sdk/lib/async/zone.dart 1236:7                                  runGuarded
../dart-sdk/lib/async/zone.dart 1276:23                                 <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 247:16     [_run]
../packages/stack_trace/src/stack_zone_specification.dart.js 170:71     <fn>
../dart-sdk/lib/async/zone.dart 1426:13                                 _rootRun
../dart-sdk/lib/async/zone.dart 1328:19                                 run
../dart-sdk/lib/async/zone.dart 1236:7                                  runGuarded
../dart-sdk/lib/async/zone.dart 1276:23                                 callback
../dart-sdk/lib/async/schedule_microtask.dart 40:11                     _microtaskLoop
../dart-sdk/lib/async/schedule_microtask.dart 49:5                      _startMicrotaskLoop
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 166:15  <fn>

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test:
Assertion failed: file:///Users/chtai/git/flutter/packages/flutter_test/lib/src/binding.dart:1291:12
!timersPending
"A Timer is still pending even after the widget tree was disposed."

When the exception was thrown, this was the stack:
../dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 251:49  throw_
../dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 29:3    assertFailed
../packages/flutter_test/src/deprecated.dart.js 1793:33                          [_verifyInvariants]
../packages/flutter_test/src/deprecated.dart.js 1352:34                          _runTestBody
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 45:50            <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 179:98              <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 247:16              [_run]
../packages/stack_trace/src/stack_zone_specification.dart.js 179:80              <fn>
../dart-sdk/lib/async/zone.dart 1434:47                                          _rootRunUnary
../dart-sdk/lib/async/zone.dart 1335:19                                          runUnary
../dart-sdk/lib/async/future_impl.dart 147:18                                    handleValue
../dart-sdk/lib/async/future_impl.dart 766:44                                    handleValueCallback
../dart-sdk/lib/async/future_impl.dart 795:13                                    _propagateToListeners
../dart-sdk/lib/async/future_impl.dart 566:5                                     [_completeWithValue]
../dart-sdk/lib/async/future_impl.dart 639:7                                     <fn>
../packages/fake_async/fake_async.dart.js 171:40                                 flushMicrotasks
../packages/flutter_test/src/deprecated.dart.js 1767:19                          <fn>
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54            runBody
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 123:5            _async
../packages/flutter_test/src/deprecated.dart.js 1764:62                          <fn>
../dart-sdk/lib/async/future.dart 276:37                                         <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 247:16              [_run]
../packages/stack_trace/src/stack_zone_specification.dart.js 170:71              <fn>
../dart-sdk/lib/async/zone.dart 1418:47                                          _rootRun
../dart-sdk/lib/async/zone.dart 1328:19                                          run
../dart-sdk/lib/async/zone.dart 1236:7                                           runGuarded
../dart-sdk/lib/async/zone.dart 1276:23                                          <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 247:16              [_run]
../packages/stack_trace/src/stack_zone_specification.dart.js 170:71              <fn>
../dart-sdk/lib/async/zone.dart 1426:13                                          _rootRun
../dart-sdk/lib/async/zone.dart 1328:19                                          run
../dart-sdk/lib/async/zone.dart 1236:7                                           runGuarded
../dart-sdk/lib/async/zone.dart 1276:23                                          callback
../dart-sdk/lib/async/schedule_microtask.dart 40:11                              _microtaskLoop
../dart-sdk/lib/async/schedule_microtask.dart 49:5                               _startMicrotaskLoop
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 166:15           <fn>

The test description was:
  mouse selection sends correct events

flutter-dashboard bot added f: focus Focus traversal, gaining or losing focus f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. labels Apr 13, 2022

gspencergoog force-pushed the delay_focus_trap branch 3 times, most recently from 73293fa to 7d588b8 Compare April 15, 2022 18:57

gspencergoog marked this pull request as ready for review April 15, 2022 23:17

gspencergoog requested a review from jonahwilliams April 15, 2022 23:17

jonahwilliams approved these changes Apr 15, 2022

View reviewed changes

gspencergoog added 5 commits April 18, 2022 09:06

Delay focus trap unfocus until post-frame

52c89cc

Revert switch to scheduleTask

68ce4cc

Fix issues

d45b990

Add Tests

e98e7cf

Empty request to trigger CI again

a735e30

gspencergoog force-pushed the delay_focus_trap branch from 02a10c5 to a735e30 Compare April 18, 2022 16:06

Fix web test

7672058

gspencergoog added the waiting for tree to go green label Apr 18, 2022

fluttergithubbot merged commit 24e964d into flutter:master Apr 18, 2022

engine-flutter-autoroll mentioned this pull request Apr 18, 2022

Roll Flutter from 3c4d7a1aed65 to 3752fb7bd15e (11 revisions) flutter/plugins#5294

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 18, 2022

24e964d Delay focus trap unfocus until post-frame (flutter/flutter#10…

020f624

…1847)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 21, 2022

24e964d Delay focus trap unfocus until post-frame (flutter/flutter#10…

ce335d8

…1847)

chunhtai mentioned this pull request Apr 22, 2022

Pending Timer when hitting FocusTrap in testwidget #102410

Closed

gspencergoog deleted the delay_focus_trap branch October 7, 2022 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delay focus trap unfocus until post-frame#101847

Delay focus trap unfocus until post-frame#101847
fluttergithubbot merged 6 commits intoflutter:masterfrom
gspencergoog:delay_focus_trap

gspencergoog commented Apr 13, 2022 •

edited

Loading

Uh oh!

jonahwilliams left a comment

Uh oh!

chunhtai commented Apr 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

gspencergoog commented Apr 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Apr 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gspencergoog commented Apr 13, 2022 •

edited

Loading