Automatic focus highlight mode for FocusManager by gspencergoog · Pull Request #37825 · flutter/flutter

gspencergoog · 2019-08-08T00:17:36Z

Description

This adds a FocusHighlightMode to the FocusManager that switches based on the type of input that has recently been received. The initial value is based on the platform, but is updated as soon as user input is received. There is also a FocusHighlightStrategy enum so that the developer can change the strategy to a fixed value if needed.

The default is to automatically detect the mode based on the last type of user input. If they use a mouse or keyboard, it shows the focus highlights. If they use a touch interface, then the highlights disappear. This is consistent with the way that Android and Chrome work. The controls still receive focus, only the display of the highlight changes.

Text fields show the focus highlight regardless of the focus highlight mode.

Tests

Added tests for both the FocusManager and InkWell.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.
I checked all the boxes!

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

No, this is not a breaking change.

HansMuller

LGTM just some small-stuff feedback

packages/flutter/lib/src/widgets/focus_manager.dart

HansMuller · 2019-08-13T19:45:30Z

packages/flutter/lib/src/widgets/focus_manager.dart

The TextField reference is little confusing. TextField doesn't always bring up a software keyboard because desktop etc.

OK, updated.

HansMuller · 2019-08-13T19:52:45Z

packages/flutter/lib/src/widgets/focus_manager.dart

Maybe use a ValueNotifier<FocusHighlightMode> instance here? Might eliminate some of the boilerplate.

No, I didn't do that because the ValueNotifier only provides for a VoidCallback, and I wanted to send the value with the callback. If I used ValueNotifier, I could wrap the ValueChanged<FocusHighlightMode> in a void callback closure, and pass the value of _highlightMode when the void callback is called, but then I couldn't remove the closure later without keeping it in a private list, which is more error-prone, and needs almost as much code as the boilerplate.

HansMuller · 2019-08-13T19:56:47Z

packages/flutter/lib/src/widgets/focus_manager.dart

Where do we remove this "global route"?

We don't, because once we construct the FocusManager, we never release it: it's a singleton attached to the binding.

This reverts commit a11d731 because of a regression in flutter_gallery_ios32__transition_perf and 90th_percentile_frame_build_time_millis. See issue flutter#38860.

…8866) This reverts commit a11d731 because of a regression in flutter_gallery_ios32__transition_perf's 90th_percentile_frame_build_time_millis. Fixes #38860.

xu-baolin · 2020-11-12T03:04:16Z

packages/flutter/lib/src/material/ink_well.dart

+    bool showFocus;
+    switch (WidgetsBinding.instance.focusManager.highlightMode) {
+      case FocusHighlightMode.touch:
+        showFocus = false;


@gspencergoog Hi,
This change lets the touch phone disabled the focusColor without API documentations. I think we should improve the documentations.

flutter/packages/flutter/lib/src/material/ink_well.dart

Lines 439 to 450 in 15d2d8a

/// The color of the ink response when the parent widget is focused. If this

/// property is null then the focus color of the theme,

/// [ThemeData.focusColor], will be used.

///

/// See also:

///

/// * [highlightShape], the shape of the focus, hover, and pressed

/// highlights.

/// * [hoverColor], the color of the hover highlight.

/// * [splashColor], the color of the splash.

/// * [splashFactory], which defines the appearance of the splash.

final Color? focusColor;

Sometimes it is useful that display the focus color on touch devices, such as #70294

Based on this, I want to add a property of InkWell like alwaysShowFocusColor to optional this behavior.

case FocusHighlightMode.touch: showFocus = alwaysShowFocusColor ? _shouldShowFocus : false; break;

Waiting for your thoughts! thanks very much.

gspencergoog requested a review from HansMuller August 8, 2019 00:17

fluttergithubbot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Aug 8, 2019

googlebot added the cla: yes label Aug 8, 2019

HansMuller approved these changes Aug 13, 2019

View reviewed changes

gspencergoog force-pushed the highlight_mode branch 2 times, most recently from f85fda2 to 45e77d7 Compare August 14, 2019 21:12

gspencergoog added 3 commits August 16, 2019 08:50

Add turning off of focus highlight when using touch interfaces.

9d5d516

Add tests

2122f5e

Review changes

87af5bb

gspencergoog force-pushed the highlight_mode branch from 45e77d7 to 98cedf5 Compare August 16, 2019 16:20

Fix test failures

9b33a3f

gspencergoog force-pushed the highlight_mode branch from 98cedf5 to 9b33a3f Compare August 16, 2019 16:21

gspencergoog added the CQ+1 label Aug 16, 2019

gspencergoog merged commit a11d731 into flutter:master Aug 16, 2019

gspencergoog mentioned this pull request Aug 30, 2019

Re-Land of HighlightMode change with benchmark improvements. #39589

Merged

9 tasks

gspencergoog deleted the highlight_mode branch October 9, 2019 23:54

xu-baolin reviewed Nov 12, 2020

View reviewed changes

huycozy mentioned this pull request Jun 12, 2025

focusColor does not work in InkWell on mobile platforms (Android, iOS) #157817

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic focus highlight mode for FocusManager#37825

Automatic focus highlight mode for FocusManager#37825
gspencergoog merged 4 commits intoflutter:masterfrom
gspencergoog:highlight_mode

gspencergoog commented Aug 8, 2019

Uh oh!

HansMuller left a comment

Uh oh!

Uh oh!

HansMuller Aug 13, 2019

Uh oh!

gspencergoog Aug 13, 2019

Uh oh!

HansMuller Aug 13, 2019

Uh oh!

gspencergoog Aug 13, 2019

Uh oh!

HansMuller Aug 13, 2019

Uh oh!

gspencergoog Aug 13, 2019

Uh oh!

xu-baolin Nov 12, 2020 •

edited

Loading

Uh oh!

xu-baolin Nov 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	/// The color of the ink response when the parent widget is focused. If this
	/// property is null then the focus color of the theme,
	/// [ThemeData.focusColor], will be used.
	///
	/// See also:
	///
	/// * [highlightShape], the shape of the focus, hover, and pressed
	/// highlights.
	/// * [hoverColor], the color of the hover highlight.
	/// * [splashColor], the color of the splash.
	/// * [splashFactory], which defines the appearance of the splash.
	final Color? focusColor;

Conversation

gspencergoog commented Aug 8, 2019

Description

Tests

Checklist

Breaking Change

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HansMuller Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

gspencergoog Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

HansMuller Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

gspencergoog Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

HansMuller Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

gspencergoog Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

xu-baolin Nov 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xu-baolin Nov 13, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xu-baolin Nov 12, 2020 •

edited

Loading