Skip to content

Consider simplifying FocusTrap? #86972

Closed
Closed

Description

@Hixie

From: #82575

If we want this behaviour, why not just put a Focus widget around the route?

...but I'm not convinced we want this behaviour. It's not what happens on Windows and macOS as far as I can tell.

I think this is just something we want on Web, and on Web the behaviour is just that the root of the tree is itself focusable, no need to check whether a tap would have focused a node and all that.

cc @jonahwilliams

Activity

  1. added
    frameworkflutter/packages/flutter repository. See also f: labels.
    P2Important issues not at the top of the work list
    on Jul 23, 2021
  2. venkatd commented on Jul 31, 2021

    @venkatd

    We're working around this behavior in our app. We only want this to happen when certain inputs are focused (TextField and a few of our own custom controls).

    We wrap our app with this widget to disable the FocusTrap
    /// Flutter introduced a FocusTrap which is intended to remove focus for elements
    /// like TextFields. This is problematic because it causes focus to be lost in
    /// for certain Focus widgets. We use this widget to disable this behavior.
    class _DisableFocusTrapBehavior extends StatefulWidget {
      const _DisableFocusTrapBehavior({required this.child});
    
      final Widget child;
    
      @override
      State<StatefulWidget> createState() => _DisableFocusTrapBehaviorState();
    }
    
    class _DisableFocusTrapBehaviorState extends State<_DisableFocusTrapBehavior> {
      late FocusNode focusNode;
    
      @override
      void initState() {
        super.initState();
        focusNode =
            FocusManager.instance.primaryFocus ?? FocusManager.instance.rootScope;
        FocusManager.instance.addListener(_onFocusNodeChanged);
      }
    
      @override
      void dispose() {
        super.dispose();
        FocusManager.instance.removeListener(_onFocusNodeChanged);
      }
    
      @override
      Widget build(BuildContext context) {
        return FocusTrapArea(focusNode: focusNode, child: widget.child);
      }
    
      void _onFocusNodeChanged() {
        final newFocusNode =
            FocusManager.instance.primaryFocus ?? FocusManager.instance.rootScope;
        if (newFocusNode != focusNode) {
          setState(() {
            focusNode = newFocusNode;
          });
        }
      }
    }

    Right now this introduces in bugs in our app, where some custom controls accidentally lose focus.

    The ideal behavior would be some sort of GestureDetector-type widget like:

    return TapOutsideDetector(
        onTapOutside: (PointerEvent event, HitTestEntry entry) {
         // do some check to ensure `unfocus` is what we want to call
         focusNode.unfocus();
        }
        child: MyInputElement(focusNode: focusNode),
    );

    Right now the FocusTrap behavior is buried deep inside the ModalRoute of Flutter and is difficult to override.

    Ideally Flutter provides the building blocks to accomplish these different behaviors and we can compose these differently for each platform. As it stands, this is hardcoded:

      /// The focus dropping behavior is only present on desktop platforms
      /// and mobile browsers.
      bool get _shouldIgnoreEvents {
        switch (defaultTargetPlatform) {
          case TargetPlatform.android:
          case TargetPlatform.iOS:
            return !kIsWeb;
          case TargetPlatform.linux:
          case TargetPlatform.macOS:
          case TargetPlatform.windows:
          case TargetPlatform.fuchsia:
            return false;
        }
      }

    The behavior on what to do (can unfocus with scope disposition) is hardcoded as well.

    Stepping back, this direction feels like Flutter will become more monolithic.

    I urge the Flutter team to prioritize primitives and ease of composition over any platform-specific behavior. If they do that, the community can help compose these primitives to match each particular platform while the core Flutter team can prioritize the primitives that are harder for us to come up with.

    Our team has invested significantly in making Flutter web match the web more closely, but we have yet to contribute any of it back because the pieces we have improved are monolithic in the core Flutter framework.

    We might want to consider any switch statement over the platform a code smell. It makes composition harder and makes outside contributions to improve the fidelity of a particular platform harder.

  3. Hixie commented on Aug 1, 2021

    @Hixie
    ContributorAuthor

    (Please don't hesitate to file bugs on those other components you mention.)

  4. venkatd commented on Aug 2, 2021

    @venkatd

    @Hixie thanks, we do participate on Flutter issues and create our own when we can't find them. In this case, the widgts are our own custom controls. We we want to make use of the focus tree but don't want the same focus change behavior that is hardcoded with the FocusTrap implementation.

    Will be more diligent about posting issues since we sometimes workaround a limitation without posting the issue on GH :)

  5. jonahwilliams commented on Aug 2, 2021

    @jonahwilliams
    Contributor

    @venkatd without a more specific example it would be hard to suggest a fix.

    @Hixie Could you elaborate a bit? I can definitely click outside of a text field and remove focus. Without that sort of behavior, how do fire off page level keyboard shortcuts?

  6. Hixie commented on Aug 6, 2021

    @Hixie
    ContributorAuthor

    But are you removing focus, or are you moving focus to something else (like "the window" or some such)?

    For example, on macOS, enable keyboard focus (^F7), go to the System Preferences, go to the General page, then tab around so that one of the controls in the bottom pane is selected. Then, click the text field. Focus moves to the text field. Then click in an empty area of the bottom pane. Focus moves back to what you had selected in that focus scope.

    That said, macOS has some clearly quirky behaviour, e.g. tabbing forward then backward sometimes gets you to a different place, and double-clicking the window title seems to move the focus in a different way than single-clicking the window title (which doesn't move the focus if a text field is selected) and the "profile" thing at the top of the System Preferences pane seems to swallow tab focus entirely and...

    ...but the point is that the model that seems most accurate here is that clicking outside of a text field moves focus because the location you click takes the focus, not because the text field is monitoring clicks outside itself to lose the focus.

  7. jonahwilliams commented on Aug 6, 2021

    @jonahwilliams
    Contributor

    What is the difference between the window taking focus (or perhaps in flutter the route) versus the text field losing focus (and returning focus to the "scope" in flutter?

  8. Hixie commented on Aug 6, 2021

    @Hixie
    ContributorAuthor

    If we only ever take focus, we can remove FocusTrap entirely.

  9. jonahwilliams commented on Aug 6, 2021

    @jonahwilliams
    Contributor

    The biggest reason I didn't introduce a gesture detector / focus node to handle this is that the behavior I observe on the web is that focus is lost on keydown and not keyup, which implies that I could not use whether or not a tap gesture detector "won" a mouse tap to correctly take focus. I was also concerned that introducing a new tap detector would break too many tests since the mandatory delay to determine the winner could adjust test results - this might not be an issue since most tests do not interact via mouse pointers (for better or worse)

    If we're OK with that sort of discrepancy, I could certainly drop the focus trap

  10. Hixie commented on Aug 7, 2021

    @Hixie
    ContributorAuthor

    That's true of all focus changes though, no? Like, on Web, if I focus another field and then click-and-hold on this text field I'm typing into, the focus moves as soon as the mouse-down happens, long before it knows whether I'm clicking or dragging. The losing-focus-when-a-text-field-is-focused case doesn't seems special here.

  11. jpnurmi commented on Aug 7, 2021

    @jpnurmi
    Member

    I agree with @venkatd that this kind of functionality should not be hard-wired deep inside the framework. Even though the change passed Google's internal testing, this is a significant behavior change and likely to break some third-party apps. It would be safer if the new behavior was opt-in.

    As far as I can tell, in contrast to the web, desktop frameworks do generally not reset input focus when clicking outside input fields. However, there are cases, such as "transient" search fields, where input focus should reset when interacting outside. Thus, wouldn't it be better if the functionality was provided by composable elements?

    Moreover, if an app chooses to fill the entire background with a focus trap, general-purpose widgets should be allowed to reverse the behavior within their bounds. As a concrete example, tapping buttons on an in-app virtual keyboard should not reset input focus: #76072.

  12. venkatd commented on Aug 7, 2021

    @venkatd

    Taking a step back, the Flutter team could extend their philosophy of "aggressive compostability" to platform-specific behaviors.

    I propose two design guidelines:

    • Prefer composition over platform checks
    • Keep the necessary platform conditionals as top-level as possible. For example a MacOSTextField, WindowsTextField, UbuntuTextField, AndroidTextField, etc, could be composed of different lower level components. To make things easier, a PlatformTextField could delegate to the proper widget depending on the platform.

    This would have the following advantages:

    • The Flutter core team won't be spread thin trying to make the nuances of every platform match. The community can start helping out more.
    • Flutter devs won't be stuck with opinionated hardcoded behavior inside of the framework.
    • The codebase will scale as more platforms are added to Flutter.
  13. cedvdb commented on Aug 8, 2021

    @cedvdb
    Contributor

    On the flip side are you sure you want different behaviors by platform ?

    If an user is used to use an app on android / web and the same app does not act the same on another platform like windows wouldn't it throw him off ?

    In other words: Is it better to adhere to platform specific guidelines or have the same sensible behavior on every platform ?

    I don't think losing focus when clicking outside on windows would make any user wonder why. If anything people are mostly used to web therefor that could be the guiding guideline.

  14. 6 remaining items

  15. venkatd commented on Sep 10, 2021

    @venkatd

    Here are some use cases:

    • For some focusable widgets, we would like clicking outside to be interpreted as "I'm done, go back to what I was doing. This means the focus should revert to the previous place, not whatever focus node was clicked on.
    • Click-to-edit behavior (clicking on Text to edit it in place with a TextFIeld and clicking outside to save) would also mean clicking outside would revert focus to the previous place unless another focusable element is clicked on, not the element that is clicked on.
    • A rich text editor may have a toolbar. Clicking on those buttons should not cause focus to get lost because you would like to keep typing after bold/italic/underline is pressed
    • An autocomplete text field, if it loses focus, will automatically close. This is fine, but if we clicking on a suggestion would cause focus to be lost.

    There are many examples where a focused widget would want control over what happens when clicking outside of its bounds.

    We have a DisableFocusTrap widget we've implement in our app as a workaround until it is fixed/removed:

    disable_focus_trap.dart
    import 'package:flutter/widgets.dart';
    
    /// Flutter introduced a FocusTrap which is intended to remove focus for elements
    /// like TextFields. This is problematic because it causes focus to be lost in
    /// for certain Focus widgets. We use this widget to disable this behavior.
    ///
    /// Usage: wrap your entire application in this.
    ///
    /// Some use cases:
    ///   - Clicking out doesn't always mean we focus what was clicked on.
    ///     We may want to give up focus or focus to the next element up.
    ///   - We may not always want to lose focus at all. For example, an autocomplete
    ///     widget or a formatting toolbar for a rich text editor.
    ///   - This behavior can vary depending on the situation.
    ///
    /// Relevant issue:
    /// https://github.com/flutter/flutter/issues/86972
    class DisableFocusTrap extends StatefulWidget {
      const DisableFocusTrap({required this.child});
    
      final Widget child;
    
      @override
      State<StatefulWidget> createState() => _DisableFocusTrapState();
    }
    
    class _DisableFocusTrapState extends State<DisableFocusTrap> {
      late FocusNode focusNode;
    
      @override
      void initState() {
        super.initState();
        focusNode =
            FocusManager.instance.primaryFocus ?? FocusManager.instance.rootScope;
        FocusManager.instance.addListener(_onFocusNodeChanged);
      }
    
      @override
      void dispose() {
        super.dispose();
        FocusManager.instance.removeListener(_onFocusNodeChanged);
      }
    
      @override
      Widget build(BuildContext context) {
        return FocusTrapArea(focusNode: focusNode, child: widget.child);
      }
    
      void _onFocusNodeChanged() {
        final newFocusNode =
            FocusManager.instance.primaryFocus ?? FocusManager.instance.rootScope;
        if (newFocusNode != focusNode) {
          setState(() {
            focusNode = newFocusNode;
          });
        }
      }
    }
    
  16. jpnurmi commented on Mar 11, 2022

    @jpnurmi
    Member

    How about adding something like Focus(Node).canTrapFocus? Not sure if the name makes sense but you get the idea. :)

    ElevatedButton(
      focusNode: FocusNode(canTrapFocus: false),
      onPressed: () {},
      child: const Text('No trap'),
    )
    Focus(
      canTrapFocus: false,
      child: Container(
        width: 300,
        height: 300,
        color: Colors.green,
        child: const Text('No traps anywhere in this area'),
      ),
    ),
    patch
    diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart
    index 4c04ee679d..e9cfa447a3 100644
    --- a/packages/flutter/lib/src/widgets/focus_manager.dart
    +++ b/packages/flutter/lib/src/widgets/focus_manager.dart
    @@ -407,6 +407,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
        this.onKey,
        this.onKeyEvent,
        bool skipTraversal = false,
    +    this.canTrapFocus = true,
        bool canRequestFocus = true,
        bool descendantsAreFocusable = true,
        bool descendantsAreTraversable = true,
    @@ -507,6 +508,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
        }
      }
    
    +  bool canTrapFocus;
    +
      /// If false, will disable focus for all of this node's descendants.
      ///
      /// Defaults to true. Does not affect focusability of this node: for that,
    @@ -1152,6 +1155,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
        properties.add(DiagnosticsProperty<BuildContext>('context', context, defaultValue: null));
        properties.add(FlagProperty('descendantsAreFocusable', value: descendantsAreFocusable, ifFalse: 'DESCENDANTS UNFOCUSABLE', defaultValue: true));
        properties.add(FlagProperty('descendantsAreTraversable', value: descendantsAreTraversable, ifFalse: 'DESCENDANTS UNTRAVERSABLE', defaultValue: true));
    +    properties.add(FlagProperty('canTrapFocus', value: canTrapFocus, ifTrue: 'CAN TRAP', defaultValue: true));
        properties.add(FlagProperty('canRequestFocus', value: canRequestFocus, ifFalse: 'NOT FOCUSABLE', defaultValue: true));
        properties.add(FlagProperty('hasFocus', value: hasFocus && !hasPrimaryFocus, ifTrue: 'IN FOCUS PATH', defaultValue: false));
        properties.add(FlagProperty('hasPrimaryFocus', value: hasPrimaryFocus, ifTrue: 'PRIMARY FOCUS', defaultValue: false));
    @@ -1211,6 +1215,7 @@ class FocusScopeNode extends FocusNode {
        FocusOnKeyEventCallback? onKeyEvent,
        FocusOnKeyCallback? onKey,
        bool skipTraversal = false,
    +    bool canTrapFocus = true,
        bool canRequestFocus = true,
      })  : assert(skipTraversal != null),
            assert(canRequestFocus != null),
    @@ -1218,6 +1223,7 @@ class FocusScopeNode extends FocusNode {
              debugLabel: debugLabel,
              onKeyEvent: onKeyEvent,
              onKey: onKey,
    +          canTrapFocus: canTrapFocus,
              canRequestFocus: canRequestFocus,
              descendantsAreFocusable: true,
              skipTraversal: skipTraversal,
    diff --git a/packages/flutter/lib/src/widgets/focus_scope.dart b/packages/flutter/lib/src/widgets/focus_scope.dart
    index fa787348ad..db77bce08d 100644
    --- a/packages/flutter/lib/src/widgets/focus_scope.dart
    +++ b/packages/flutter/lib/src/widgets/focus_scope.dart
    @@ -8,6 +8,7 @@ import 'basic.dart';
    import 'focus_manager.dart';
    import 'framework.dart';
    import 'inherited_notifier.dart';
    +import 'routes.dart';
    
    /// A widget that manages a [FocusNode] to allow keyboard focus to be given
    /// to this widget and its descendants.
    @@ -121,6 +122,7 @@ class Focus extends StatefulWidget {
        this.onFocusChange,
        FocusOnKeyEventCallback? onKeyEvent,
        FocusOnKeyCallback? onKey,
    +    bool? canTrapFocus,
        bool? canRequestFocus,
        bool? skipTraversal,
        bool? descendantsAreFocusable,
    @@ -129,6 +131,7 @@ class Focus extends StatefulWidget {
        String? debugLabel,
      })  : _onKeyEvent = onKeyEvent,
            _onKey = onKey,
    +        _canTrapFocus = canTrapFocus,
            _canRequestFocus = canRequestFocus,
            _skipTraversal = skipTraversal,
            _descendantsAreFocusable = descendantsAreFocusable,
    @@ -232,6 +235,9 @@ class Focus extends StatefulWidget {
      FocusOnKeyCallback? get onKey => _onKey ?? focusNode?.onKey;
      final FocusOnKeyCallback? _onKey;
    
    +  bool get canTrapFocus => _canTrapFocus ?? focusNode?.canTrapFocus ?? true;
    +  final bool? _canTrapFocus;
    +
      /// {@template flutter.widgets.Focus.canRequestFocus}
      /// If true, this widget may request the primary focus.
      ///
    @@ -451,6 +457,7 @@ class Focus extends StatefulWidget {
        super.debugFillProperties(properties);
        properties.add(StringProperty('debugLabel', debugLabel, defaultValue: null));
        properties.add(FlagProperty('autofocus', value: autofocus, ifTrue: 'AUTOFOCUS', defaultValue: false));
    +    properties.add(FlagProperty('canTrapFocus', value: canTrapFocus, ifTrue: 'CAN TRAP', defaultValue: true));
        properties.add(FlagProperty('canRequestFocus', value: canRequestFocus, ifFalse: 'NOT FOCUSABLE', defaultValue: false));
        properties.add(FlagProperty('descendantsAreFocusable', value: descendantsAreFocusable, ifFalse: 'DESCENDANTS UNFOCUSABLE', defaultValue: true));
        properties.add(FlagProperty('descendantsAreTraversable', value: descendantsAreTraversable, ifFalse: 'DESCENDANTS UNTRAVERSABLE', defaultValue: true));
    @@ -487,6 +494,8 @@ class _FocusWithExternalFocusNode extends Focus {
      @override
      FocusOnKeyCallback? get onKey => focusNode!.onKey;
      @override
    +  bool get canTrapFocus => focusNode!.canTrapFocus;
    +  @override
      bool get canRequestFocus => focusNode!.canRequestFocus;
      @override
      bool get skipTraversal => focusNode!.skipTraversal;
    @@ -544,6 +553,7 @@ class _FocusState extends State<Focus> {
      FocusNode _createNode() {
        return FocusNode(
          debugLabel: widget.debugLabel,
    +      canTrapFocus: widget.canTrapFocus,
          canRequestFocus: widget.canRequestFocus,
          descendantsAreFocusable: widget.descendantsAreFocusable,
          descendantsAreTraversable: widget.descendantsAreTraversable,
    @@ -673,9 +683,12 @@ class _FocusState extends State<Focus> {
            child: widget.child,
          );
        }
    -    return _FocusMarker(
    -      node: focusNode,
    -      child: child,
    +    return FocusTrapArea(
    +      focusNode: focusNode,
    +      child: _FocusMarker(
    +        node: focusNode,
    +        child: child,
    +      ),
        );
      }
    }
    @@ -752,6 +765,7 @@ class FocusScope extends Focus {
        required Widget child,
        bool autofocus = false,
        ValueChanged<bool>? onFocusChange,
    +    bool? canTrapFocus,
        bool? canRequestFocus,
        bool? skipTraversal,
        FocusOnKeyEventCallback? onKeyEvent,
    @@ -765,6 +779,7 @@ class FocusScope extends Focus {
              focusNode: node,
              autofocus: autofocus,
              onFocusChange: onFocusChange,
    +          canTrapFocus: canTrapFocus,
              canRequestFocus: canRequestFocus,
              skipTraversal: skipTraversal,
              onKeyEvent: onKeyEvent,
    @@ -825,6 +840,8 @@ class _FocusScopeWithExternalFocusNode extends FocusScope {
      @override
      FocusOnKeyCallback? get onKey => focusNode!.onKey;
      @override
    +  bool get canTrapFocus => focusNode!.canTrapFocus;
    +  @override
      bool get canRequestFocus => focusNode!.canRequestFocus;
      @override
      bool get skipTraversal => focusNode!.skipTraversal;
    @@ -841,6 +858,7 @@ class _FocusScopeState extends _FocusState {
      FocusScopeNode _createNode() {
        return FocusScopeNode(
          debugLabel: widget.debugLabel,
    +      canTrapFocus: widget.canTrapFocus,
          canRequestFocus: widget.canRequestFocus,
          skipTraversal: widget.skipTraversal,
        );
    @@ -890,6 +908,7 @@ class ExcludeFocus extends StatelessWidget {
      const ExcludeFocus({
        Key? key,
        this.excluding = true,
    +    this.canTrapFocus = true,
        required this.child,
      })  : assert(excluding != null),
            assert(child != null),
    @@ -916,6 +935,8 @@ class ExcludeFocus extends StatelessWidget {
      ///   subtree.
      final bool excluding;
    
    +  final bool canTrapFocus;
    +
      /// The child widget of this [ExcludeFocus].
      ///
      /// {@macro flutter.widgets.ProxyWidget.child}
    @@ -924,6 +945,7 @@ class ExcludeFocus extends StatelessWidget {
      @override
      Widget build(BuildContext context) {
        return Focus(
    +      canTrapFocus: canTrapFocus,
          canRequestFocus: false,
          skipTraversal: true,
          includeSemantics: false,
    diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart
    index a239096643..322e2ef59e 100644
    --- a/packages/flutter/lib/src/widgets/routes.dart
    +++ b/packages/flutter/lib/src/widgets/routes.dart
    @@ -2189,7 +2189,7 @@ class _RenderFocusTrap extends RenderProxyBoxWithHitTestBehavior {
            hitCurrentFocus = true;
            break;
          }
    -      if (target is _RenderFocusTrapArea && target.focusNode == focusNode) {
    +      if (target is _RenderFocusTrapArea && (target.focusNode == focusNode || target.focusNode.canTrapFocus == false)) {
            hitCurrentFocus = true;
            break;
          }
  17. venkatd commented on Mar 12, 2022

    @venkatd

    @jpnurmi @justinmc

    I propose creating primitives and composing them to accomplish for this use case

    I have a proof of concept in our app if you have any specific questions about this. We have written a hack to disable focus trap and use the solution below inside our app.

    These primitives can be composed without making assumptions about app-specific or platform-specific behavior.

    • TapOutsideDetector: a widget to detect when tapping outside of the bounds of the widget
    • TapOutsideSurface: a widget to wrap your entire app in to detect when tapping outside of any registered TapOutsideDetectors on the page
    • TapOutsideUnfocuser: a light wrapper around TapOutsideDetector to cause a widget to unfocus when clicking outside of it.

    Some advantages I see:

    • We can compose on a per-widget and per-platform basis exactly how we want to handle tapping outside. Some these can come from community contributions.
    • Allows other focusable widgets to take advantage of this behavior. For example, we will see text-like widgets like zephyr and super_editor which may also want to hook into this behavior
    • The hard part--detecting the tap outside of a widget--has other use cases such as "click outside to save" and can be reused

    I don't think this is at odds with good defaults. The TextField, in its build method, could internally wrap the web platform in a TapOutsideUnfocuser with a way override this behavior. So the end-user would still get a "just works" experience.

    Code snippets below:

    tap_outside_detector.dart
    import 'package:flutter/gestures.dart';
    import 'package:flutter/rendering.dart';
    import 'package:flutter/widgets.dart';
    
    typedef TapOutsideCallback = void Function(BoxHitTestResult hitTestResult);
    
    /// Allows widgets to detect when tapping outside of the bounds of the widget.
    ///
    /// A common use case is allowing [TextField]s and other widgets to give up
    /// their focus when someone taps outside of them.
    ///
    /// Your app must be wrapped in a [TapOutsideSurface] to use this widget.
    ///
    /// Because widgets can only detect taps inside their bounds a root-level widget
    /// is necessary to detect outside taps.
    class TapOutsideDetector extends SingleChildRenderObjectWidget {
      TapOutsideDetector({
        Key? key,
        required Widget child,
        this.onTapOutside,
        this.enabled = true,
      }) : super(key: key, child: child);
    
      /// Called when [enabled] and a tap outside the bounds [child] widget
      /// Uses the nearest ancestor [TapOutsideSurface] to catch taps outside of [child]
      final void Function(BoxHitTestResult hitTestResult)? onTapOutside;
    
      /// Whether [onTapOutside] should be called
      final bool enabled;
    
      @override
      _TapOutsideDetectorRenderObject createRenderObject(BuildContext context) {
        final renderObject = _TapOutsideDetectorRenderObject(
          onTapOutside: onTapOutside,
          enabled: enabled,
        );
    
        final surfaceRenderObject = context
            .findAncestorRenderObjectOfType<_TapOutsideSurfaceRenderObject>();
    
        if (surfaceRenderObject == null) {
          throw TapOutsideSurfaceNotFoundError._(renderObject);
        }
    
        if (onTapOutside != null && enabled) {
          surfaceRenderObject.register(renderObject);
          renderObject.lastRegisteredSurface = surfaceRenderObject;
        }
    
        return renderObject;
      }
    
      @override
      void updateRenderObject(
          BuildContext context, _TapOutsideDetectorRenderObject renderObject) {
        final wasRegistered =
            renderObject.onTapOutside != null && renderObject.enabled;
        final shouldBeRegistered = onTapOutside != null && enabled;
    
        renderObject.onTapOutside = onTapOutside;
        renderObject.enabled = enabled;
    
        if (shouldBeRegistered == wasRegistered) {
          return;
        }
    
        /// Below we register or deregister the widget with the nearest tap outside surface
        final surfaceRenderObject = context
            .findAncestorRenderObjectOfType<_TapOutsideSurfaceRenderObject>();
    
        if (surfaceRenderObject == null) {
          throw TapOutsideSurfaceNotFoundError._(renderObject);
        }
    
        if (shouldBeRegistered && !wasRegistered) {
          surfaceRenderObject.register(renderObject);
          renderObject.lastRegisteredSurface = surfaceRenderObject;
        } else if (!shouldBeRegistered && wasRegistered) {
          surfaceRenderObject.unregister(renderObject);
          renderObject.lastRegisteredSurface = null;
        }
      }
    
      @override
      void didUnmountRenderObject(_TapOutsideDetectorRenderObject renderObject) {
        renderObject.lastRegisteredSurface?.unregister(renderObject);
        renderObject.lastRegisteredSurface = null;
      }
    }
    
    class _TapOutsideDetectorRenderObject extends RenderProxyBox {
      _TapOutsideDetectorRenderObject({
        required this.onTapOutside,
        required this.enabled,
      });
    
      TapOutsideCallback? onTapOutside;
      bool enabled;
    
      _TapOutsideSurfaceRenderObject? lastRegisteredSurface;
    }
    
    /// In order to use [TapOutsideDetector], you must wrap the entire application
    /// in a [TapOutsideSurface] which can detect tap outside events on behalf of
    /// [TapOutsideDetector]s.
    ///
    /// For performance reasons, widgets in Flutter can't respond hit tests
    /// outside of their bounds.
    class TapOutsideSurface extends SingleChildRenderObjectWidget {
      const TapOutsideSurface({
        required Widget child,
        Key? key,
      }) : super(child: child, key: key);
    
      @override
      RenderObject createRenderObject(BuildContext context) {
        return _TapOutsideSurfaceRenderObject();
      }
    
      @override
      void updateRenderObject(
          BuildContext context, _TapOutsideSurfaceRenderObject renderObject) {}
    }
    
    class _TapOutsideSurfaceRenderObject extends RenderProxyBoxWithHitTestBehavior {
      final cachedResults = Expando<BoxHitTestResult>();
      final _registeredDetectors = Set<_TapOutsideDetectorRenderObject>();
    
      void register(_TapOutsideDetectorRenderObject detector) {
        _registeredDetectors.add(detector);
      }
    
      void unregister(_TapOutsideDetectorRenderObject detector) {
        _registeredDetectors.remove(detector);
      }
    
      @override
      bool hitTest(BoxHitTestResult result, {required Offset position}) {
        if (!size.contains(position)) {
          return false;
        }
    
        final hitTarget =
            hitTestChildren(result, position: position) || hitTestSelf(position);
    
        if (hitTarget) {
          final entry = BoxHitTestEntry(this, position);
          cachedResults[entry] = result;
          result.add(entry);
        }
    
        return hitTarget;
      }
    
      @override
      void handleEvent(PointerEvent event, HitTestEntry entry) {
        // Return early because no detectors are registered
        if (_registeredDetectors.isEmpty) {
          return;
        }
    
        assert(debugHandleEvent(event, entry));
    
        if (event is! PointerDownEvent ||
            event.buttons != kPrimaryButton ||
            event.kind != PointerDeviceKind.mouse) {
          return;
        }
    
        final BoxHitTestResult? result = cachedResults[entry];
    
        if (result == null) return;
    
        final matchingDetectors =
            _matchingDetectors(_registeredDetectors, result.path);
    
        for (final detector in matchingDetectors) {
          assert(detector.enabled);
          detector.onTapOutside!.call(result);
        }
      }
    
      Iterable<_TapOutsideDetectorRenderObject> _matchingDetectors(
          Set<_TapOutsideDetectorRenderObject> detectors,
          Iterable<HitTestEntry> hitTestPath) {
        final hitDetectors = Set<HitTestTarget>();
        for (final entry in hitTestPath) {
          final target = entry.target;
          if (_registeredDetectors.contains(target)) {
            hitDetectors.add(target);
          }
        }
        return detectors.difference(hitDetectors);
      }
    }
    
    /// The error that will be thrown if [TapOutsideDetector] fails to find a [TapOutsideSurface].
    class TapOutsideSurfaceNotFoundError<_TapOutsideSurfaceRenderObject>
        extends Error {
      TapOutsideSurfaceNotFoundError._(this._detector);
    
      final _TapOutsideSurfaceRenderObject _detector;
    
      @override
      String toString() {
        return '''
    Error: Could not find a TapOutsideSurface ancestor above $_detector
    ''';
      }
    }
    tap_outside_unfocuser.dart
    import 'package:flutter/rendering.dart';
    import 'package:flutter/widgets.dart';
    
    import 'tap_outside_detector.dart';
    
    /// Unfocuses the widget with the primary focus when a tap occurs outside the
    /// bounds of [child]. Unfocus is only triggered if [child] is an ancestor of
    /// the widget with primary focus.
    class TapOutsideUnfocuser extends StatefulWidget {
      const TapOutsideUnfocuser({
        required this.child,
        this.shouldUnfocus = defaultShouldUnfocus,
      });
    
      final Widget child;
    
      /// Given a hit test result, whether to actually unfocus
      /// By default, an unfocus isn't triggered when a TextField (RenderEditable) is clicked on
      /// because the focus system will bring focus to it.
      final bool Function(BoxHitTestResult result) shouldUnfocus;
    
      @override
      State<TapOutsideUnfocuser> createState() => _TapOutsideUnfocuserState();
    }
    
    class _TapOutsideUnfocuserState extends State<TapOutsideUnfocuser> {
      late final FocusNode focusNode;
      bool focusNodeHasFocus = false;
    
      @override
      initState() {
        super.initState();
    
        focusNode = FocusNode(
          debugLabel: 'TapOutsideUnfocuser',
          canRequestFocus: false,
          skipTraversal: true,
        );
        focusNodeHasFocus = focusNode.hasFocus;
    
        focusNode.addListener(_onFocusMaybeChanged);
      }
    
      @override
      Widget build(BuildContext context) {
        return Focus(
          focusNode: focusNode,
          child: TapOutsideDetector(
            enabled: focusNodeHasFocus,
            onTapOutside: (BoxHitTestResult result) {
              if (widget.shouldUnfocus(result)) {
                FocusManager.instance.primaryFocus?.unfocus();
              }
            },
            child: widget.child,
          ),
        );
      }
    
      void _onFocusMaybeChanged() {
        if (focusNodeHasFocus != focusNode.hasFocus) {
          setState(() {
            focusNodeHasFocus = focusNode.hasFocus;
          });
        }
      }
    
      @override
      void dispose() {
        super.dispose();
        focusNode.dispose();
      }
    }
    
    /// Default implementation of whether to unfocus for [TapOutsideUnfocuser]
    ///
    /// If we click on a TextField or something that is wrapped in [IgnoreUnfocuserRenderBox]
    /// we prevent the unfocus because it would be redundant.
    bool defaultShouldUnfocus(BoxHitTestResult result) {
      for (final e in result.path) {
        if (e.target is RenderEditable || e.target is IgnoreUnfocuserRenderBox) {
          return false;
        }
      }
      return true;
    }
    
    /// Widgets besides TextField that you don't want to trigger an unfocus for
    /// can be wrapped with [IgnoreUnfocuser]
    class IgnoreUnfocuser extends SingleChildRenderObjectWidget {
      const IgnoreUnfocuser({required this.child}) : super(child: child);
    
      final Widget child;
    
      @override
      IgnoreUnfocuserRenderBox createRenderObject(BuildContext context) =>
          IgnoreUnfocuserRenderBox();
    }
    
    class IgnoreUnfocuserRenderBox extends RenderPointerListener {}
    
  18. Hixie commented on Mar 16, 2022

    @Hixie
    ContributorAuthor

    I don't understand why we need anything other than Focus at the root. What does that not solve?

  19. venkatd commented on Mar 23, 2022

    @venkatd

    I don't understand why we need anything other than Focus at the root. What does that not solve?

    I'd also like to know why a Focus node at the root as a catch-all doesn't solve the issues in #82515 and #32433

    Would love to see a list of use cases FocusTrap is trying to accomplish.

    In its current state, I'd rather have FocusTrap not exist in the framework. It has caused problems for us by calling unfocus at times we don't want it to. So we've had to implement hacks to work around it.

  20. justinmc commented on Mar 29, 2022

    @justinmc
    Contributor

    Here's my attempt at just using a Focus widget at the root: #99434

    I'm seeing tons of test failures because it steals gestures from drags and things like that. I think I would need some kind of special gesture detector to get it to work, if it's possible.

  21. canastro commented on May 16, 2022

    @canastro
    Contributor

    I'm facing another issue due to the fact that FocusTrap is buried deep inside the ModalRoute. The gist of it is that I can't have a FocusTrapArea with focusNodes from a different route.

    In my application, I have an input that should be able to compose text by clicking on other widgets. This has been working fine with FocusTrapArea. But, when this input is rendered in a different PageRoute (eg. a drawer-like side panel), it's no longer able to keep focus when a widget from the base ModelScopeRoute is clicked.

    Just for a quick check, I tried to remove the FocusTrap at _ModalScope and everything worked as I was expecting.

    @justinmc should I open a separate issue for this?

    In case you want more details on the specific use case, expand this section
    Screen.Recording.2022-05-16.at.15.59.04.mov
    1. first see a case where the FocusTrapArea works perfectly fine, as the Textfield and the Table are within the same route.
    2. the a similar input is shown on the side-panel, which is a different PageRoute, when tapping the Table the focus is lost
    3. finally, I click on a button which also has a FocusTrapArea, but its in the same route as the input, and the focus is kept as expected.
  22. justinmc commented on May 16, 2022

    @justinmc
    Contributor

    @canastro I pinged you on Discord, but let's keep this in this issue for now. I hope that a solution to this issue would solve your problem as well. Thanks for adding a clear example.

  23. gspencergoog commented on Jul 7, 2022

    @gspencergoog
    Contributor

    @venkatd

    I propose creating primitives and composing them to accomplish for this use case

    I have a proof of concept in our app if you have any specific questions about this. We have written a hack to disable focus trap and use the solution below inside our app.

    These primitives can be composed without making assumptions about app-specific or platform-specific behavior.

    • TapOutsideDetector: a widget to detect when tapping outside of the bounds of the widget
    • TapOutsideSurface: a widget to wrap your entire app in to detect when tapping outside of any registered TapOutsideDetectors on the page
    • TapOutsideUnfocuser: a light wrapper around TapOutsideDetector to cause a widget to unfocus when clicking outside of it.

    I like this idea. When I was exploring a solution for a similar problem for handling tapping outside of menus, I had a similar answer, although mine wasn't as nice as yours, since it used inherited widgets to find the thing to register with.

    I also realized that the same detector could be used to detect a click inside of a collection of render boxes (I mean, if the tap isn't outside, then it's inside, right?), which might be useful if you want to know that without participating in the gesture arena.

    I'm going to try coding up something similar to this, remove FocusTrap, and see if that addresses the problem.

    One thing that it means is that you don't get the "unfocus" behavior by default: you have to wrap each widget that wants to do something in response to a tap outside in a TapOutsideDetector. I think that's a feature, not a bug, but it might be inconvenient in some situations (and will cause breakage for people who are relying on it).

  24. gspencergoog commented on Jul 7, 2022

    @gspencergoog
    Contributor

    Here's my attempt at a solution to this problem: #107262

  25. gspencergoog commented on Jul 29, 2022

    @gspencergoog
    Contributor

    This has been fixed via #107262, which is heavily based on @venkatd 's implementation, with the addition of a grouping mechanism that allows grouped render objects to behave as one when it comes to tapping outside.

    You can now override the "tap outside" behavior of text fields by setting the onClickOutside callback to do something different. By default, it will do this:

      void _defaultOnTapOutside(PointerDownEvent event) {
        /// The focus dropping behavior is only present on desktop platforms
        /// and mobile browsers.
        switch (defaultTargetPlatform) {
          case TargetPlatform.android:
          case TargetPlatform.iOS:
          case TargetPlatform.fuchsia:
          // On mobile platforms, we don't unfocus on touch events unless they're
          // in the web browser, but we do unfocus for all other kinds of events.
            switch (event.kind) {
              case ui.PointerDeviceKind.touch:
                if (kIsWeb) {
                  widget.focusNode.unfocus();
                }
                break;
              case ui.PointerDeviceKind.mouse:
              case ui.PointerDeviceKind.stylus:
              case ui.PointerDeviceKind.invertedStylus:
              case ui.PointerDeviceKind.unknown:
                widget.focusNode.unfocus();
                break;
              case ui.PointerDeviceKind.trackpad:
                throw UnimplementedError('Unexpected pointer down event for trackpad');
            }
            break;
          case TargetPlatform.linux:
          case TargetPlatform.macOS:
          case TargetPlatform.windows:
            widget.focusNode.unfocus();
            break;
        }
      }
  26. github-actions commented on Aug 12, 2022

    @github-actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

  27. locked as resolved and limited conversation to collaborators on Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Important issues not at the top of the work listf: focusFocus traversal, gaining or losing focusframeworkflutter/packages/flutter repository. See also f: labels.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions