From a5b76edf7991bab0ccdd7a75bbc4411d341b6669 Mon Sep 17 00:00:00 2001 From: Kenzie Davisson Date: Thu, 6 Mar 2025 08:43:33 -0800 Subject: [PATCH 1/3] Add support for universal banner messages --- .../src/shared/managers/banner_messages.dart | 24 ++++-- .../shared/managers/banner_messages_test.dart | 80 +++++++++++++++---- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/managers/banner_messages.dart b/packages/devtools_app/lib/src/shared/managers/banner_messages.dart index bef002b240d..0636ff6727b 100644 --- a/packages/devtools_app/lib/src/shared/managers/banner_messages.dart +++ b/packages/devtools_app/lib/src/shared/managers/banner_messages.dart @@ -23,6 +23,13 @@ const _runInProfileModeDocsUrl = 'https://flutter.dev/to/use-profile-mode'; const _cpuSamplingRateDocsUrl = 'https://docs.flutter.dev/tools/devtools/cpu-profiler#cpu-sampling-rate'; +/// Screen id to use for banner messages that are intended to be universal for +/// every DevTools screen. +/// +/// Messages with this screen id will be added to the list of messages for +/// every screen from the [BannerMessages] widget. +const universalBannerMessageId = 'universal'; + class BannerMessagesController { final _messages = >{}; final _dismissedMessageKeys = {}; @@ -77,13 +84,13 @@ class BannerMessagesController { }); } - void removeMessageByKey(Key key, String screenId) { + void removeMessageByKey(Key key, String screenId, {bool dismiss = false}) { final currentMessages = _messagesForScreen(screenId); final messageWithKey = currentMessages.value.firstWhereOrNull( (m) => m.key == key, ); if (messageWithKey != null) { - removeMessage(messageWithKey); + removeMessage(messageWithKey, dismiss: dismiss); } } @@ -119,13 +126,18 @@ class BannerMessages extends StatelessWidget { // TODO(kenz): use an AnimatedList for message changes. @override Widget build(BuildContext context) { + final universalMessages = bannerMessages.messagesForScreen( + universalBannerMessageId, + ); final messagesForScreen = bannerMessages.messagesForScreen(screen.screenId); return Column( children: [ - ValueListenableBuilder>( - valueListenable: messagesForScreen, - builder: (context, messages, _) { - return Column(children: messages); + MultiValueListenableBuilder( + listenables: [universalMessages, messagesForScreen], + builder: (context, values, _) { + final universalMessages = values[0] as List; + final messages = values[1] as List; + return Column(children: [...universalMessages, ...messages]); }, ), Expanded(child: screen.build(context)), diff --git a/packages/devtools_app/test/shared/managers/banner_messages_test.dart b/packages/devtools_app/test/shared/managers/banner_messages_test.dart index 5080ac3851f..4441113e9dd 100644 --- a/packages/devtools_app/test/shared/managers/banner_messages_test.dart +++ b/packages/devtools_app/test/shared/managers/banner_messages_test.dart @@ -2,11 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. +import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/framework/scaffold/scaffold.dart'; -import 'package:devtools_app/src/service/service_manager.dart'; -import 'package:devtools_app/src/shared/globals.dart'; -import 'package:devtools_app/src/shared/managers/banner_messages.dart'; -import 'package:devtools_app/src/shared/managers/notifications.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; import 'package:devtools_test/devtools_test.dart'; @@ -34,20 +31,25 @@ void main() { await tester.pumpAndSettle(); } - Widget buildBannerMessages() { + Widget buildBannerMessages({Screen? screen}) { return wrap( Directionality( textDirection: TextDirection.ltr, child: BannerMessages( - screen: SimpleScreen( - Column( - children: [ - // This is button is present so that we can tap it and - // simulate a frame being drawn. - ElevatedButton(onPressed: () => {}, child: const SizedBox()), - ], - ), - ), + screen: + screen ?? + SimpleScreen( + Column( + children: [ + // This is button is present so that we can tap it and + // simulate a frame being drawn. + ElevatedButton( + onPressed: () => {}, + child: const SizedBox(), + ), + ], + ), + ), ), ), ); @@ -66,6 +68,30 @@ void main() { expect(find.byKey(k2), findsOneWidget); }); + testWidgets('displays universal banner messages for every screen', ( + WidgetTester tester, + ) async { + await tester.pumpWidget(buildBannerMessages()); + bannerMessages.addMessage(universalMessage); + await pumpTestFrame(tester); + expect(find.byKey(kUniversal), findsOneWidget); + + await tester.pumpWidget( + buildBannerMessages( + screen: TestScreen( + Column( + children: [ + // This is button is present so that we can tap it and + // simulate a frame being drawn. + ElevatedButton(onPressed: () => {}, child: const SizedBox()), + ], + ), + ), + ), + ); + expect(find.byKey(kUniversal), findsOneWidget); + }); + testWidgets('does not add duplicate messages', (WidgetTester tester) async { await tester.pumpWidget(buildBannerMessages()); expect(find.byKey(k1), findsNothing); @@ -164,17 +190,43 @@ void main() { final testMessage1ScreenId = SimpleScreen.id; final testMessage2ScreenId = SimpleScreen.id; + const k1 = Key('test message 1'); const k2 = Key('test message 2'); +const kUniversal = Key('universal message'); + final testMessage1 = BannerMessage( key: k1, buildTextSpans: (_) => const [TextSpan(text: 'Test Message 1')], screenId: testMessage1ScreenId, messageType: BannerMessageType.warning, ); + final testMessage2 = BannerMessage( key: k2, buildTextSpans: (_) => const [TextSpan(text: 'Test Message 2')], screenId: testMessage2ScreenId, messageType: BannerMessageType.warning, ); + +final universalMessage = BannerMessage( + key: kUniversal, + buildTextSpans: (_) => const [TextSpan(text: 'Universal Message')], + screenId: universalBannerMessageId, + messageType: BannerMessageType.warning, +); + +class TestScreen extends Screen { + TestScreen(this.child) : super(id, showFloatingDebuggerControls: false); + + // This is arbitrary for the test. It just needs to be something different + // than [ScreenMetaData.simple.id]. + static final id = ScreenMetaData.logging.id; + + final Widget child; + + @override + Widget buildScreenBody(BuildContext context) { + return child; + } +} From 1655ec1fddf177d7e2d56c10afdc62c528bcfe8c Mon Sep 17 00:00:00 2001 From: Kenzie Davisson Date: Thu, 6 Mar 2025 09:07:00 -0800 Subject: [PATCH 2/3] formatting --- .../devtools_app/lib/src/shared/managers/banner_messages.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/lib/src/shared/managers/banner_messages.dart b/packages/devtools_app/lib/src/shared/managers/banner_messages.dart index 0636ff6727b..8b6a49da5e2 100644 --- a/packages/devtools_app/lib/src/shared/managers/banner_messages.dart +++ b/packages/devtools_app/lib/src/shared/managers/banner_messages.dart @@ -25,7 +25,7 @@ const _cpuSamplingRateDocsUrl = /// Screen id to use for banner messages that are intended to be universal for /// every DevTools screen. -/// +/// /// Messages with this screen id will be added to the list of messages for /// every screen from the [BannerMessages] widget. const universalBannerMessageId = 'universal'; From b399b7cc90e5c62f88346fae603d3c84a1cddf3d Mon Sep 17 00:00:00 2001 From: Kenzie Davisson Date: Thu, 6 Mar 2025 09:46:43 -0800 Subject: [PATCH 3/3] review comments --- .../shared/managers/banner_messages_test.dart | 50 +++++++------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/packages/devtools_app/test/shared/managers/banner_messages_test.dart b/packages/devtools_app/test/shared/managers/banner_messages_test.dart index 4441113e9dd..14eb3e85b64 100644 --- a/packages/devtools_app/test/shared/managers/banner_messages_test.dart +++ b/packages/devtools_app/test/shared/managers/banner_messages_test.dart @@ -36,20 +36,7 @@ void main() { Directionality( textDirection: TextDirection.ltr, child: BannerMessages( - screen: - screen ?? - SimpleScreen( - Column( - children: [ - // This is button is present so that we can tap it and - // simulate a frame being drawn. - ElevatedButton( - onPressed: () => {}, - child: const SizedBox(), - ), - ], - ), - ), + screen: screen ?? SimpleScreen(const _TestScreenBody()), ), ), ); @@ -76,19 +63,7 @@ void main() { await pumpTestFrame(tester); expect(find.byKey(kUniversal), findsOneWidget); - await tester.pumpWidget( - buildBannerMessages( - screen: TestScreen( - Column( - children: [ - // This is button is present so that we can tap it and - // simulate a frame being drawn. - ElevatedButton(onPressed: () => {}, child: const SizedBox()), - ], - ), - ), - ), - ); + await tester.pumpWidget(buildBannerMessages(screen: TestScreen())); expect(find.byKey(kUniversal), findsOneWidget); }); @@ -217,16 +192,29 @@ final universalMessage = BannerMessage( ); class TestScreen extends Screen { - TestScreen(this.child) : super(id, showFloatingDebuggerControls: false); + TestScreen() : super(id, showFloatingDebuggerControls: false); // This is arbitrary for the test. It just needs to be something different // than [ScreenMetaData.simple.id]. static final id = ScreenMetaData.logging.id; - final Widget child; - @override Widget buildScreenBody(BuildContext context) { - return child; + return const _TestScreenBody(); + } +} + +class _TestScreenBody extends StatelessWidget { + const _TestScreenBody(); + + @override + Widget build(BuildContext context) { + return Column( + children: [ + // This button is present so that we can tap it and + // simulate a frame being drawn. + ElevatedButton(onPressed: () => {}, child: const SizedBox()), + ], + ); } }