From 020fee24856b875d5a10d419c9e6ee776e281aad Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 21 May 2020 13:10:56 +0100 Subject: [PATCH 1/5] Add an embedded flag to run without top/bottom app bars --- packages/devtools_app/lib/src/app.dart | 15 +++++++++++-- .../devtools_app/lib/src/initializer.dart | 22 +++++++++++++------ packages/devtools_app/lib/src/scaffold.dart | 9 ++++++-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart index 7937e303086..58f67054c66 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart @@ -36,6 +36,7 @@ import 'ui/service_extension_widgets.dart'; import 'utils.dart'; const homeRoute = '/'; +const disconnectedRoute = '/disconnected'; const snapshotRoute = '/snapshot'; /// Top-level configuration for the app. @@ -123,12 +124,19 @@ class DevToolsAppState extends State { return _routes ??= { homeRoute: (_, params, __) { if (params['uri']?.isNotEmpty ?? false) { + final embed = params['embed'] == 'true'; + final page = params['page']; + final tabs = embed && page != null + ? _visibleScreens().where((p) => p.screenId == page).toList() + : _visibleScreens(); return Initializer( url: params['uri'], + disconnectedRoute: embed ? disconnectedRoute : homeRoute, builder: (_) => _providedControllers( child: DevToolsScaffold( - initialPage: params['page'], - tabs: _visibleScreens(), + embed: embed, + initialPage: page, + tabs: tabs, actions: [ if (serviceManager.connectedApp.isFlutterAppNow) ...[ HotReloadButton(), @@ -151,6 +159,9 @@ class DevToolsAppState extends State { child: SnapshotScreenBody(args, _screens), ), ); + }, + disconnectedRoute: (_, __, ___) { + return const CenteredMessage('App Disconnected.'); } }; } diff --git a/packages/devtools_app/lib/src/initializer.dart b/packages/devtools_app/lib/src/initializer.dart index 7991e00d2c6..95cbd690c7b 100644 --- a/packages/devtools_app/lib/src/initializer.dart +++ b/packages/devtools_app/lib/src/initializer.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +import 'app.dart'; import 'auto_dispose_mixin.dart'; import 'framework/framework_core.dart'; import 'globals.dart'; @@ -24,8 +25,12 @@ import 'url_utils.dart'; /// connected. As we require additional services to be available, add them /// here. class Initializer extends StatefulWidget { - const Initializer({Key key, @required this.url, @required this.builder}) - : assert(builder != null), + const Initializer({ + Key key, + @required this.url, + @required this.builder, + this.disconnectedRoute = homeRoute, + }) : assert(builder != null), super(key: key); /// The builder for the widget's children. @@ -38,6 +43,9 @@ class Initializer extends StatefulWidget { /// If null, the app will navigate to the [ConnectScreen]. final String url; + /// The route to navigate to when the VM becomes disconnected. + final String disconnectedRoute; + @override _InitializerState createState() => _InitializerState(); } @@ -86,7 +94,7 @@ class _InitializerState extends State Future _attemptUrlConnection() async { if (widget.url == null) { - _navigateToConnectPage(); + _handleNoConnection(); return; } @@ -99,15 +107,15 @@ class _InitializerState extends State ); if (!connected) { - _navigateToConnectPage(); + _handleNoConnection(); } } - /// Goes to the connect page if the [service.serviceManager] is not currently connected. - void _navigateToConnectPage() { + /// Goes to the connect/disconnected page if the [service.serviceManager] is not currently connected. + void _handleNoConnection() { WidgetsBinding.instance.addPostFrameCallback((_) { if (!_checkLoaded() && ModalRoute.of(context).isCurrent) { - Navigator.of(context).popAndPushNamed('/'); + Navigator.of(context).popAndPushNamed(widget.disconnectedRoute); } }); } diff --git a/packages/devtools_app/lib/src/scaffold.dart b/packages/devtools_app/lib/src/scaffold.dart index 9bd4b39d27e..489d6e69965 100644 --- a/packages/devtools_app/lib/src/scaffold.dart +++ b/packages/devtools_app/lib/src/scaffold.dart @@ -35,6 +35,7 @@ class DevToolsScaffold extends StatefulWidget { @required this.tabs, this.initialPage, this.actions, + this.embed = false, }) : assert(tabs != null), super(key: key); @@ -68,6 +69,9 @@ class DevToolsScaffold extends StatefulWidget { /// The initial page to render. final String initialPage; + /// Whether to render the embedded view (without the header). + final bool embed; + /// Actions that it's possible to perform in this Scaffold. /// /// These will generally be [RegisteredServiceExtensionButton]s. @@ -271,13 +275,14 @@ class DevToolsScaffoldState extends State // to make sure we are only handling drops from the active scaffold. handleDrop: _importController.importData, child: Scaffold( - appBar: _buildAppBar(), + appBar: widget.embed ? null : _buildAppBar(), body: TabBarView( physics: defaultTabBarViewPhysics, controller: _tabController, children: tabBodies, ), - bottomNavigationBar: _buildStatusLine(context), + bottomNavigationBar: + widget.embed ? null : _buildStatusLine(context), ), ), ), From 0266275a00402fb7b1b498691344c232da370123 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 27 May 2020 11:40:43 +0100 Subject: [PATCH 2/5] Use an overlay when disconnected --- packages/devtools_app/lib/src/app.dart | 6 +-- .../devtools_app/lib/src/initializer.dart | 43 +++++++++++++++++-- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart index 58f67054c66..0313d5e4e6e 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart @@ -36,7 +36,6 @@ import 'ui/service_extension_widgets.dart'; import 'utils.dart'; const homeRoute = '/'; -const disconnectedRoute = '/disconnected'; const snapshotRoute = '/snapshot'; /// Top-level configuration for the app. @@ -131,7 +130,7 @@ class DevToolsAppState extends State { : _visibleScreens(); return Initializer( url: params['uri'], - disconnectedRoute: embed ? disconnectedRoute : homeRoute, + allowConnectionScreenOnDisconnect: !embed, builder: (_) => _providedControllers( child: DevToolsScaffold( embed: embed, @@ -160,9 +159,6 @@ class DevToolsAppState extends State { ), ); }, - disconnectedRoute: (_, __, ___) { - return const CenteredMessage('App Disconnected.'); - } }; } diff --git a/packages/devtools_app/lib/src/initializer.dart b/packages/devtools_app/lib/src/initializer.dart index 95cbd690c7b..2ebbe7bd12b 100644 --- a/packages/devtools_app/lib/src/initializer.dart +++ b/packages/devtools_app/lib/src/initializer.dart @@ -29,7 +29,7 @@ class Initializer extends StatefulWidget { Key key, @required this.url, @required this.builder, - this.disconnectedRoute = homeRoute, + this.allowConnectionScreenOnDisconnect = true, }) : assert(builder != null), super(key: key); @@ -43,8 +43,8 @@ class Initializer extends StatefulWidget { /// If null, the app will navigate to the [ConnectScreen]. final String url; - /// The route to navigate to when the VM becomes disconnected. - final String disconnectedRoute; + /// Whether to allow navigating to the connection screen upon disconnect. + final bool allowConnectionScreenOnDisconnect; @override _InitializerState createState() => _InitializerState(); @@ -115,11 +115,46 @@ class _InitializerState extends State void _handleNoConnection() { WidgetsBinding.instance.addPostFrameCallback((_) { if (!_checkLoaded() && ModalRoute.of(context).isCurrent) { - Navigator.of(context).popAndPushNamed(widget.disconnectedRoute); + Overlay.of(context).insert(_createDisconnectedOverlay()); } }); } + OverlayEntry _createDisconnectedOverlay() { + final theme = Theme.of(context); + OverlayEntry overlay; + overlay = OverlayEntry( + builder: (context) => Container( + color: const Color.fromRGBO(128, 128, 128, 0.5), + child: Center( + child: Column( + children: [ + const Spacer(), + Text( + 'Disconnected', + style: theme.textTheme.headline3, + ), + if (widget.allowConnectionScreenOnDisconnect) + RaisedButton( + onPressed: () { + overlay.remove(); + Navigator.of(context).popAndPushNamed(homeRoute); + }, + child: const Text('Connect to Another App')) + else + Text( + 'Run a new debug session to reconnect', + style: theme.textTheme.bodyText2, + ), + const Spacer(), + ], + ), + ), + ), + ); + return overlay; + } + @override Widget build(BuildContext context) { return _checkLoaded() && _dependenciesLoaded From 77ac4e6c9765a3913726987583b529d1b872f38c Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 10 Jun 2020 08:04:45 +0100 Subject: [PATCH 3/5] Add a TODO --- packages/devtools_app/lib/src/initializer.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/devtools_app/lib/src/initializer.dart b/packages/devtools_app/lib/src/initializer.dart index 2ebbe7bd12b..df5b5876fd1 100644 --- a/packages/devtools_app/lib/src/initializer.dart +++ b/packages/devtools_app/lib/src/initializer.dart @@ -125,6 +125,7 @@ class _InitializerState extends State OverlayEntry overlay; overlay = OverlayEntry( builder: (context) => Container( + // TODO(dantup): Change this to a theme colour and ensure it works in both dart/light themes color: const Color.fromRGBO(128, 128, 128, 0.5), child: Center( child: Column( From f1e32a3ef301b210e9b5a735d5ca3841cdca06aa Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 10 Jun 2020 09:26:54 +0100 Subject: [PATCH 4/5] Fix tests for disconnect overlay --- .../devtools_app/lib/src/initializer.dart | 12 ++----- .../devtools_app/test/initializer_test.dart | 36 ++++++++++++------- packages/devtools_app/test/support/mocks.dart | 13 +++++-- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/packages/devtools_app/lib/src/initializer.dart b/packages/devtools_app/lib/src/initializer.dart index df5b5876fd1..a30ab8aeff6 100644 --- a/packages/devtools_app/lib/src/initializer.dart +++ b/packages/devtools_app/lib/src/initializer.dart @@ -76,10 +76,7 @@ class _InitializerState extends State // If we become disconnected, attempt to reconnect. autoDispose( serviceManager.onStateChange.where((connected) => !connected).listen((_) { - // TODO(https://github.com/flutter/devtools/issues/1285): On losing - // the connection, only provide an option to reconnect; don't - // immediately go to the connection page. - _attemptUrlConnection(); + _handleNoConnection(); }), ); // Trigger a rebuild when the connection becomes available. This is done @@ -111,7 +108,7 @@ class _InitializerState extends State } } - /// Goes to the connect/disconnected page if the [service.serviceManager] is not currently connected. + /// Shows a "disconnected" overlay if the [service.serviceManager] is not currently connected. void _handleNoConnection() { WidgetsBinding.instance.addPostFrameCallback((_) { if (!_checkLoaded() && ModalRoute.of(context).isCurrent) { @@ -131,10 +128,7 @@ class _InitializerState extends State child: Column( children: [ const Spacer(), - Text( - 'Disconnected', - style: theme.textTheme.headline3, - ), + Text('Disconnected', style: theme.textTheme.headline3), if (widget.allowConnectionScreenOnDisconnect) RaisedButton( onPressed: () { diff --git a/packages/devtools_app/test/initializer_test.dart b/packages/devtools_app/test/initializer_test.dart index 960d14d9492..70ab10cded4 100644 --- a/packages/devtools_app/test/initializer_test.dart +++ b/packages/devtools_app/test/initializer_test.dart @@ -3,9 +3,9 @@ // found in the LICENSE file. @TestOn('vm') +import 'package:devtools_app/src/globals.dart'; import 'package:devtools_app/src/initializer.dart' hide ensureInspectorDependencies; -import 'package:devtools_app/src/globals.dart'; import 'package:devtools_app/src/service_manager.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -16,7 +16,6 @@ import 'support/mocks.dart'; void main() { group('Initializer', () { MaterialApp app; - const Key connectKey = Key('connect'); const Key initializedKey = Key('initialized'); setUp(() async { await ensureInspectorDependencies(); @@ -29,12 +28,8 @@ void main() { ); app = MaterialApp( - // This test uses a fake route of /init for the initializer but - // in the real app it's loaded based on whether there's a ?uri= on - // the querystring, with / loading the connect dialog. initialRoute: '/init', routes: { - '/': (_) => const SizedBox(key: connectKey), '/init': (_) => Initializer( url: null, builder: (_) => const SizedBox(key: initializedKey), @@ -43,23 +38,40 @@ void main() { ); }); - testWidgets('navigates back to the connection page when uninitialized', + testWidgets('shows disconnected overlay if not connected', (WidgetTester tester) async { setGlobal( ServiceConnectionManager, FakeServiceManager(useFakeService: true, hasConnection: false), ); - await tester.pumpWidget(app); - await tester.pumpAndSettle(); - expect(find.byKey(connectKey), findsOneWidget); - expect(find.byKey(initializedKey), findsNothing); + + await tester.pumpFrames(app, const Duration(milliseconds: 100)); + expect(find.text('Disconnected'), findsOneWidget); + }); + + testWidgets('shows disconnected overlay upon disconnect', + (WidgetTester tester) async { + final serviceManager = FakeServiceManager(useFakeService: true); + setGlobal(ServiceConnectionManager, serviceManager); + + // Expect standard connected state. + await tester.pumpFrames(app, const Duration(milliseconds: 100)); + expect(find.byKey(initializedKey), findsOneWidget); + expect(find.text('Disconnected'), findsNothing); + + // Trigger a disconnect. + serviceManager.changeState(false); + + // Expect Disconnected overlay. + await tester.pumpFrames(app, const Duration(milliseconds: 100)); + expect(find.text('Disconnected'), findsOneWidget); }); testWidgets('builds contents when initialized', (WidgetTester tester) async { await tester.pumpWidget(app); await tester.pumpAndSettle(); - expect(find.byKey(connectKey), findsNothing); + expect(find.text('Disconnected'), findsNothing); expect(find.byKey(initializedKey), findsOneWidget); }); }); diff --git a/packages/devtools_app/test/support/mocks.dart b/packages/devtools_app/test/support/mocks.dart index cb6a691fa6d..2fbffa33e28 100644 --- a/packages/devtools_app/test/support/mocks.dart +++ b/packages/devtools_app/test/support/mocks.dart @@ -4,9 +4,9 @@ import 'dart:async'; +import 'package:devtools_app/src/banner_messages.dart'; import 'package:devtools_app/src/connected_app.dart'; import 'package:devtools_app/src/debugger/debugger_controller.dart'; -import 'package:devtools_app/src/banner_messages.dart'; import 'package:devtools_app/src/initializer.dart' as initializer; import 'package:devtools_app/src/logging/logging_controller.dart'; import 'package:devtools_app/src/memory/memory_controller.dart' @@ -68,7 +68,7 @@ class FakeServiceManager extends Fake implements ServiceConnectionManager { Future getDisplayRefreshRate() async => 60; @override - final bool hasConnection; + bool hasConnection; @override final IsolateManager isolateManager = FakeIsolateManager(); @@ -108,7 +108,14 @@ class FakeServiceManager extends Fake implements ServiceConnectionManager { } @override - Stream get onStateChange => const Stream.empty(); + Stream get onStateChange => stateChangeStream.stream; + + StreamController stateChangeStream = StreamController(); + + void changeState(bool value) { + hasConnection = value; + stateChangeStream.add(value); + } } class FakeVmService extends Fake implements VmServiceWrapper { From 2625c57abd65bd3992102618878070e2cde49dfb Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 10 Jun 2020 09:31:10 +0100 Subject: [PATCH 5/5] Restore behaviour to attempt to re-connect before showing overlay --- packages/devtools_app/lib/src/initializer.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/devtools_app/lib/src/initializer.dart b/packages/devtools_app/lib/src/initializer.dart index a30ab8aeff6..2202596d667 100644 --- a/packages/devtools_app/lib/src/initializer.dart +++ b/packages/devtools_app/lib/src/initializer.dart @@ -76,7 +76,9 @@ class _InitializerState extends State // If we become disconnected, attempt to reconnect. autoDispose( serviceManager.onStateChange.where((connected) => !connected).listen((_) { - _handleNoConnection(); + // Try to reconnect (otherwise, will fall back to showing the disconnected + // overlay). + _attemptUrlConnection(); }), ); // Trigger a rebuild when the connection becomes available. This is done