Commit d6580489 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Allow multiple FloatingActionButtons to be used on one screen. (#12074)

* Allow FloatingActionButton to not have a heroTag.
* Allow FloatingActionButton to not have a child.
* Allow Tooltip to not have a child.
* Improve the debug output of the default FloatingActionButton hero tag.
* Improve the error message in the Hero clashing-tag case.
* Improve the debug output of the Hero widget.
* Improve the debug output of gesture-related widgets.
* Minor improvements to documentation.
* Fix some typos in comments.
* Fix some style nits.
parent 08869497
......@@ -549,7 +549,7 @@ class _PrefixedStringBuilder {
if (s == '\n') {
// Edge case to avoid adding trailing whitespace when the caller did
// not explicitly add trailing trailing whitespace.
// not explicitly add trailing whitespace.
if (_buffer.isEmpty) {
_buffer.write(prefixLineOne.trimRight());
} else if (_atLineStart) {
......@@ -810,6 +810,11 @@ abstract class DiagnosticsNode {
/// Returns a string representation of this node and its descendants.
///
/// `prefixLineOne` will be added to the front of the first line of the
/// output. `prefixOtherLines` will be added to the front of each other line.
/// If `prefixOtherLines` is null, the `prefixLineOne` is used for every line.
/// By default, there is no prefix.
///
/// `minLevel` specifies the minimum [DiagnosticLevel] for properties included
/// in the output.
///
......@@ -835,8 +840,10 @@ abstract class DiagnosticsNode {
if (prefixOtherLines.isEmpty)
prefixOtherLines += config.prefixOtherLinesRootNode;
final _PrefixedStringBuilder builder = new _PrefixedStringBuilder(
prefixLineOne, prefixOtherLines);
final _PrefixedStringBuilder builder = new _PrefixedStringBuilder(
prefixLineOne,
prefixOtherLines,
);
final String description = toDescription(parentConfiguration: parentConfiguration);
if (description == null || description.isEmpty) {
......@@ -856,8 +863,9 @@ abstract class DiagnosticsNode {
builder.write(description);
}
final List<DiagnosticsNode> properties =
getProperties().where((DiagnosticsNode n) => !n.isFiltered(minLevel)).toList();
final List<DiagnosticsNode> properties = getProperties().where(
(DiagnosticsNode n) => !n.isFiltered(minLevel)
).toList();
if (properties.isNotEmpty || children.isNotEmpty || emptyBodyDescription != null)
builder.write(config.afterDescriptionIfBody);
......@@ -2322,6 +2330,11 @@ abstract class DiagnosticableTree extends Diagnosticable {
/// Returns a string representation of this node and its descendants.
///
/// `prefixLineOne` will be added to the front of the first line of the
/// output. `prefixOtherLines` will be added to the front of each other line.
/// If `prefixOtherLines` is null, the `prefixLineOne` is used for every line.
/// By default, there is no prefix.
///
/// `minLevel` specifies the minimum [DiagnosticLevel] for properties included
/// in the output.
///
......
......@@ -15,7 +15,12 @@ import 'tooltip.dart';
// http://material.google.com/layout/metrics-keylines.html#metrics-keylines-keylines-spacing
const double _kSize = 56.0;
const double _kSizeMini = 40.0;
final Object _kDefaultHeroTag = new Object();
class _DefaultHeroTag {
const _DefaultHeroTag();
@override
String toString() => '<default FloatingActionButton tag>';
}
/// A material design floating action button.
///
......@@ -42,10 +47,10 @@ class FloatingActionButton extends StatefulWidget {
/// Most commonly used in the [Scaffold.floatingActionButton] field.
const FloatingActionButton({
Key key,
@required this.child,
this.child,
this.tooltip,
this.backgroundColor,
this.heroTag,
this.heroTag: const _DefaultHeroTag(),
this.elevation: 6.0,
this.highlightElevation: 12.0,
@required this.onPressed,
......@@ -69,6 +74,15 @@ class FloatingActionButton extends StatefulWidget {
/// The tag to apply to the button's [Hero] widget.
///
/// Defaults to a tag that matches other floating action buttons.
///
/// Set this to null explicitly if you don't want the floating action button to
/// have a hero tag.
///
/// If this is not explicitly set, then there can only be one
/// [FloatingActionButton] per route (that is, per screen), since otherwise
/// there would be a tag conflict (multiple heroes on one route can't have the
/// same tag). The material design specification recommends only using one
/// floating action button per screen.
final Object heroTag;
/// The callback that is called when the button is tapped or otherwise activated.
......@@ -124,36 +138,46 @@ class _FloatingActionButtonState extends State<FloatingActionButton> {
iconColor = themeData.accentIconTheme.color;
}
Widget result = new Center(
child: IconTheme.merge(
data: new IconThemeData(color: iconColor),
child: widget.child
)
);
Widget result;
if (widget.child != null) {
result = new Center(
child: IconTheme.merge(
data: new IconThemeData(color: iconColor),
child: widget.child,
),
);
}
if (widget.tooltip != null) {
result = new Tooltip(
message: widget.tooltip,
child: result
child: result,
);
}
return new Hero(
tag: widget.heroTag ?? _kDefaultHeroTag,
child: new Material(
color: materialColor,
type: MaterialType.circle,
elevation: _highlight ? widget.highlightElevation : widget.elevation,
child: new Container(
width: widget.mini ? _kSizeMini : _kSize,
height: widget.mini ? _kSizeMini : _kSize,
child: new InkWell(
onTap: widget.onPressed,
onHighlightChanged: _handleHighlightChanged,
child: result
)
)
)
result = new Material(
color: materialColor,
type: MaterialType.circle,
elevation: _highlight ? widget.highlightElevation : widget.elevation,
child: new Container(
width: widget.mini ? _kSizeMini : _kSize,
height: widget.mini ? _kSizeMini : _kSize,
child: new InkWell(
onTap: widget.onPressed,
onHighlightChanged: _handleHighlightChanged,
child: result,
),
),
);
if (widget.heroTag != null) {
result = new Hero(
tag: widget.heroTag,
child: result,
);
}
return result;
}
}
......@@ -231,9 +231,7 @@ class InkResponse extends StatefulWidget {
gestures.add('double tap');
if (onLongPress != null)
gestures.add('long press');
if (gestures.isEmpty)
gestures.add('<none>');
description.add(new IterableProperty<String>('gestures', gestures));
description.add(new IterableProperty<String>('gestures', gestures, ifEmpty: '<none>'));
description.add(new DiagnosticsProperty<bool>('containedInkWell', containedInkWell, level: DiagnosticLevel.fine));
description.add(new DiagnosticsProperty<BoxShape>(
'highlightShape',
......
......@@ -48,13 +48,12 @@ class Tooltip extends StatefulWidget {
this.padding: const EdgeInsets.symmetric(horizontal: 16.0),
this.verticalOffset: 24.0,
this.preferBelow: true,
@required this.child,
this.child,
}) : assert(message != null),
assert(height != null),
assert(padding != null),
assert(verticalOffset != null),
assert(preferBelow != null),
assert(child != null),
super(key: key);
/// The text to display in the tooltip.
......
......@@ -4102,9 +4102,7 @@ class Listener extends SingleChildRenderObjectWidget {
listeners.add('up');
if (onPointerCancel != null)
listeners.add('cancel');
if (listeners.isEmpty)
listeners.add('<none>');
description.add(new IterableProperty<String>('listeners', listeners));
description.add(new IterableProperty<String>('listeners', listeners, ifEmpty: '<none>'));
description.add(new EnumProperty<HitTestBehavior>('behavior', behavior));
}
}
......
......@@ -710,9 +710,7 @@ class RawGestureDetectorState extends State<RawGestureDetector> {
description.add(new DiagnosticsNode.message('DISPOSED'));
} else {
final List<String> gestures = _recognizers.values.map<String>((GestureRecognizer recognizer) => recognizer.debugDescription).toList();
if (gestures.isEmpty)
gestures.add('<none>');
description.add(new IterableProperty<String>('gestures', gestures));
description.add(new IterableProperty<String>('gestures', gestures, ifEmpty: '<none>'));
description.add(new IterableProperty<GestureRecognizer>('recognizers', _recognizers.values, level: DiagnosticLevel.fine));
}
description.add(new EnumProperty<HitTestBehavior>('behavior', widget.behavior, defaultValue: null));
......
......@@ -115,7 +115,9 @@ class Hero extends StatefulWidget {
'There are multiple heroes that share the same tag within a subtree.\n'
'Within each subtree for which heroes are to be animated (typically a PageRoute subtree), '
'each Hero must have a unique non-null tag.\n'
'In this case, multiple heroes had the tag "$tag".'
'In this case, multiple heroes had the following tag: $tag\n'
'Here is the subtree for one of the offending heroes:\n'
'${element.toStringDeep(prefixLineOne: "# ")}'
);
}
return true;
......@@ -131,6 +133,12 @@ class Hero extends StatefulWidget {
@override
_HeroState createState() => new _HeroState();
@override
void debugFillProperties(DiagnosticPropertiesBuilder description) {
super.debugFillProperties(description);
description.add(new DiagnosticsProperty<Object>('tag', tag));
}
}
class _HeroState extends State<Hero> {
......
......@@ -43,4 +43,93 @@ void main() {
await tester.tap(find.byType(Icon));
expect(find.byTooltip('Add'), findsOneWidget);
});
testWidgets('Floating Action Button tooltip (no child)', (WidgetTester tester) async {
await tester.pumpWidget(
new MaterialApp(
home: const Scaffold(
floatingActionButton: const FloatingActionButton(
onPressed: null,
tooltip: 'Add',
),
),
),
);
expect(find.byType(Text), findsNothing);
await tester.longPress(find.byType(FloatingActionButton));
await tester.pump();
expect(find.byType(Text), findsOneWidget);
});
testWidgets('Floating Action Button heroTag', (WidgetTester tester) async {
BuildContext theContext;
await tester.pumpWidget(
new MaterialApp(
home: new Scaffold(
body: new Builder(
builder: (BuildContext context) {
theContext = context;
return const FloatingActionButton(heroTag: 1, onPressed: null);
},
),
floatingActionButton: const FloatingActionButton(heroTag: 2, onPressed: null),
),
),
);
Navigator.push(theContext, new PageRouteBuilder<Null>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return const Placeholder();
},
));
await tester.pump(); // this would fail if heroTag was the same on both FloatingActionButtons (see below).
});
testWidgets('Floating Action Button heroTag - with duplicate', (WidgetTester tester) async {
BuildContext theContext;
await tester.pumpWidget(
new MaterialApp(
home: new Scaffold(
body: new Builder(
builder: (BuildContext context) {
theContext = context;
return const FloatingActionButton(onPressed: null);
},
),
floatingActionButton: const FloatingActionButton(onPressed: null),
),
),
);
Navigator.push(theContext, new PageRouteBuilder<Null>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return const Placeholder();
},
));
await tester.pump();
expect(tester.takeException().toString(), contains('FloatingActionButton'));
});
testWidgets('Floating Action Button heroTag - with duplicate', (WidgetTester tester) async {
BuildContext theContext;
await tester.pumpWidget(
new MaterialApp(
home: new Scaffold(
body: new Builder(
builder: (BuildContext context) {
theContext = context;
return const FloatingActionButton(heroTag: 'xyzzy', onPressed: null);
},
),
floatingActionButton: const FloatingActionButton(heroTag: 'xyzzy', onPressed: null),
),
),
);
Navigator.push(theContext, new PageRouteBuilder<Null>(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return const Placeholder();
},
));
await tester.pump();
expect(tester.takeException().toString(), contains('xyzzy'));
});
}
......@@ -70,15 +70,15 @@ void main() {
// Analyze in the current directory - no arguments
testUsingContext('flutter analyze working directory with errors', () async {
// Break the code to produce the "The parameter 'child' is required" hint
// Break the code to produce the "The parameter 'onPressed' is required" hint
// that is upgraded to a warning in package:flutter/analysis_options_user.yaml
// to assert that we are using the default Flutter analysis options.
// Also insert a statement that should not trigger a lint here
// but will trigger a lint later on when an analysis_options.yaml is added.
String source = await libMain.readAsString();
source = source.replaceFirst(
'child: new Icon(Icons.add),',
'// child: new Icon(Icons.add),',
'onPressed: _incrementCounter,',
'// onPressed: _incrementCounter,',
);
source = source.replaceFirst(
'_counter++;',
......@@ -92,8 +92,9 @@ void main() {
arguments: <String>['analyze'],
statusTextContains: <String>[
'Analyzing',
'warning $analyzerSeparator The parameter \'child\' is required',
'1 issue found.',
'warning $analyzerSeparator The parameter \'onPressed\' is required',
'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used',
'2 issues found.',
],
toolExit: true,
);
......@@ -106,8 +107,9 @@ void main() {
arguments: <String>['analyze', libMain.path],
statusTextContains: <String>[
'Analyzing',
'warning $analyzerSeparator The parameter \'child\' is required',
'1 issue found.',
'warning $analyzerSeparator The parameter \'onPressed\' is required',
'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used',
'2 issues found.',
],
toolExit: true,
);
......@@ -132,9 +134,10 @@ void main() {
arguments: <String>['analyze'],
statusTextContains: <String>[
'Analyzing',
'warning $analyzerSeparator The parameter \'child\' is required',
'warning $analyzerSeparator The parameter \'onPressed\' is required',
'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used',
'lint $analyzerSeparator Only throw instances of classes extending either Exception or Error',
'2 issues found.',
'3 issues found.',
],
toolExit: true,
);
......@@ -181,9 +184,10 @@ void bar() {
arguments: <String>['analyze', libMain.path],
statusTextContains: <String>[
'Analyzing',
'warning $analyzerSeparator The parameter \'child\' is required',
'warning $analyzerSeparator The parameter \'onPressed\' is required',
'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used',
'lint $analyzerSeparator Only throw instances of classes extending either Exception or Error',
'2 issues found.',
'3 issues found.',
],
toolExit: true,
);
......
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