Unverified Commit f4ba8d1a authored by MH Johnson's avatar MH Johnson Committed by GitHub

[Material] Remove text ripple from TextFields (#41320)

* remove splash logic

* update tests
parent 25f2399b
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:collection';
import 'package:flutter/cupertino.dart'; import 'package:flutter/cupertino.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
...@@ -13,7 +11,6 @@ import 'package:flutter/gestures.dart'; ...@@ -13,7 +11,6 @@ import 'package:flutter/gestures.dart';
import 'debug.dart'; import 'debug.dart';
import 'feedback.dart'; import 'feedback.dart';
import 'ink_well.dart' show InteractiveInkFeature;
import 'input_decorator.dart'; import 'input_decorator.dart';
import 'material.dart'; import 'material.dart';
import 'material_localizations.dart'; import 'material_localizations.dart';
...@@ -44,12 +41,6 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete ...@@ -44,12 +41,6 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete
final _TextFieldState _state; final _TextFieldState _state;
@override
void onTapDown(TapDownDetails details) {
super.onTapDown(details);
_state._startSplash(details.globalPosition);
}
@override @override
void onForcePressStart(ForcePressDetails details) { void onForcePressStart(ForcePressDetails details) {
super.onForcePressStart(details); super.onForcePressStart(details);
...@@ -100,16 +91,10 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete ...@@ -100,16 +91,10 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete
} }
} }
_state._requestKeyboard(); _state._requestKeyboard();
_state._confirmCurrentSplash();
if (_state.widget.onTap != null) if (_state.widget.onTap != null)
_state.widget.onTap(); _state.widget.onTap();
} }
@override
void onSingleTapCancel() {
_state._cancelCurrentSplash();
}
@override @override
void onSingleLongTapStart(LongPressStartDetails details) { void onSingleLongTapStart(LongPressStartDetails details) {
if (delegate.selectionEnabled) { if (delegate.selectionEnabled) {
...@@ -127,13 +112,6 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete ...@@ -127,13 +112,6 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete
break; break;
} }
} }
_state._confirmCurrentSplash();
}
@override
void onDragSelectionStart(DragStartDetails details) {
super.onDragSelectionStart(details);
_state._startSplash(details.globalPosition);
} }
} }
...@@ -160,9 +138,7 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete ...@@ -160,9 +138,7 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete
/// extra padding introduced by the decoration to save space for the labels. /// extra padding introduced by the decoration to save space for the labels.
/// ///
/// If [decoration] is non-null (which is the default), the text field requires /// If [decoration] is non-null (which is the default), the text field requires
/// one of its ancestors to be a [Material] widget. When the [TextField] is /// one of its ancestors to be a [Material] widget.
/// tapped an ink splash that paints on the material is triggered, see
/// [ThemeData.splashFactory].
/// ///
/// To integrate the [TextField] into a [Form] with other [FormField] widgets, /// To integrate the [TextField] into a [Form] with other [FormField] widgets,
/// consider using [TextFormField]. /// consider using [TextFormField].
...@@ -720,10 +696,7 @@ class TextField extends StatefulWidget { ...@@ -720,10 +696,7 @@ class TextField extends StatefulWidget {
} }
} }
class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixin implements TextSelectionGestureDetectorBuilderDelegate { class _TextFieldState extends State<TextField> implements TextSelectionGestureDetectorBuilderDelegate {
Set<InteractiveInkFeature> _splashes;
InteractiveInkFeature _currentSplash;
TextEditingController _controller; TextEditingController _controller;
TextEditingController get _effectiveController => widget.controller ?? _controller; TextEditingController get _effectiveController => widget.controller ?? _controller;
...@@ -905,75 +878,6 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi ...@@ -905,75 +878,6 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
} }
} }
InteractiveInkFeature _createInkFeature(Offset globalPosition) {
final MaterialInkController inkController = Material.of(context);
final ThemeData themeData = Theme.of(context);
final BuildContext editableContext = editableTextKey.currentContext;
final RenderBox referenceBox = InputDecorator.containerOf(editableContext) ?? editableContext.findRenderObject();
final Offset position = referenceBox.globalToLocal(globalPosition);
final Color color = themeData.splashColor;
InteractiveInkFeature splash;
void handleRemoved() {
if (_splashes != null) {
assert(_splashes.contains(splash));
_splashes.remove(splash);
if (_currentSplash == splash)
_currentSplash = null;
updateKeepAlive();
} // else we're probably in deactivate()
}
splash = themeData.splashFactory.create(
controller: inkController,
referenceBox: referenceBox,
position: position,
color: color,
containedInkWell: true,
// TODO(hansmuller): splash clip borderRadius should match the input decorator's border.
borderRadius: BorderRadius.zero,
onRemoved: handleRemoved,
textDirection: Directionality.of(context),
);
return splash;
}
void _startSplash(Offset globalPosition) {
if (_effectiveFocusNode.hasFocus)
return;
final InteractiveInkFeature splash = _createInkFeature(globalPosition);
_splashes ??= HashSet<InteractiveInkFeature>();
_splashes.add(splash);
_currentSplash = splash;
updateKeepAlive();
}
void _confirmCurrentSplash() {
_currentSplash?.confirm();
_currentSplash = null;
}
void _cancelCurrentSplash() {
_currentSplash?.cancel();
}
@override
bool get wantKeepAlive => _splashes != null && _splashes.isNotEmpty;
@override
void deactivate() {
if (_splashes != null) {
final Set<InteractiveInkFeature> splashes = _splashes;
_splashes = null;
for (InteractiveInkFeature splash in splashes)
splash.dispose();
_currentSplash = null;
}
assert(_currentSplash == null);
super.deactivate();
}
void _handleMouseEnter(PointerEnterEvent event) => _handleHover(true); void _handleMouseEnter(PointerEnterEvent event) => _handleHover(true);
void _handleMouseExit(PointerExitEvent event) => _handleHover(false); void _handleMouseExit(PointerExitEvent event) => _handleHover(false);
...@@ -987,7 +891,6 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi ...@@ -987,7 +891,6 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
super.build(context); // See AutomaticKeepAliveClientMixin.
assert(debugCheckHasMaterial(context)); assert(debugCheckHasMaterial(context));
// TODO(jonahwilliams): uncomment out this check once we have migrated tests. // TODO(jonahwilliams): uncomment out this check once we have migrated tests.
// assert(debugCheckHasMaterialLocalizations(context)); // assert(debugCheckHasMaterialLocalizations(context));
......
...@@ -7,8 +7,8 @@ import 'package:flutter/material.dart'; ...@@ -7,8 +7,8 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
int confirmCount = 0; bool confirmCalled = false;
int cancelCount = 0; bool cancelCalled = false;
class TestInkSplash extends InkSplash { class TestInkSplash extends InkSplash {
TestInkSplash({ TestInkSplash({
...@@ -39,13 +39,13 @@ class TestInkSplash extends InkSplash { ...@@ -39,13 +39,13 @@ class TestInkSplash extends InkSplash {
@override @override
void confirm() { void confirm() {
confirmCount += 1; confirmCalled = true;
super.confirm(); super.confirm();
} }
@override @override
void cancel() { void cancel() {
cancelCount += 1; cancelCalled = true;
super.cancel(); super.cancel();
} }
} }
...@@ -84,7 +84,12 @@ class TestInkSplashFactory extends InteractiveInkFeatureFactory { ...@@ -84,7 +84,12 @@ class TestInkSplashFactory extends InteractiveInkFeatureFactory {
} }
void main() { void main() {
testWidgets('Tap and no focus causes a splash', (WidgetTester tester) async { setUp(() {
confirmCalled = false;
cancelCalled = false;
});
testWidgets('Tapping should never cause a splash', (WidgetTester tester) async {
final Key textField1 = UniqueKey(); final Key textField1 = UniqueKey();
final Key textField2 = UniqueKey(); final Key textField2 = UniqueKey();
...@@ -117,41 +122,33 @@ void main() { ...@@ -117,41 +122,33 @@ void main() {
) )
); );
confirmCount = 0;
cancelCount = 0;
await tester.tap(find.byKey(textField1)); await tester.tap(find.byKey(textField1));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(confirmCount, 1); expect(confirmCalled, isFalse);
expect(cancelCount, 0); expect(cancelCalled, isFalse);
// textField1 already has the focus, no new splash
await tester.tap(find.byKey(textField1)); await tester.tap(find.byKey(textField1));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(confirmCount, 1); expect(confirmCalled, isFalse);
expect(cancelCount, 0); expect(cancelCalled, isFalse);
// textField2 gets the focus and a splash
await tester.tap(find.byKey(textField2)); await tester.tap(find.byKey(textField2));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(confirmCount, 2); expect(confirmCalled, isFalse);
expect(cancelCount, 0); expect(cancelCalled, isFalse);
// Tap outside of textField1's editable. It still gets focus and splash.
await tester.tapAt(tester.getTopLeft(find.byKey(textField1))); await tester.tapAt(tester.getTopLeft(find.byKey(textField1)));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(confirmCount, 3); expect(confirmCalled, isFalse);
expect(cancelCount, 0); expect(cancelCalled, isFalse);
// Tap in the center of textField2's editable. It still gets the focus
// and the splash. There is no splash cancel.
await tester.tap(find.byKey(textField2)); await tester.tap(find.byKey(textField2));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(confirmCount, 4); expect(confirmCalled, isFalse);
expect(cancelCount, 0); expect(cancelCalled, isFalse);
}); });
testWidgets('Splash cancel', (WidgetTester tester) async { testWidgets('Splash should never be created or canceled', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
home: Theme( home: Theme(
...@@ -180,29 +177,22 @@ void main() { ...@@ -180,29 +177,22 @@ void main() {
) )
); );
confirmCount = 0; // If there were a splash, this would cancel the splash.
cancelCount = 0;
// Pointer is dragged below the textfield, splash is canceled.
final TestGesture gesture1 = await tester.startGesture(tester.getCenter(find.text('label1'))); final TestGesture gesture1 = await tester.startGesture(tester.getCenter(find.text('label1')));
// Splashes start on tapDown.
// If the timeout is less than kPressTimeout the recognizer will just trigger
// the onTapCancel callback. If the timeout is greater or equal to kPressTimeout
// and less than kLongPressTimeout then onTapDown, onCancel will be called.
await tester.pump(kPressTimeout); await tester.pump(kPressTimeout);
await gesture1.moveTo(const Offset(400.0, 300.0)); await gesture1.moveTo(const Offset(400.0, 300.0));
await gesture1.up(); await gesture1.up();
expect(confirmCount, 0); expect(confirmCalled, isFalse);
expect(cancelCount, 1); expect(cancelCalled, isFalse);
// Pointer is dragged upwards causing a scroll, splash is canceled. // Pointer is dragged upwards causing a scroll, splash would be canceled.
final TestGesture gesture2 = await tester.startGesture(tester.getCenter(find.text('label2'))); final TestGesture gesture2 = await tester.startGesture(tester.getCenter(find.text('label2')));
await tester.pump(kPressTimeout); await tester.pump(kPressTimeout);
await gesture2.moveBy(const Offset(0.0, -200.0)); await gesture2.moveBy(const Offset(0.0, -200.0));
await gesture2.up(); await gesture2.up();
expect(confirmCount, 0); expect(confirmCalled, isFalse);
expect(cancelCount, 2); expect(cancelCalled, isFalse);
}); });
} }
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