Skip to content

Change Focus.unfocus to take a disposition for where the focus should go. - #50831

Merged
gspencergoog merged 8 commits into
flutter:masterfrom
gspencergoog:unfocus_disposition
Feb 27, 2020
Merged

gspencergoog merged 8 commits into
flutter:masterfrom
gspencergoog:unfocus_disposition

Conversation

@gspencergoog

@gspencergoog gspencergoog commented Feb 14, 2020 •

Copy link
Copy Markdown
Contributor

Description

When Focus.unfocus is called, the caller usually just thinks about wanting to remove focus from the node, but really, unfocus is a request to automatically pass the focus to another (hopefully useful) node.

This PR removes the focusPrevious flag from unfocus, and replaces it with a disposition enum that indicates where the focus should go from here.

The other value of the UnfocusDisposition enum is UnfocusDisposition.scope.

UnfocusDisposition.previouslyFocusedChild is closest to what focusPrevious used to do: focus the nearest enclosing scope and use its focusedChild field to walk down the tree, finding the leaf focusedChild. This PR modifies it slightly so that it walks up to the nearest focusable enclosing scope before trying to focus the children. This change addresses #48903

A new mode: UnfocusDisposition.scope will focus the nearest focusable enclosing scope of this node without trying to use the FocusScopeNode.focusedChild value to descend to the leaf focused child. This is useful as a default for both text field finalization and for what happens when canRequestFocus is set to false. It allows the scope to stay focused so that nextFocus/previousFocus still work as expected, but removes the focus from primary focus.

In addition to those changes, unfocus called on a FocuScope that wasn't the primary focus used to unfocus the primary focus instead. I removed that behavior, since it was buggy: if the primary focus was inside of a child scope, and you called unfocus on the parent scope, then the child scope could have focused another of its children instead, leaving the scope that you called unfocus on with hasFocus returning true still. If you want to remove the focus from the primary focus instead of the scope, that's easy enough to do: just call primaryFocus.unfocus().

Related Issues

Tests

Breaking Change

  • No, this is not a breaking change, as the focusPrevious argument doesn't appear in any customer tests.

@fluttergithubbot fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Feb 14, 2020
@gspencergoog

Copy link
Copy Markdown
Contributor Author

cc @chunhtai, this may supersede your change #50522

Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
@goderbauer

Copy link
Copy Markdown
Member

Cirrus seems unhappy.

@gspencergoog gspencergoog mentioned this pull request Feb 19, 2020
1 task done
@Piinks Piinks added f: focus Focus traversal, gaining or losing focus a: desktop Running on desktop f: routes Navigator, Router, and related APIs. labels Feb 19, 2020
@gspencergoog
gspencergoog force-pushed the unfocus_disposition branch 3 times, most recently from e610b7d to 26a0baf Compare February 20, 2020 23:51
@gspencergoog

Copy link
Copy Markdown
Contributor Author

@goderbauer, OK Cirrus should be happy again. PTAL.

@gspencergoog
gspencergoog force-pushed the unfocus_disposition branch 3 times, most recently from de7cbbd to 01bee7b Compare February 24, 2020 18:04
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/test/widgets/editable_text_test.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
@gspencergoog
gspencergoog force-pushed the unfocus_disposition branch 3 times, most recently from bc414d8 to 44bfe73 Compare February 25, 2020 22:25
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/lib/src/widgets/focus_manager.dart Outdated
Comment thread packages/flutter/test/widgets/focus_manager_test.dart Outdated

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@gspencergoog
gspencergoog merged commit d422656 into flutter:master Feb 27, 2020
@gspencergoog
gspencergoog deleted the unfocus_disposition branch February 27, 2020 02:04
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: desktop Running on desktop f: focus Focus traversal, gaining or losing focus f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keyboard focus is lost when popping multiple screens with Navigator.popuntil

6 participants