From cf87f68fd08d697273c780d09fc647616749e8b1 Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Fri, 23 Aug 2019 09:41:03 -0700 Subject: [PATCH] Correct InheritedTheme.captureAll() for multiple theme ancestors of the same type (#39089) --- .../lib/src/widgets/inherited_theme.dart | 10 +- .../test/widgets/inherited_theme_test.dart | 105 ++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/packages/flutter/lib/src/widgets/inherited_theme.dart b/packages/flutter/lib/src/widgets/inherited_theme.dart index 75d07d4da6..0704f3d360 100644 --- a/packages/flutter/lib/src/widgets/inherited_theme.dart +++ b/packages/flutter/lib/src/widgets/inherited_theme.dart @@ -114,10 +114,18 @@ abstract class InheritedTheme extends InheritedWidget { assert(context != null); final List themes = []; + final Set themeTypes = {}; context.visitAncestorElements((Element ancestor) { if (ancestor is InheritedElement && ancestor.widget is InheritedTheme) { final InheritedTheme theme = ancestor.widget; - themes.add(theme); + final Type themeType = theme.runtimeType; + // Only remember the first theme of any type. This assumes + // that inherited themes completely shadow ancestors of the + // the same type. + if (!themeTypes.contains(themeType)) { + themeTypes.add(themeType); + themes.add(theme); + } } return true; }); diff --git a/packages/flutter/test/widgets/inherited_theme_test.dart b/packages/flutter/test/widgets/inherited_theme_test.dart index 1a56cfa764..e18b2ac045 100644 --- a/packages/flutter/test/widgets/inherited_theme_test.dart +++ b/packages/flutter/test/widgets/inherited_theme_test.dart @@ -144,6 +144,111 @@ void main() { expect(getTextStyle('Hello').fontSize, fontSize); expect(getIconStyle().color, iconColor); expect(getIconStyle().fontSize, iconSize); + }); + testWidgets('InheritedTheme.captureAll() multiple IconTheme ancestors', (WidgetTester tester) async { + // This is a regression test for https://github.com/flutter/flutter/issues/39087 + + const Color outerColor = Color(0xFF0000FF); + const Color innerColor = Color(0xFF00FF00); + const double iconSize = 48; + final Key icon1 = UniqueKey(); + final Key icon2 = UniqueKey(); + + await tester.pumpWidget( + WidgetsApp( + color: const Color(0xFFFFFFFF), + onGenerateRoute: (RouteSettings settings) { + return TestRoute( + IconTheme( + data: const IconThemeData(color: outerColor), + child: IconTheme( + data: const IconThemeData(size: iconSize, color: innerColor), + child: Center( + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + Icon(const IconData(0x41, fontFamily: 'Roboto'), key: icon1), + Builder( + builder: (BuildContext context) { + // The same IconThemes are visible from this context + // and the context that the widget returned by captureAll() + // is built in. So only the inner green IconTheme should + // apply to the icon, i.e. both icons will be big and green. + return InheritedTheme.captureAll( + context, + Icon(const IconData(0x41, fontFamily: 'Roboto'), key: icon2), + ); + }, + ), + ], + ), + ), + ), + ), + ); + }, + ) + ); + + TextStyle getIconStyle(Key key) { + return tester.widget( + find.descendant( + of: find.byKey(key), + matching: find.byType(RichText), + ), + ).text.style; + } + + expect(getIconStyle(icon1).color, innerColor); + expect(getIconStyle(icon1).fontSize, iconSize); + expect(getIconStyle(icon2).color, innerColor); + expect(getIconStyle(icon2).fontSize, iconSize); + }); + + testWidgets('InheritedTheme.captureAll() multiple DefaultTextStyle ancestors', (WidgetTester tester) async { + // This is a regression test for https://github.com/flutter/flutter/issues/39087 + + const Color textColor = Color(0xFF00FF00); + + await tester.pumpWidget( + WidgetsApp( + color: const Color(0xFFFFFFFF), + onGenerateRoute: (RouteSettings settings) { + return TestRoute( + DefaultTextStyle( + style: const TextStyle(fontSize: 48), + child: DefaultTextStyle( + style: const TextStyle(color: textColor), + child: Row( + children: [ + const Text('Hello'), + Builder( + builder: (BuildContext context) { + return InheritedTheme.captureAll(context, const Text('World')); + }, + ), + ], + ), + ), + ), + ); + }, + ), + ); + + TextStyle getTextStyle(String text) { + return tester.widget( + find.descendant( + of: find.text(text), + matching: find.byType(RichText), + ), + ).text.style; + } + + expect(getTextStyle('Hello').fontSize, null); + expect(getTextStyle('Hello').color, textColor); + expect(getTextStyle('World').fontSize, null); + expect(getTextStyle('World').color, textColor); }); }