From bebfe49bca7d5477af167580a84a200284370bb7 Mon Sep 17 00:00:00 2001 From: Kenzie Davisson <43759233+kenzieschmoll@users.noreply.github.com> Date: Fri, 21 Jun 2024 10:03:44 -0700 Subject: [PATCH] Show a screen unavailable message for disabled screens. (#7958) --- packages/devtools_app/lib/src/app.dart | 83 +++++++++++++++---- .../devtools_app/lib/src/shared/screen.dart | 53 +++++++++--- .../release_notes/NEXT_RELEASE_NOTES.md | 3 +- .../test/shared/visible_screens_test.dart | 2 +- .../lib/src/helpers/actions.dart | 2 +- 5 files changed, 115 insertions(+), 28 deletions(-) diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart index c8e24d6db5a..839d69da77b 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart @@ -230,8 +230,9 @@ class DevToolsAppState extends State with AutoDisposeMixin { child: DevToolsScaffold.withChild( key: const Key('not-found'), embedMode: params.embedMode, - child: PageNotFound( - page: page, + child: ScreenUnavailable( + title: "The '$page' cannot be found.", + embedMode: params.embedMode, routerDelegate: routerDelegate, ), ), @@ -279,7 +280,24 @@ class DevToolsAppState extends State with AutoDisposeMixin { removeHiddenScreens(screens, queryParams); DevToolsScaffold scaffold; - if (screens.isEmpty) { + + if (page != null && + ScreenMetaData.lookup(page) != null && + !screens.containsWhere((s) => s.screenId == page)) { + // The requested [page] is one of the first-party DevTools screens, + // but is not available in list of available screens for this + // scaffold. + scaffold = DevToolsScaffold.withChild( + key: const Key('screen-disabled'), + embedMode: embedMode, + child: ScreenUnavailable( + title: "The '$page' screen is unavailable.", + description: _screenUnavailableReason(page), + routerDelegate: routerDelegate, + embedMode: embedMode, + ), + ); + } else if (screens.isEmpty) { // TODO(https://github.com/dart-lang/pub-dev/issues/7216): add an // extensions store or a link to a pub.dev query for packages with // extensions. @@ -376,7 +394,8 @@ class DevToolsAppState extends State with AutoDisposeMixin { routerDelegate.refreshPages(); } - List _visibleScreens() => _screens.where(shouldShowScreen).toList(); + List _visibleScreens() => + _screens.where((screen) => shouldShowScreen(screen).show).toList(); List _providedControllers({bool offline = false}) { // We use [widget.originalScreens] here instead of [_screens] because @@ -475,6 +494,25 @@ class DevToolsAppState extends State with AutoDisposeMixin { screens.removeWhere((s) => s is! ExtensionScreen); } } + + String? _screenUnavailableReason(String screenId) { + final screenMetaData = ScreenMetaData.lookup(screenId); + String? disabledReason; + if (screenMetaData != null) { + final screen = + _originalScreens.firstWhere((s) => s.screenId == screenMetaData.id); + final reason = shouldShowScreen(screen).disabledReason; + if (reason == ScreenDisabledReason.requiresDartLibrary) { + // Special case for screens that require a library since the message + // needs to be generated dynamically. + disabledReason = 'The ${screen.title} screen requires library ' + '${screen.requiresLibrary}, but the library was not detected.'; + } else if (reason?.message != null) { + disabledReason = 'The ${screen.title} screen ${reason!.message!}'; + } + } + return disabledReason; + } } /// DevTools screen wrapper that is responsible for creating and providing the @@ -553,30 +591,45 @@ class _AlternateCheckedModeBanner extends StatelessWidget { } } -class PageNotFound extends StatelessWidget { - const PageNotFound({ +class ScreenUnavailable extends StatelessWidget { + const ScreenUnavailable({ super.key, - required this.page, + required this.title, + required this.embedMode, required this.routerDelegate, + this.description, }); - final String page; - + final String title; final DevToolsRouterDelegate routerDelegate; + final EmbedMode embedMode; + final String? description; @override Widget build(BuildContext context) { + final theme = Theme.of(context); return Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ - Text("'$page' not found."), - const SizedBox(height: defaultSpacing), - ElevatedButton( - onPressed: () => - routerDelegate.navigateHome(clearScreenParam: true), - child: const Text('Go to Home screen'), + Text( + title, + style: theme.textTheme.titleLarge, ), + const SizedBox(height: denseSpacing), + if (description != null) + Text( + description!, + style: theme.regularTextStyle, + ), + if (embedMode == EmbedMode.none) ...[ + const SizedBox(height: defaultSpacing), + ElevatedButton( + onPressed: () => + routerDelegate.navigateHome(clearScreenParam: true), + child: const Text('Go to Home screen'), + ), + ], ], ), ); diff --git a/packages/devtools_app/lib/src/shared/screen.dart b/packages/devtools_app/lib/src/shared/screen.dart index 9c444158749..2a3190296bb 100644 --- a/packages/devtools_app/lib/src/shared/screen.dart +++ b/packages/devtools_app/lib/src/shared/screen.dart @@ -448,11 +448,18 @@ abstract class Screen { } /// Check whether a screen should be shown in the UI. -bool shouldShowScreen(Screen screen) { +({bool show, ScreenDisabledReason? disabledReason}) shouldShowScreen( + Screen screen, +) { _log.finest('shouldShowScreen: ${screen.screenId}'); if (offlineDataController.showingOfflineData.value) { _log.finest('for offline mode: returning ${screen.worksWithOfflineData}'); - return screen.worksWithOfflineData; + return ( + show: screen.worksWithOfflineData, + disabledReason: screen.worksWithOfflineData + ? null + : ScreenDisabledReason.offlineDataNotSupported, + ); } final serviceReady = serviceConnection.serviceManager.isServiceAvailable && @@ -460,13 +467,16 @@ bool shouldShowScreen(Screen screen) { if (!serviceReady) { if (!screen.requiresConnection) { _log.finest('screen does not require connection: returning true'); - return true; + return (show: true, disabledReason: null); } else { // All of the following checks require a connected vm service, so verify // that one exists. This also avoids odd edge cases where we could show // screens while the ServiceManager is still initializing. _log.finest('service not ready: returning false'); - return false; + return ( + show: false, + disabledReason: ScreenDisabledReason.serviceNotReady, + ); } } @@ -478,36 +488,45 @@ bool shouldShowScreen(Screen screen) { _log.finest( 'screen requires library ${screen.requiresLibrary}: returning false', ); - return false; + return ( + show: false, + disabledReason: ScreenDisabledReason.requiresDartLibrary, + ); } } if (screen.requiresDartVm) { if (serviceConnection.serviceManager.connectedApp!.isRunningOnDartVM != true) { _log.finest('screen requires Dart VM: returning false'); - return false; + return (show: false, disabledReason: ScreenDisabledReason.requiresDartVm); } } if (screen.requiresFlutter && serviceConnection.serviceManager.connectedApp!.isFlutterAppNow == false) { _log.finest('screen requires Flutter: returning false'); - return false; + return (show: false, disabledReason: ScreenDisabledReason.requiresFlutter); } if (screen.requiresDebugBuild) { if (serviceConnection.serviceManager.connectedApp!.isProfileBuildNow == true) { _log.finest('screen requires debug build: returning false'); - return false; + return ( + show: false, + disabledReason: ScreenDisabledReason.requiresDebugBuild, + ); } } if (screen.requiresVmDeveloperMode) { if (!preferences.vmDeveloperModeEnabled.value) { _log.finest('screen requires vm developer mode: returning false'); - return false; + return ( + show: false, + disabledReason: ScreenDisabledReason.requiresVmDeveloperMode, + ); } } _log.finest('${screen.screenId} screen supported: returning true'); - return true; + return (show: true, disabledReason: null); } class BadgePainter extends CustomPainter { @@ -573,3 +592,17 @@ class ShortcutsConfiguration { bool get isEmpty => shortcuts.isEmpty && actions.isEmpty; } + +enum ScreenDisabledReason { + offlineDataNotSupported('does not support offline data.'), + requiresDartLibrary(null), + requiresDartVm('requires the Dart VM, but it is not available.'), + requiresDebugBuild('only supports debug builds.'), + requiresFlutter('only supports Flutter applications.'), + requiresVmDeveloperMode('only works when VM Developer Mode is enabled'), + serviceNotReady('requires a connection but the VM service is not ready.'); + + const ScreenDisabledReason(this.message); + + final String? message; +} diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 923f5ac471a..20b7274829b 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -10,7 +10,8 @@ To learn more about DevTools, check out the ## General updates -TODO: Remove this section if there are not any general updates. +* Improve messaging when a screen is unavailable for the platform of the +connected app. - [#7958](https://github.com/flutter/devtools/pull/7958) ## Inspector updates diff --git a/packages/devtools_app/test/shared/visible_screens_test.dart b/packages/devtools_app/test/shared/visible_screens_test.dart index 43c5ec1d19f..08c5a81998b 100644 --- a/packages/devtools_app/test/shared/visible_screens_test.dart +++ b/packages/devtools_app/test/shared/visible_screens_test.dart @@ -274,6 +274,6 @@ void main() { List get visibleScreenTypes => defaultScreens() .map((s) => s.screen) - .where(shouldShowScreen) + .where((s) => shouldShowScreen(s).show) .map((s) => s.runtimeType) .toList(); diff --git a/packages/devtools_test/lib/src/helpers/actions.dart b/packages/devtools_test/lib/src/helpers/actions.dart index 0da71455e47..8efeda1eff8 100644 --- a/packages/devtools_test/lib/src/helpers/actions.dart +++ b/packages/devtools_test/lib/src/helpers/actions.dart @@ -59,7 +59,7 @@ List generateVisibleScreenIds() { final availableScreenIds = []; // ignore: invalid_use_of_visible_for_testing_member, valid use from package:devtools_test for (final screen in devtoolsScreens!) { - if (shouldShowScreen(screen.screen)) { + if (shouldShowScreen(screen.screen).show) { availableScreenIds.add(screen.screen.screenId); } }