Merge pull request #1852 from Hixie/stock-icons

Fix color of icons in drawers in dark theme.

examples/stocks/test/icon_color_test.dart

@@ -0,0 +1,109 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:ui' as ui show window;
+
+import 'package:test/test.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter/rendering.dart';
+import 'package:stocks/main.dart' as stocks;
+import 'package:stocks/stock_data.dart' as stock_data;
+
+Element findElementOfExactWidgetTypeGoingDown(Element node, Type targetType) {
+  void walker(Element child) {
+    if (child.widget.runtimeType == targetType)
+      throw child;
+    child.visitChildElements(walker);
+  }
+  try {
+    walker(node);
+  } on Element catch (result) {
+    return result;
+  }
+  return null;
+}
+
+Element findElementOfExactWidgetTypeGoingUp(Element node, Type targetType) {
+  Element result;
+  bool walker(Element ancestor) {
+    if (ancestor.widget.runtimeType == targetType)
+      result = ancestor;
+    return result == null;
+  }
+  node.visitAncestorElements(walker);
+  return result;
+}
+
+final RegExp materialIconAssetNameColorExtractor = new RegExp(r'[^/]+/ic_.+_(white|black)_[0-9]+dp\.png');
+
+void checkIconColor(WidgetTester tester, String label, Color color) {
+  // The icon is going to be in the same merged semantics box as the text
+  // regardless of how the menu item is represented, so this is a good
+  // way to find the menu item. I hope.
+  Element semantics = findElementOfExactWidgetTypeGoingUp(tester.findText(label), MergeSemantics);
+  expect(semantics, isNotNull);
+  Element asset = findElementOfExactWidgetTypeGoingDown(semantics, AssetImage);
+  RenderImage imageBox = asset.findRenderObject();
+  Match match = materialIconAssetNameColorExtractor.firstMatch(asset.widget.name);
+  expect(match, isNotNull);
+  if (color == const Color(0xFFFFFFFF)) {
+    expect(match[1], equals('white'));
+    expect(imageBox.color, isNull);
+  } else if (color == const Color(0xFF000000)) {
+    expect(match[1], equals('black'));
+    expect(imageBox.color, isNull);
+  } else {
+    expect(imageBox.color, equals(color));
+  }
+}
+
+void main() {
+  stock_data.StockDataFetcher.actuallyFetchData = false;
+
+  test("Test icon colors", () {
+    testWidgets((WidgetTester tester) {
+      stocks.main(); // builds the app and schedules a frame but doesn't trigger one
+      tester.pump(); // see https://github.com/flutter/flutter/issues/1865
+      tester.pump(); // triggers a frame
+
+      // sanity check
+      expect(tester.findText('MARKET'), isNotNull);
+      expect(tester.findText('Help & Feedback'), isNull);
+      tester.pump(new Duration(seconds: 2));
+      expect(tester.findText('MARKET'), isNotNull);
+      expect(tester.findText('Help & Feedback'), isNull);
+
+      // drag the drawer out
+      TestPointer pointer = new TestPointer(1);
+      Point left = new Point(0.0, ui.window.size.height / 2.0);
+      Point right = new Point(ui.window.size.width, left.y);
+      tester.dispatchEvent(pointer.down(left), left);
+      tester.pump();
+      tester.dispatchEvent(pointer.move(right), left);
+      tester.pump();
+      tester.dispatchEvent(pointer.up(), left);
+      tester.pump();
+      expect(tester.findText('MARKET'), isNotNull);
+      expect(tester.findText('Help & Feedback'), isNotNull);
+
+      // check the colour of the icon - light mode
+      checkIconColor(tester, 'Stock List', Colors.purple[500]); // theme primary color
+      checkIconColor(tester, 'Account Balance', Colors.black45); // enabled
+      checkIconColor(tester, 'Help & Feedback', Colors.black26); // disabled
+
+      // switch to dark mode
+      tester.tap(tester.findText('Pessimistic'));
+      tester.pump(); // get the tap and send the notification that the theme has changed
+      tester.pump(); // start the theme transition
+      tester.pump(const Duration(seconds: 5)); // end the transition
+
+      // check the colour of the icon - dark mode
+      checkIconColor(tester, 'Stock List', Colors.redAccent[200]); // theme accent color
+      checkIconColor(tester, 'Account Balance', Colors.white); // enabled
+      checkIconColor(tester, 'Help & Feedback', Colors.white30); // disabled
+
+    });
+  });
+}

examples/stocks/test/locale_test.dart

@@ -1,3 +1,6 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
 
 import 'package:test/test.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -11,7 +14,8 @@ void main() {
   test("Test changing locale", () {
     testWidgets((WidgetTester tester) {
       stocks.main();
-      tester.pump(const Duration(seconds: 1)); // Unclear why duration is required.
+      tester.async.flushMicrotasks(); // see https://github.com/flutter/flutter/issues/1865
+      tester.pump();
 
       Element<Text> tab = tester.findText('MARKET');
       expect(tab, isNotNull);

packages/flutter/lib/src/material/drawer_item.dart

@@ -24,21 +24,31 @@ class DrawerItem extends StatelessComponent {
   final bool selected;
 
   Color _getIconColor(ThemeData themeData) {
-    if (selected) {
-      if (themeData.brightness == ThemeBrightness.dark)
-        return themeData.accentColor;
-      return themeData.primaryColor;
+    switch (themeData.brightness) {
+      case ThemeBrightness.light:
+        if (selected)
+          return themeData.primaryColor;
+        if (onPressed == null)
+          return Colors.black26;
+        return Colors.black45;
+      case ThemeBrightness.dark:
+        if (selected)
+          return themeData.accentColor;
+        if (onPressed == null)
+          return Colors.white30;
+        return null; // use default icon theme colour unmodified
     }
-    return Colors.black45;
   }
 
   TextStyle _getTextStyle(ThemeData themeData) {
     TextStyle result = themeData.text.body2;
     if (selected) {
-      if (themeData.brightness == ThemeBrightness.dark)
-        result = result.copyWith(color: themeData.accentColor);
-      else
-        result = result.copyWith(color: themeData.primaryColor);
+      switch (themeData.brightness) {
+        case ThemeBrightness.light:
+          return result.copyWith(color: themeData.primaryColor);
+        case ThemeBrightness.dark:
+          return result.copyWith(color: themeData.accentColor);
+      }
     }
     return result;
   }

packages/flutter/lib/src/material/icon.dart

@@ -77,9 +77,9 @@ class Icon extends StatelessComponent {
     Color iconColor = color;
     final int iconAlpha = (255.0 * (IconTheme.of(context)?.clampedOpacity ?? 1.0)).round();
     if (iconAlpha != 255) {
-      if (color != null)
+      if (color != null) {
         iconColor = color.withAlpha(iconAlpha);
-      else {
+      } else {
         switch(iconThemeColor) {
           case IconThemeColor.black:
             iconColor = Colors.black.withAlpha(iconAlpha);
@@ -103,5 +103,9 @@ class Icon extends StatelessComponent {
     super.debugFillDescription(description);
     description.add('$icon');
     description.add('size: $size');
+    if (this.colorTheme != null)
+      description.add('colorTheme: $colorTheme');
+    if (this.color != null)
+      description.add('color: $color');
   }
 }

packages/flutter/lib/src/rendering/binding.dart

@@ -111,7 +111,7 @@ void debugDumpLayerTree() {
 /// This will only work if there is a semantics client attached.
 /// Otherwise, the tree is empty and this will print "null".
 void debugDumpSemanticsTree() {
-  debugPrint(Renderer.instance?.renderView?.debugSemantics?.toStringDeep());
+  debugPrint(Renderer.instance?.renderView?.debugSemantics?.toStringDeep() ?? 'Semantics not collected.');
 }
 
 /// A concrete binding for applications that use the Rendering framework

packages/flutter/lib/src/rendering/image.dart

@@ -251,7 +251,25 @@ class RenderImage extends RenderBox {
 
   void debugDescribeSettings(List<String> settings) {
     super.debugDescribeSettings(settings);
-    settings.add('width: $width');
-    settings.add('height: $height');
+    if (image != null)
+      settings.add('image: [${image.width}\u00D7${image.height}]');
+    else
+      settings.add('image: null');
+    if (width != null)
+      settings.add('width: $width');
+    if (height != null)
+      settings.add('height: $height');
+    if (scale != 1.0)
+      settings.add('scale: $scale');
+    if (color != null)
+      settings.add('color: $color');
+    if (fit != null)
+      settings.add('fit: $fit');
+    if (alignment != null)
+      settings.add('alignment: $alignment');
+    if (repeat != ImageRepeat.noRepeat)
+      settings.add('repeat: $repeat');
+    if (centerSlice != null)
+      settings.add('centerSlice: $centerSlice');
   }
 }

packages/flutter/lib/src/rendering/object.dart

@@ -1712,6 +1712,23 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
     return result;
   }
 
+  /// Returns a one-line detailed description of the render object.
+  /// This description is often somewhat long.
+  ///
+  /// This includes the same information for this RenderObject as given by
+  /// [toStringDeep()], but does not recurse to any children.
+  String toStringShallow() {
+    RenderObject debugPreviousActiveLayout = _debugActiveLayout;
+    _debugActiveLayout = null;
+    StringBuffer result = new StringBuffer();
+    result.write('$this; ');
+    List<String> settings = <String>[];
+    debugDescribeSettings(settings);
+    result.write(settings.join('; '));
+    _debugActiveLayout = debugPreviousActiveLayout;
+    return result.toString();
+  }
+
   /// Returns a list of strings describing the current node's fields, one field
   /// per string. Subclasses should override this to have their information
   /// included in toStringDeep().

packages/flutter/lib/src/services/asset_bundle.dart

@@ -21,6 +21,7 @@ abstract class AssetBundle {
   ImageResource loadImage(String key);
   Future<String> loadString(String key);
   Future<core.MojoDataPipeConsumer> load(String key);
+  String toString() => '$runtimeType@$hashCode()';
 }
 
 class NetworkAssetBundle extends AssetBundle {
@@ -39,6 +40,8 @@ class NetworkAssetBundle extends AssetBundle {
   Future<String> loadString(String key) async {
     return (await http.get(_urlFromKey(key))).body;
   }
+
+  String toString() => '$runtimeType@$hashCode($_baseUrl)';
 }
 
 abstract class CachingAssetBundle extends AssetBundle {

packages/flutter/lib/src/services/image_resource.dart

@@ -11,6 +11,7 @@ class ImageInfo {
   ImageInfo({ this.image, this.scale: 1.0 });
   final ui.Image image;
   final double scale;
+  String toString() => '[${image.width}\u00D7${image.height}] @ ${scale}x';
 }
 
 /// A callback for when the image is available.
@@ -82,4 +83,16 @@ class ImageResource {
     debugPrint('$stack');
     debugPrint('------------------------------------------------------------------------');
   }
+
+  String toString() {
+    StringBuffer result = new StringBuffer();
+    result.write('$runtimeType(');
+    if (!_resolved)
+      result.write('unresolved');
+    else
+      result.write('$_image');
+    result.write('; ${_listeners.length} listener${_listeners.length == 1 ? "" : "s" }');
+    result.write(')');
+    return result.toString();
+  }
 }

packages/flutter/lib/src/widgets/asset_vendor.dart

@@ -191,14 +191,20 @@ class AssetVendor extends StatefulComponent {
   final Widget child;
 
   _AssetVendorState createState() => new _AssetVendorState();
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('bundle: $bundle');
+    if (devicePixelRatio != null)
+      description.add('devicePixelRatio: $devicePixelRatio');
+  }
 }
 
 class _AssetVendorState extends State<AssetVendor> {
 
   _ResolvingAssetBundle _bundle;
 
-  void initState() {
-    super.initState();
+  void _initBundle() {
     _bundle = new _ResolutionAwareAssetBundle(
       bundle: config.bundle,
       resolver: new _ResolutionAwareAssetResolver(
@@ -208,20 +214,24 @@ class _AssetVendorState extends State<AssetVendor> {
     );
   }
 
+  void initState() {
+    super.initState();
+    _initBundle();
+  }
+
   void didUpdateConfig(AssetVendor oldConfig) {
     if (config.bundle != oldConfig.bundle ||
         config.devicePixelRatio != oldConfig.devicePixelRatio) {
-      _bundle = new _ResolutionAwareAssetBundle(
-        bundle: config.bundle,
-        resolver: new _ResolutionAwareAssetResolver(
-          bundle: config.bundle,
-          devicePixelRatio: config.devicePixelRatio
-        )
-      );
+      _initBundle();
     }
   }
 
   Widget build(BuildContext context) {
     return new DefaultAssetBundle(bundle: _bundle, child: config.child);
   }
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('bundle: $_bundle');
+  }
 }

packages/flutter/lib/src/widgets/basic.dart

@@ -1576,7 +1576,8 @@ class RawImage extends LeafRenderObjectWidget {
     fit: fit,
     alignment: alignment,
     repeat: repeat,
-    centerSlice: centerSlice);
+    centerSlice: centerSlice
+  );
 
   void updateRenderObject(RenderImage renderObject, RawImage oldWidget) {
     renderObject.image = image;
@@ -1589,6 +1590,30 @@ class RawImage extends LeafRenderObjectWidget {
     renderObject.repeat = repeat;
     renderObject.centerSlice = centerSlice;
   }
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    if (image != null)
+      description.add('image: [${image.width}\u00D7${image.height}]');
+    else
+      description.add('image: null');
+    if (width != null)
+      description.add('width: $width');
+    if (height != null)
+      description.add('height: $height');
+    if (scale != 1.0)
+      description.add('scale: $scale');
+    if (color != null)
+      description.add('color: $color');
+    if (fit != null)
+      description.add('fit: $fit');
+    if (alignment != null)
+      description.add('alignment: $alignment');
+    if (repeat != ImageRepeat.noRepeat)
+      description.add('repeat: $repeat');
+    if (centerSlice != null)
+      description.add('centerSlice: $centerSlice');
+  }
 }
 
 /// Displays an [ImageResource].
@@ -1655,10 +1680,29 @@ class RawImageResource extends StatefulComponent {
   /// the center slice will be stretched only vertically.
   final Rect centerSlice;
 
-  _ImageListenerState createState() => new _ImageListenerState();
+  _RawImageResourceState createState() => new _RawImageResourceState();
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('image: $image');
+    if (width != null)
+      description.add('width: $width');
+    if (height != null)
+      description.add('height: $height');
+    if (color != null)
+      description.add('color: $color');
+    if (fit != null)
+      description.add('fit: $fit');
+    if (alignment != null)
+      description.add('alignment: $alignment');
+    if (repeat != ImageRepeat.noRepeat)
+      description.add('repeat: $repeat');
+    if (centerSlice != null)
+      description.add('centerSlice: $centerSlice');
+  }
 }
 
-class _ImageListenerState extends State<RawImageResource> {
+class _RawImageResourceState extends State<RawImageResource> {
   void initState() {
     super.initState();
     config.image.addListener(_handleImageChanged);
@@ -1771,6 +1815,27 @@ class NetworkImage extends StatelessComponent {
       centerSlice: centerSlice
     );
   }
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('src: $src');
+    if (width != null)
+      description.add('width: $width');
+    if (height != null)
+      description.add('height: $height');
+    if (scale != 1.0)
+      description.add('scale: $scale');
+    if (color != null)
+      description.add('color: $color');
+    if (fit != null)
+      description.add('fit: $fit');
+    if (alignment != null)
+      description.add('alignment: $alignment');
+    if (repeat != ImageRepeat.noRepeat)
+      description.add('repeat: $repeat');
+    if (centerSlice != null)
+      description.add('centerSlice: $centerSlice');
+  }
 }
 
 /// Sets a default asset bundle for its descendants.
@@ -1869,6 +1934,25 @@ class AsyncImage extends StatelessComponent {
       centerSlice: centerSlice
     );
   }
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('provider: $provider');
+    if (width != null)
+      description.add('width: $width');
+    if (height != null)
+      description.add('height: $height');
+    if (color != null)
+      description.add('color: $color');
+    if (fit != null)
+      description.add('fit: $fit');
+    if (alignment != null)
+      description.add('alignment: $alignment');
+    if (repeat != ImageRepeat.noRepeat)
+      description.add('repeat: $repeat');
+    if (centerSlice != null)
+      description.add('centerSlice: $centerSlice');
+  }
 }
 
 /// Displays an image from an [AssetBundle].
@@ -1949,6 +2033,27 @@ class AssetImage extends StatelessComponent {
       centerSlice: centerSlice
     );
   }
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('name: $name');
+    if (width != null)
+      description.add('width: $width');
+    if (height != null)
+      description.add('height: $height');
+    if (color != null)
+      description.add('color: $color');
+    if (fit != null)
+      description.add('fit: $fit');
+    if (alignment != null)
+      description.add('alignment: $alignment');
+    if (repeat != ImageRepeat.noRepeat)
+      description.add('repeat: $repeat');
+    if (centerSlice != null)
+      description.add('centerSlice: $centerSlice');
+    if (bundle != null)
+      description.add('bundle: $bundle');
+  }
 }
 
 /// An adapter for placing a specific [RenderBox] in the widget tree.

packages/flutter_test/lib/src/widget_tester.dart

@@ -29,17 +29,26 @@ class WidgetTester extends Instrumentation {
   final FakeAsync async;
   final Clock clock;
 
+  /// Calls [runApp()] with the given widget, then triggers a frame sequent and
+  /// flushes microtasks, by calling [pump()] with the same duration (if any).
   void pumpWidget(Widget widget, [ Duration duration ]) {
     runApp(widget);
     pump(duration);
   }
 
+  /// Artificially calls dispatchLocaleChanged on the Widget binding,
+  /// then flushes microtasks.
   void setLocale(String languageCode, String countryCode) {
     Locale locale = new Locale(languageCode, countryCode);
     binding.dispatchLocaleChanged(locale);
     async.flushMicrotasks();
   }
 
+  /// Triggers a frame sequence (build/layout/paint/etc),
+  /// then flushes microtasks.
+  ///
+  /// If duration is set, then advances the clock by that much first.
+  /// Doing this flushes microtasks.
   void pump([ Duration duration ]) {
     if (duration != null)
       async.elapse(duration);