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

Fix WidgetsApp to wrap its contents in a Directionality (#12018)

...rather than only wrapping the children.

Also, improvements to Banner to better support RTL, and mock_canvas to
make debugging things easier.
parent c46347b6
......@@ -104,7 +104,7 @@ enum _WordWrapParseMode { inSpace, inWord, atBreak }
///
/// The default [debugPrint] implementation uses this for its line wrapping.
Iterable<String> debugWordWrap(String message, int width, { String wrapIndent: '' }) sync* {
if (message.length < width || message[0] == '#') {
if (message.length < width || message.trimLeft()[0] == '#') {
yield message;
return;
}
......
......@@ -2293,6 +2293,7 @@ class RenderCustomPaint extends RenderProxyBox {
int debugPreviousCanvasSaveCount;
canvas.save();
assert(() { debugPreviousCanvasSaveCount = canvas.getSaveCount(); return true; });
if (offset != Offset.zero)
canvas.translate(offset.dx, offset.dy);
painter.paint(canvas, size);
assert(() {
......
......@@ -296,28 +296,18 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
@override
Widget build(BuildContext context) {
Widget result = new MediaQuery(
data: new MediaQueryData.fromWindow(ui.window),
child: new Localizations(
locale: widget.locale ?? _locale,
delegates: _localizationsDelegates.toList(),
child: new Title(
title: widget.title,
color: widget.color,
child: new Navigator(
Widget result = new Navigator(
key: _navigator,
initialRoute: widget.initialRoute ?? ui.window.defaultRouteName,
onGenerateRoute: widget.onGenerateRoute,
onUnknownRoute: widget.onUnknownRoute,
observers: widget.navigatorObservers
)
)
)
observers: widget.navigatorObservers,
);
if (widget.textStyle != null) {
result = new DefaultTextStyle(
style: widget.textStyle,
child: result
child: result,
);
}
......@@ -335,7 +325,6 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
checkerboardOffscreenLayers: widget.checkerboardOffscreenLayers,
);
}
if (performanceOverlay != null) {
result = new Stack(
children: <Widget>[
......@@ -344,11 +333,13 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
]
);
}
if (widget.showSemanticsDebugger) {
result = new SemanticsDebugger(
child: result,
);
}
assert(() {
if (widget.debugShowWidgetInspector || WidgetsApp.debugShowWidgetInspectorOverride) {
result = new WidgetInspector(
......@@ -364,7 +355,17 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
return true;
});
return result;
return new MediaQuery(
data: new MediaQueryData.fromWindow(ui.window),
child: new Localizations(
locale: widget.locale ?? _locale,
delegates: _localizationsDelegates.toList(),
child: new Title(
title: widget.title,
color: widget.color,
child: result,
),
),
);
}
}
......@@ -23,25 +23,30 @@ const TextStyle _kTextStyle = const TextStyle(
);
/// Where to show a [Banner].
///
/// The start and end locations are relative to the ambient [Directionality]
/// (which can be overridden by [Banner.layoutDirection]).
enum BannerLocation {
/// Show the banner in the top-right corner when the ambient [Directionality]
/// is [TextDirection.rtl] and in the top-left corner when the ambient
/// [Directionality] is [TextDirection.ltr].
/// (or [Banner.layoutDirection]) is [TextDirection.rtl] and in the top-left
/// corner when the ambient [Directionality] is [TextDirection.ltr].
topStart,
/// Show the banner in the top-left corner when the ambient [Directionality]
/// is [TextDirection.rtl] and in the top-right corner when the ambient
/// [Directionality] is [TextDirection.ltr].
/// (or [Banner.layoutDirection]) is [TextDirection.rtl] and in the top-right
/// corner when the ambient [Directionality] is [TextDirection.ltr].
topEnd,
/// Show the banner in the bottom-right corner when the ambient
/// [Directionality] is [TextDirection.rtl] and in the bottom-left corner when
/// the ambient [Directionality] is [TextDirection.ltr].
/// [Directionality] (or [Banner.layoutDirection]) is [TextDirection.rtl] and
/// in the bottom-left corner when the ambient [Directionality] is
/// [TextDirection.ltr].
bottomStart,
/// Show the banner in the bottom-left corner when the ambient
/// [Directionality] is [TextDirection.rtl] and in the bottom-right corner when
/// the ambient [Directionality] is [TextDirection.ltr].
/// [Directionality] (or [Banner.layoutDirection]) is [TextDirection.rtl] and
/// in the bottom-right corner when the ambient [Directionality] is
/// [TextDirection.ltr].
bottomEnd,
}
......@@ -49,11 +54,13 @@ enum BannerLocation {
class BannerPainter extends CustomPainter {
/// Creates a banner painter.
///
/// The [message], [textDirection], and [location] arguments must not be null.
/// The [message], [textDirection], [location], and [layoutDirection]
/// arguments must not be null.
BannerPainter({
@required this.message,
@required this.textDirection,
@required this.location,
@required this.layoutDirection,
this.color: _kColor,
this.textStyle: _kTextStyle,
}) : assert(message != null),
......@@ -74,12 +81,21 @@ class BannerPainter extends CustomPainter {
/// context, the English phrase will be on the right and the Hebrow phrase on
/// its left.
///
/// This value is also used to interpret the [location] of the banner.
/// See also [layoutDirection], which controls the interpretation of values in
/// [location].
final TextDirection textDirection;
/// Where to show the banner (e.g., the upper right corder).
final BannerLocation location;
/// The directionality of the layout.
///
/// This value is used to interpret the [location] of the banner.
///
/// See also [textDirection], which controls the reading direction of the
/// [message].
final TextDirection layoutDirection;
/// The color to paint behind the [message].
///
/// Defaults to a dark red.
......@@ -136,8 +152,8 @@ class BannerPainter extends CustomPainter {
double _translationX(double width) {
assert(location != null);
assert(textDirection != null);
switch (textDirection) {
assert(layoutDirection != null);
switch (layoutDirection) {
case TextDirection.rtl:
switch (location) {
case BannerLocation.bottomEnd:
......@@ -181,8 +197,8 @@ class BannerPainter extends CustomPainter {
double get _rotation {
assert(location != null);
assert(textDirection != null);
switch (textDirection) {
assert(layoutDirection != null);
switch (layoutDirection) {
case TextDirection.rtl:
switch (location) {
case BannerLocation.bottomStart:
......@@ -215,7 +231,8 @@ class BannerPainter extends CustomPainter {
///
/// See also:
///
/// * [CheckedModeBanner].
/// * [CheckedModeBanner], which the [WidgetsApp] widget includes by default in
/// debug mode, to show a banner that says "SLOW MODE".
class Banner extends StatelessWidget {
/// Creates a banner.
///
......@@ -226,6 +243,7 @@ class Banner extends StatelessWidget {
@required this.message,
this.textDirection,
@required this.location,
this.layoutDirection,
this.color: _kColor,
this.textStyle: _kTextStyle,
}) : assert(message != null),
......@@ -250,11 +268,24 @@ class Banner extends StatelessWidget {
/// its left.
///
/// Defaults to the ambient [Directionality], if any.
///
/// See also [layoutDirection], which controls the interpretation of the
/// [location].
final TextDirection textDirection;
/// Where to show the banner (e.g., the upper right corder).
final BannerLocation location;
/// The directionality of the layout.
///
/// This is used to resolve the [location] values.
///
/// Defaults to the ambient [Directionality], if any.
///
/// See also [textDirection], which controls the reading direction of the
/// [message].
final TextDirection layoutDirection;
/// The color of the banner.
final Color color;
......@@ -268,6 +299,7 @@ class Banner extends StatelessWidget {
message: message,
textDirection: textDirection ?? Directionality.of(context),
location: location,
layoutDirection: layoutDirection ?? Directionality.of(context),
color: color,
textStyle: textStyle,
),
......@@ -281,6 +313,7 @@ class Banner extends StatelessWidget {
description.add(new StringProperty('message', message, showName: false));
description.add(new EnumProperty<TextDirection>('textDirection', textDirection, defaultValue: null));
description.add(new EnumProperty<BannerLocation>('location', location));
description.add(new EnumProperty<TextDirection>('layoutDirection', layoutDirection, defaultValue: null));
description.add(new DiagnosticsProperty<Color>('color', color, showName: false));
textStyle?.debugFillProperties(description, prefix: 'text ');
}
......
......@@ -2,8 +2,34 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
/// An [Invocation] and the [stack] trace that led to it.
///
/// Used by [TestRecordingCanvas] to trace canvas calls.
class RecordedInvocation {
/// Create a record for an invocation list.
const RecordedInvocation(this.invocation, { this.stack });
/// The method that was called and its arguments.
final Invocation invocation;
/// The stack trace at the time of the method call.
final StackTrace stack;
@override
String toString() => _describeInvocation(invocation);
/// Converts [stack] to a string using the [FlutterError.defaultStackFilter] logic.
String stackToString({ String indent: '' }) {
assert(indent != null);
return indent + FlutterError.defaultStackFilter(
stack.toString().trimRight().split('\n')
).join('\n$indent');
}
}
/// A [Canvas] for tests that records its method calls.
///
/// This class can be used in conjuction with [TestRecordingPaintingContext]
......@@ -25,7 +51,7 @@ import 'package:flutter/rendering.dart';
/// pattern matching API over [TestRecordingCanvas].
class TestRecordingCanvas implements Canvas {
/// All of the method calls on this canvas.
final List<Invocation> invocations = <Invocation>[];
final List<RecordedInvocation> invocations = <RecordedInvocation>[];
int _saveCount = 0;
......@@ -35,25 +61,25 @@ class TestRecordingCanvas implements Canvas {
@override
void save() {
_saveCount += 1;
invocations.add(new _MethodCall(#save));
invocations.add(new RecordedInvocation(new _MethodCall(#save), stack: StackTrace.current));
}
@override
void saveLayer(Rect bounds, Paint paint) {
_saveCount += 1;
invocations.add(new _MethodCall(#saveLayer, <dynamic>[bounds, paint]));
invocations.add(new RecordedInvocation(new _MethodCall(#saveLayer, <dynamic>[bounds, paint]), stack: StackTrace.current));
}
@override
void restore() {
_saveCount -= 1;
assert(_saveCount >= 0);
invocations.add(new _MethodCall(#restore));
invocations.add(new RecordedInvocation(new _MethodCall(#restore), stack: StackTrace.current));
}
@override
void noSuchMethod(Invocation invocation) {
invocations.add(invocation);
invocations.add(new RecordedInvocation(invocation, stack: StackTrace.current));
}
}
......@@ -101,3 +127,39 @@ class _MethodCall implements Invocation {
@override
List<dynamic> get positionalArguments => _arguments;
}
String _valueName(Object value) {
if (value is double)
return value.toStringAsFixed(1);
return value.toString();
}
// Workaround for https://github.com/dart-lang/sdk/issues/28372
String _symbolName(Symbol symbol) {
// WARNING: Assumes a fixed format for Symbol.toString which is *not*
// guaranteed anywhere.
final String s = '$symbol';
return s.substring(8, s.length - 2);
}
// Workaround for https://github.com/dart-lang/sdk/issues/28373
String _describeInvocation(Invocation call) {
final StringBuffer buffer = new StringBuffer();
buffer.write(_symbolName(call.memberName));
if (call.isSetter) {
buffer.write(call.positionalArguments[0].toString());
} else if (call.isMethod) {
buffer.write('(');
buffer.writeAll(call.positionalArguments.map<String>(_valueName), ', ');
String separator = call.positionalArguments.isEmpty ? '' : ', ';
call.namedArguments.forEach((Symbol name, Object value) {
buffer.write(separator);
buffer.write(_symbolName(name));
buffer.write(': ');
buffer.write(_valueName(value));
separator = ', ';
});
buffer.write(')');
}
return buffer.toString();
}
......@@ -27,15 +27,12 @@ class TestRoute<T> extends PageRoute<T> {
Future<Null> pumpApp(WidgetTester tester) async {
await tester.pumpWidget(
new Directionality(
textDirection: TextDirection.ltr,
child: new WidgetsApp(
new WidgetsApp(
color: const Color(0xFF333333),
onGenerateRoute: (RouteSettings settings) {
return new TestRoute<Null>(settings: settings, child: new Container());
},
),
),
);
}
......
......@@ -4,8 +4,10 @@
import 'dart:math' as math;
import 'package:flutter/widgets.dart';
import 'package:test/test.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import '../rendering/mock_canvas.dart';
class TestCanvas implements Canvas {
final List<Invocation> invocations = <Invocation>[];
......@@ -17,11 +19,16 @@ class TestCanvas implements Canvas {
}
void main() {
// the textDirection values below are intentionally sometimes different and
// sometimes the same as the layoutDirection, to make sure that they don't
// affect the layout.
test('A Banner with a location of topStart paints in the top left (LTR)', () {
final BannerPainter bannerPainter = new BannerPainter(
message: 'foo',
textDirection: TextDirection.ltr,
location: BannerLocation.topStart
textDirection: TextDirection.rtl,
location: BannerLocation.topStart,
layoutDirection: TextDirection.ltr,
);
final TestCanvas canvas = new TestCanvas();
......@@ -47,8 +54,9 @@ void main() {
test('A Banner with a location of topStart paints in the top right (RTL)', () {
final BannerPainter bannerPainter = new BannerPainter(
message: 'foo',
textDirection: TextDirection.rtl,
textDirection: TextDirection.ltr,
location: BannerLocation.topStart,
layoutDirection: TextDirection.rtl,
);
final TestCanvas canvas = new TestCanvas();
......@@ -75,7 +83,8 @@ void main() {
final BannerPainter bannerPainter = new BannerPainter(
message: 'foo',
textDirection: TextDirection.ltr,
location: BannerLocation.topEnd
location: BannerLocation.topEnd,
layoutDirection: TextDirection.ltr,
);
final TestCanvas canvas = new TestCanvas();
......@@ -103,6 +112,7 @@ void main() {
message: 'foo',
textDirection: TextDirection.rtl,
location: BannerLocation.topEnd,
layoutDirection: TextDirection.rtl,
);
final TestCanvas canvas = new TestCanvas();
......@@ -129,7 +139,8 @@ void main() {
final BannerPainter bannerPainter = new BannerPainter(
message: 'foo',
textDirection: TextDirection.ltr,
location: BannerLocation.bottomStart
location: BannerLocation.bottomStart,
layoutDirection: TextDirection.ltr,
);
final TestCanvas canvas = new TestCanvas();
......@@ -157,6 +168,7 @@ void main() {
message: 'foo',
textDirection: TextDirection.rtl,
location: BannerLocation.bottomStart,
layoutDirection: TextDirection.rtl,
);
final TestCanvas canvas = new TestCanvas();
......@@ -182,8 +194,9 @@ void main() {
test('A Banner with a location of bottomEnd paints in the bottom right (LTR)', () {
final BannerPainter bannerPainter = new BannerPainter(
message: 'foo',
textDirection: TextDirection.ltr,
location: BannerLocation.bottomEnd
textDirection: TextDirection.rtl,
location: BannerLocation.bottomEnd,
layoutDirection: TextDirection.ltr,
);
final TestCanvas canvas = new TestCanvas();
......@@ -209,8 +222,9 @@ void main() {
test('A Banner with a location of bottomEnd paints in the bottom left (RTL)', () {
final BannerPainter bannerPainter = new BannerPainter(
message: 'foo',
textDirection: TextDirection.rtl,
textDirection: TextDirection.ltr,
location: BannerLocation.bottomEnd,
layoutDirection: TextDirection.rtl,
);
final TestCanvas canvas = new TestCanvas();
......@@ -233,4 +247,34 @@ void main() {
expect(rotateCommand.positionalArguments[0], equals(math.PI / 4.0));
});
testWidgets('Banner widget', (WidgetTester tester) async {
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: const Banner(message: 'Hello', location: BannerLocation.topEnd),
),
);
expect(find.byType(CustomPaint), paints
..save
..translate(x: 800.0, y: 0.0)
..rotate(angle: math.PI / 4.0)
..rect(rect: new Rect.fromLTRB(-40.0, 28.0, 40.0, 40.0), color: const Color(0x7f000000), hasMaskFilter: true)
..rect(rect: new Rect.fromLTRB(-40.0, 28.0, 40.0, 40.0), color: const Color(0xa0b71c1c), hasMaskFilter: false)
..paragraph(offset: const Offset(-40.0, 29.0))
..restore
);
});
testWidgets('Banner widget in MaterialApp', (WidgetTester tester) async {
await tester.pumpWidget(new MaterialApp(home: const Placeholder()));
expect(find.byType(CheckedModeBanner), paints
..save
..translate(x: 800.0, y: 0.0)
..rotate(angle: math.PI / 4.0)
..rect(rect: new Rect.fromLTRB(-40.0, 28.0, 40.0, 40.0), color: const Color(0x7f000000), hasMaskFilter: true)
..rect(rect: new Rect.fromLTRB(-40.0, 28.0, 40.0, 40.0), color: const Color(0xa0b71c1c), hasMaskFilter: false)
..paragraph(offset: const Offset(-40.0, 24.0))
..restore
);
});
}
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