Unverified Commit f515bf6a authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Revert "Automatic focus highlight mode for FocusManager (#37825)" (#38866)

This reverts commit a11d7314 because of a regression in
flutter_gallery_ios32__transition_perf's 90th_percentile_frame_build_time_millis.

Fixes #38860.
parent 471995f1
...@@ -478,7 +478,6 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe ...@@ -478,7 +478,6 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
_focusNode?.removeListener(_handleFocusUpdate); _focusNode?.removeListener(_handleFocusUpdate);
_focusNode = Focus.of(context, nullOk: true); _focusNode = Focus.of(context, nullOk: true);
_focusNode?.addListener(_handleFocusUpdate); _focusNode?.addListener(_handleFocusUpdate);
WidgetsBinding.instance.focusManager.addHighlightModeListener(_handleFocusHighlightModeChange);
} }
@override @override
...@@ -492,7 +491,6 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe ...@@ -492,7 +491,6 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
@override @override
void dispose() { void dispose() {
WidgetsBinding.instance.focusManager.removeHighlightModeListener(_handleFocusHighlightModeChange);
_focusNode?.removeListener(_handleFocusUpdate); _focusNode?.removeListener(_handleFocusUpdate);
super.dispose(); super.dispose();
} }
...@@ -610,25 +608,8 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe ...@@ -610,25 +608,8 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
return splash; return splash;
} }
void _handleFocusHighlightModeChange(FocusHighlightMode mode) {
if (!mounted) {
return;
}
setState(() {
_handleFocusUpdate();
});
}
void _handleFocusUpdate() { void _handleFocusUpdate() {
bool showFocus; final bool showFocus = enabled && (Focus.of(context, nullOk: true)?.hasPrimaryFocus ?? false);
switch (WidgetsBinding.instance.focusManager.highlightMode) {
case FocusHighlightMode.touch:
showFocus = false;
break;
case FocusHighlightMode.traditional:
showFocus = enabled && (Focus.of(context, nullOk: true)?.hasPrimaryFocus ?? false);
break;
}
updateHighlight(_HighlightType.focus, value: showFocus); updateHighlight(_HighlightType.focus, value: showFocus);
} }
......
...@@ -3,12 +3,10 @@ ...@@ -3,12 +3,10 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async'; import 'dart:async';
import 'dart:io';
import 'dart:ui'; import 'dart:ui';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/painting.dart'; import 'package:flutter/painting.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'binding.dart'; import 'binding.dart';
...@@ -552,9 +550,6 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { ...@@ -552,9 +550,6 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
/// This object notifies its listeners whenever this value changes. /// This object notifies its listeners whenever this value changes.
bool get hasPrimaryFocus => _manager?.primaryFocus == this; bool get hasPrimaryFocus => _manager?.primaryFocus == this;
/// Returns the [FocusHighlightMode] that is currently in effect for this node.
FocusHighlightMode get highlightMode => WidgetsBinding.instance.focusManager.highlightMode;
/// Returns the nearest enclosing scope node above this node, including /// Returns the nearest enclosing scope node above this node, including
/// this node, if it's a scope. /// this node, if it's a scope.
/// ///
...@@ -997,40 +992,6 @@ class FocusScopeNode extends FocusNode { ...@@ -997,40 +992,6 @@ class FocusScopeNode extends FocusNode {
} }
} }
/// An enum to describe which kind of focus highlight behavior to use when
/// displaying focus information.
enum FocusHighlightMode {
/// Touch interfaces will not show the focus highlight except for controls
/// which bring up the soft keyboard.
///
/// If a device that uses a traditional mouse and keyboard has a touch screen
/// attached, it can also enter `touch` mode if the user is using the touch
/// screen.
touch,
/// Traditional interfaces (keyboard and mouse) will show the currently
/// focused control via a focus highlight of some sort.
///
/// If a touch device (like a mobile phone) has a keyboard and/or mouse
/// attached, it also can enter `traditional` mode if the user is using these
/// input devices.
traditional,
}
/// An enum to describe how the current value of [FocusManager.highlightMode] is
/// determined. The strategy is set on [FocusManager.highlightStrategy].
enum FocusHighlightStrategy {
/// Automatic switches between the various highlight modes based on the last
/// kind of input that was received. This is the default.
automatic,
/// [FocusManager.highlightMode] always returns [FocusHighlightMode.touch].
alwaysTouch,
/// [FocusManager.highlightMode] always returns [FocusHighlightMode.traditional].
alwaysTraditional,
}
/// Manages the focus tree. /// Manages the focus tree.
/// ///
/// The focus tree keeps track of which [FocusNode] is the user's current /// The focus tree keeps track of which [FocusNode] is the user's current
...@@ -1069,129 +1030,6 @@ class FocusManager with DiagnosticableTreeMixin { ...@@ -1069,129 +1030,6 @@ class FocusManager with DiagnosticableTreeMixin {
FocusManager() { FocusManager() {
rootScope._manager = this; rootScope._manager = this;
RawKeyboard.instance.addListener(_handleRawKeyEvent); RawKeyboard.instance.addListener(_handleRawKeyEvent);
RendererBinding.instance.pointerRouter.addGlobalRoute(_handlePointerEvent);
}
bool _lastInteractionWasTouch = true;
/// Sets the strategy by which [highlightMode] is determined.
///
/// If set to [FocusHighlightStrategy.automatic], then the highlight mode will
/// change depending upon the interaction mode used last. For instance, if the
/// last interaction was a touch interaction, then [highlightMode] will return
/// [FocusHighlightMode.touch], and focus highlights will only appear on
/// widgets that bring up a soft keyboard. If the last interaction was a
/// non-touch interaction (hardware keyboard press, mouse click, etc.), then
/// [highlightMode] will return [FocusHighlightMode.traditional], and focus
/// highlights will appear on all widgets.
///
/// If set to [FocusHighlightStrategy.alwaysTouch] or
/// [FocusHighlightStrategy.alwaysTraditional], then [highlightMode] will
/// always return [FocusHighlightMode.touch] or
/// [FocusHighlightMode.traditional], respectively, regardless of the last UI
/// interaction type.
///
/// The initial value of [highlightMode] depends upon the value of
/// [defaultTargetPlatform] and
/// [RendererBinding.instance.mouseTracker.mouseIsConnected], making a guess
/// about which interaction is most appropriate for the initial interaction
/// mode.
///
/// Defaults to [FocusHighlightStrategy.automatic].
FocusHighlightStrategy get highlightStrategy => _highlightStrategy;
FocusHighlightStrategy _highlightStrategy = FocusHighlightStrategy.automatic;
set highlightStrategy(FocusHighlightStrategy highlightStrategy) {
_highlightStrategy = highlightStrategy;
_updateHighlightMode();
}
/// Indicates the current interaction mode for focus highlights.
///
/// The value returned depends upon the [highlightStrategy] used, and possibly
/// (depending on the value of [highlightStrategy]) the most recent
/// interaction mode that they user used.
///
/// If [highlightMode] returns [FocusHighlightMode.touch], then widgets should
/// not draw their focus highlight unless they perform text entry.
///
/// If [highlightMode] returns [FocusHighlightMode.traditional], then widgets should
/// draw their focus highlight whenever they are focused.
FocusHighlightMode get highlightMode => _highlightMode;
FocusHighlightMode _highlightMode = FocusHighlightMode.touch;
// Update function to be called whenever the state relating to highlightMode
// changes.
void _updateHighlightMode() {
// Assume that if we're on one of these mobile platforms, or if there's no
// mouse connected, that the initial interaction will be touch-based, and
// that it's traditional mouse and keyboard on all others.
//
// This only affects the initial value: the ongoing value is updated as soon
// as any input events are received.
_lastInteractionWasTouch ??= Platform.isAndroid || Platform.isIOS || !WidgetsBinding.instance.mouseTracker.mouseIsConnected;
FocusHighlightMode newMode;
switch (highlightStrategy) {
case FocusHighlightStrategy.automatic:
if (_lastInteractionWasTouch) {
newMode = FocusHighlightMode.touch;
} else {
newMode = FocusHighlightMode.traditional;
}
break;
case FocusHighlightStrategy.alwaysTouch:
newMode = FocusHighlightMode.touch;
break;
case FocusHighlightStrategy.alwaysTraditional:
newMode = FocusHighlightMode.traditional;
break;
}
if (newMode != _highlightMode) {
_highlightMode = newMode;
_notifyHighlightModeListeners();
}
}
// The list of listeners for [highlightMode] state changes.
ObserverList<ValueChanged<FocusHighlightMode>> _listeners;
/// Register a closure to be called when the [FocusManager] notifies its listeners
/// that the value of [highlightMode] has changed.
void addHighlightModeListener(ValueChanged<FocusHighlightMode> listener) {
_listeners ??= ObserverList<ValueChanged<FocusHighlightMode>>();
_listeners.add(listener);
}
/// Remove a previously registered closure from the list of closures that the
/// [FocusManager] notifies.
void removeHighlightModeListener(ValueChanged<FocusHighlightMode> listener) {
_listeners?.remove(listener);
}
void _notifyHighlightModeListeners() {
if (_listeners != null) {
final List<ValueChanged<FocusHighlightMode>> localListeners = List<ValueChanged<FocusHighlightMode>>.from(_listeners);
for (ValueChanged<FocusHighlightMode> listener in localListeners) {
try {
if (_listeners.contains(listener)) {
listener(_highlightMode);
}
} catch (exception, stack) {
FlutterError.reportError(FlutterErrorDetails(
exception: exception,
stack: stack,
library: 'widgets library',
context: ErrorDescription('while dispatching notifications for $runtimeType'),
informationCollector: () sync* {
yield DiagnosticsProperty<FocusManager>(
'The $runtimeType sending notification was',
this,
style: DiagnosticsTreeStyle.errorProperty,
);
},
));
}
}
}
} }
/// The root [FocusScopeNode] in the focus tree. /// The root [FocusScopeNode] in the focus tree.
...@@ -1200,33 +1038,7 @@ class FocusManager with DiagnosticableTreeMixin { ...@@ -1200,33 +1038,7 @@ class FocusManager with DiagnosticableTreeMixin {
/// for a given [FocusNode], call [FocusNode.nearestScope]. /// for a given [FocusNode], call [FocusNode.nearestScope].
final FocusScopeNode rootScope = FocusScopeNode(debugLabel: 'Root Focus Scope'); final FocusScopeNode rootScope = FocusScopeNode(debugLabel: 'Root Focus Scope');
void _handlePointerEvent(PointerEvent event) {
bool newState;
switch (event.kind) {
case PointerDeviceKind.touch:
case PointerDeviceKind.stylus:
case PointerDeviceKind.invertedStylus:
newState = true;
break;
case PointerDeviceKind.mouse:
case PointerDeviceKind.unknown:
newState = false;
break;
}
if (_lastInteractionWasTouch != newState) {
_lastInteractionWasTouch = newState;
_updateHighlightMode();
}
}
void _handleRawKeyEvent(RawKeyEvent event) { void _handleRawKeyEvent(RawKeyEvent event) {
// Update this first, since things responding to the keys might look at the
// highlight mode, and it should be accurate.
if (_lastInteractionWasTouch) {
_lastInteractionWasTouch = false;
_updateHighlightMode();
}
// Walk the current focus from the leaf to the root, calling each one's // Walk the current focus from the leaf to the root, calling each one's
// onKey on the way up, and if one responds that they handled it, stop. // onKey on the way up, and if one responds that they handled it, stop.
if (_primaryFocus == null) { if (_primaryFocus == null) {
......
...@@ -2176,14 +2176,7 @@ class BuildOwner { ...@@ -2176,14 +2176,7 @@ class BuildOwner {
/// the [FocusScopeNode] for a given [BuildContext]. /// the [FocusScopeNode] for a given [BuildContext].
/// ///
/// See [FocusManager] for more details. /// See [FocusManager] for more details.
FocusManager get focusManager { FocusManager focusManager = FocusManager();
_focusManager ??= FocusManager();
return _focusManager;
}
FocusManager _focusManager;
set focusManager(FocusManager focusManager) {
_focusManager = focusManager;
}
/// Adds an element to the dirty elements list so that it will be rebuilt /// Adds an element to the dirty elements list so that it will be rebuilt
/// when [WidgetsBinding.drawFrame] calls [buildScope]. /// when [WidgetsBinding.drawFrame] calls [buildScope].
......
...@@ -406,8 +406,6 @@ void main() { ...@@ -406,8 +406,6 @@ void main() {
), ),
), ),
); );
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
focusNode.requestFocus(); focusNode.requestFocus();
await tester.pumpAndSettle(); await tester.pumpAndSettle();
...@@ -499,7 +497,6 @@ void main() { ...@@ -499,7 +497,6 @@ void main() {
), ),
); );
await tester.pumpAndSettle(); await tester.pumpAndSettle();
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
// Base elevation // Base elevation
Material material = tester.widget<Material>(rawButtonMaterial); Material material = tester.widget<Material>(rawButtonMaterial);
......
...@@ -121,7 +121,6 @@ void main() { ...@@ -121,7 +121,6 @@ void main() {
}); });
testWidgets('ink response changes color on focus', (WidgetTester tester) async { testWidgets('ink response changes color on focus', (WidgetTester tester) async {
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus'); final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus');
await tester.pumpWidget(Material( await tester.pumpWidget(Material(
child: Directionality( child: Directionality(
...@@ -155,40 +154,6 @@ void main() { ...@@ -155,40 +154,6 @@ void main() {
..rect(rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0), color: const Color(0xff0000ff))); ..rect(rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0), color: const Color(0xff0000ff)));
}); });
testWidgets("ink response doesn't change color on focus when on touch device", (WidgetTester tester) async {
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTouch;
final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus');
await tester.pumpWidget(Material(
child: Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: Focus(
focusNode: focusNode,
child: Container(
width: 100,
height: 100,
child: InkWell(
hoverColor: const Color(0xff00ff00),
splashColor: const Color(0xffff0000),
focusColor: const Color(0xff0000ff),
highlightColor: const Color(0xf00fffff),
onTap: () {},
onLongPress: () {},
onHover: (bool hover) {}
),
),
),
),
),
));
await tester.pumpAndSettle();
final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures');
expect(inkFeatures, paintsExactlyCountTimes(#rect, 0));
focusNode.requestFocus();
await tester.pumpAndSettle();
expect(inkFeatures, paintsExactlyCountTimes(#rect, 0));
});
group('feedback', () { group('feedback', () {
FeedbackTester feedback; FeedbackTester feedback;
......
...@@ -215,7 +215,6 @@ void main() { ...@@ -215,7 +215,6 @@ void main() {
const Key key = Key('test'); const Key key = Key('test');
const Color focusColor = Color(0xff00ff00); const Color focusColor = Color(0xff00ff00);
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
home: Center( home: Center(
...@@ -242,7 +241,6 @@ void main() { ...@@ -242,7 +241,6 @@ void main() {
const Key key = Key('test'); const Key key = Key('test');
const Color hoverColor = Color(0xff00ff00); const Color hoverColor = Color(0xff00ff00);
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
home: Center( home: Center(
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
import 'dart:typed_data'; import 'dart:typed_data';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -484,55 +483,6 @@ void main() { ...@@ -484,55 +483,6 @@ void main() {
// receive it. // receive it.
expect(receivedAnEvent, isEmpty); expect(receivedAnEvent, isEmpty);
}); });
testWidgets('Events change focus highlight mode.', (WidgetTester tester) async {
await setupWidget(tester);
int callCount = 0;
FocusHighlightMode lastMode;
void handleModeChange(FocusHighlightMode mode) {
lastMode = mode;
callCount++;
}
final FocusManager focusManager = WidgetsBinding.instance.focusManager;
focusManager.addHighlightModeListener(handleModeChange);
addTearDown(() => focusManager.removeHighlightModeListener(handleModeChange));
expect(callCount, equals(0));
expect(lastMode, isNull);
focusManager.highlightStrategy = FocusHighlightStrategy.automatic;
expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
sendFakeKeyEvent(<String, dynamic>{
'type': 'keydown',
'keymap': 'fuchsia',
'hidUsage': 0x04,
'codePoint': 0x64,
'modifiers': RawKeyEventDataFuchsia.modifierLeftMeta,
});
expect(callCount, equals(1));
expect(lastMode, FocusHighlightMode.traditional);
expect(focusManager.highlightMode, equals(FocusHighlightMode.traditional));
await tester.tap(find.byType(Container));
expect(callCount, equals(2));
expect(lastMode, FocusHighlightMode.touch);
expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
final TestGesture gesture = await tester.startGesture(Offset.zero, kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.up();
expect(callCount, equals(3));
expect(lastMode, FocusHighlightMode.traditional);
expect(focusManager.highlightMode, equals(FocusHighlightMode.traditional));
await tester.tap(find.byType(Container));
expect(callCount, equals(4));
expect(lastMode, FocusHighlightMode.touch);
expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
expect(callCount, equals(5));
expect(lastMode, FocusHighlightMode.traditional);
expect(focusManager.highlightMode, equals(FocusHighlightMode.traditional));
focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTouch;
expect(callCount, equals(6));
expect(lastMode, FocusHighlightMode.touch);
expect(focusManager.highlightMode, equals(FocusHighlightMode.touch));
});
testWidgets('implements debugFillProperties', (WidgetTester tester) async { testWidgets('implements debugFillProperties', (WidgetTester tester) async {
final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder(); final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder();
FocusScopeNode( FocusScopeNode(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment