Automatic focus highlight mode for FocusManager #37825
Conversation
| /// | ||
| /// If [highlightMode] returns [FocusHighlightMode.touch], then widgets should | ||
| /// only draw their focus highlight if they are currently focused, and they | ||
| /// bring up the soft keyboard (e.g. [TextField]) when focused. |
There was a problem hiding this comment.
The TextField reference is little confusing. TextField doesn't always bring up a software keyboard because desktop etc.
| } | ||
| } | ||
|
|
||
| // The list of listeners for [highlightMode] state changes. |
There was a problem hiding this comment.
Maybe use a ValueNotifier<FocusHighlightMode> instance here? Might eliminate some of the boilerplate.
There was a problem hiding this comment.
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.
| @@ -956,6 +995,130 @@ class FocusManager with DiagnosticableTreeMixin { | |||
| FocusManager() { | |||
| rootScope._manager = this; | |||
| RawKeyboard.instance.addListener(_handleRawKeyEvent); | |||
| RendererBinding.instance.pointerRouter.addGlobalRoute(_handlePointerEvent); | |||
There was a problem hiding this comment.
Where do we remove this "global route"?
There was a problem hiding this comment.
We don't, because once we construct the FocusManager, we never release it: it's a singleton attached to the binding.
f85fda2 to
45e77d7
Compare
45e77d7 to
98cedf5
Compare
98cedf5 to
9b33a3f
Compare
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.
| bool showFocus; | ||
| switch (WidgetsBinding.instance.focusManager.highlightMode) { | ||
| case FocusHighlightMode.touch: | ||
| showFocus = false; |
There was a problem hiding this comment.
@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
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.
Description
This adds a
FocusHighlightModeto theFocusManagerthat 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 aFocusHighlightStrategyenum 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
FocusManagerandInkWell.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.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?