From fa9fe9a2e86d30117fa8d774b46306d6625abbc6 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 18 Mar 2025 09:46:38 -0700 Subject: [PATCH 1/4] Show reconnection overlay when disconnection detected --- .../lib/src/shared/editor/editor_client.dart | 21 ++++++ .../property_editor_controller.dart | 27 +++++++ .../property_editor_panel.dart | 49 ++++++++----- .../property_editor/property_editor_view.dart | 10 ++- .../property_editor/reconnecting_overlay.dart | 72 +++++++++++++++++++ .../property_editor/utils/_utils_desktop.dart | 4 ++ .../property_editor/utils/_utils_web.dart | 4 ++ .../property_editor/utils/utils.dart | 7 ++ 8 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart diff --git a/packages/devtools_app/lib/src/shared/editor/editor_client.dart b/packages/devtools_app/lib/src/shared/editor/editor_client.dart index 58ff98cfdb1..dffc8ae6881 100644 --- a/packages/devtools_app/lib/src/shared/editor/editor_client.dart +++ b/packages/devtools_app/lib/src/shared/editor/editor_client.dart @@ -317,6 +317,27 @@ class EditorClient extends DisposableController } } + Future isClientClosed() async { + try { + // Make an empty request to DTD. + await _dtd.call('', ''); + } on StateError catch (e) { + // TODO(https://github.com/flutter/devtools/issues/9028): Replace with a + // check for whether DTD is closed. Requires a change to package:dtd. + // + // This is only a temporary fix. If the error in package:json_rpc_2 is + // changed, this will no longer catch it. See: + // https://github.com/dart-lang/tools/blob/b55643dadafd3ac6b2bd20823802f75929ebf98e/pkgs/json_rpc_2/lib/src/client.dart#L151 + if (e.message.contains('The client is closed.')) { + return true; + } + } catch (e) { + // Ignore other exceptions. If the client is open, we expect this to fail + // with the error: 'Unknown method "."'. + } + return false; + } + Future _call( EditorMethod method, { Map? params, diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart index 5f8a493a845..5178d34d27a 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart @@ -2,6 +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 'dart:async'; + import 'package:devtools_app_shared/utils.dart'; import 'package:flutter/foundation.dart'; @@ -37,17 +39,31 @@ class PropertyEditorController extends DisposableController _editableWidgetData; final _editableWidgetData = ValueNotifier(null); + ValueListenable get shouldReconnect => _shouldReconnect; + final _shouldReconnect = ValueNotifier(false); + + bool get waitingForFirstEvent => _waitingForFirstEvent; + bool _waitingForFirstEvent = true; + late final Debouncer _editableArgsDebouncer; + late final Timer _checkConnectionTimer; + static const _editableArgsDebounceDuration = Duration(milliseconds: 600); + static const _checkConnectionInterval = Duration(seconds: 5); + @override void init() { super.init(); _editableArgsDebouncer = Debouncer(duration: _editableArgsDebounceDuration); + _checkConnectionTimer = _periodicallyCheckConnection( + _checkConnectionInterval, + ); autoDisposeStreamSubscription( editorClient.activeLocationChangedStream.listen((event) async { + if (_waitingForFirstEvent) _waitingForFirstEvent = false; final textDocument = event.textDocument; final cursorPosition = event.selections.first.active; // Don't do anything if the text document is null. @@ -78,6 +94,7 @@ class PropertyEditorController extends DisposableController @override void dispose() { _editableArgsDebouncer.dispose(); + _checkConnectionTimer.cancel(); super.dispose(); } @@ -121,6 +138,16 @@ class PropertyEditorController extends DisposableController ); } + Timer _periodicallyCheckConnection(Duration interval) { + return Timer.periodic(interval, (timer) async { + final isClosed = await editorClient.isClientClosed(); + if (isClosed) { + _shouldReconnect.value = true; + timer.cancel(); + } + }); + } + @visibleForTesting void initForTestsOnly({ EditableArgumentsResult? editableArgsResult, diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart index b0c67e9a554..7cb8ff3b5f1 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart @@ -15,6 +15,7 @@ import '../../../shared/editor/editor_client.dart'; import '../../../shared/ui/common_widgets.dart'; import 'property_editor_controller.dart'; import 'property_editor_view.dart'; +import 'reconnecting_overlay.dart'; /// The side panel for the Property Editor. class PropertyEditorPanel extends StatefulWidget { @@ -106,24 +107,36 @@ class _PropertyEditorConnectedPanelState @override Widget build(BuildContext context) { - return Scrollbar( - controller: scrollController, - thumbVisibility: true, - child: SingleChildScrollView( - controller: scrollController, - child: Padding( - padding: const EdgeInsets.fromLTRB( - denseSpacing, - defaultSpacing, - defaultSpacing, // Additional right padding for scroll bar. - defaultSpacing, - ), - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [PropertyEditorView(controller: widget.controller)], - ), - ), - ), + return ValueListenableBuilder( + valueListenable: widget.controller.shouldReconnect, + builder: (context, shouldReconnect, _) { + return Stack( + children: [ + Scrollbar( + controller: scrollController, + thumbVisibility: true, + child: SingleChildScrollView( + controller: scrollController, + child: Padding( + padding: const EdgeInsets.fromLTRB( + denseSpacing, + defaultSpacing, + defaultSpacing, // Additional right padding for scroll bar. + defaultSpacing, + ), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + PropertyEditorView(controller: widget.controller), + ], + ), + ), + ), + ), + if (shouldReconnect) const ReconnectingOverlay(), + ], + ); + }, ); } } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart index 3db138d8e9e..4007a5436f5 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart @@ -37,8 +37,14 @@ class PropertyEditorView extends StatelessWidget { final editableWidgetData = values.third as EditableWidgetData?; if (editableWidgetData == null) { - return const CenteredMessage( - message: 'No Flutter widget found at the current cursor location.', + final introSentence = + controller.waitingForFirstEvent + ? '👋 Welcome to the Flutter Property Editor!' + : 'No Flutter widget found at the current cursor location.'; + const howToUseSentence = + 'Please move your cursor to a Flutter widget constructor invocation to view its properties.'; + return CenteredMessage( + message: '$introSentence\n\n$howToUseSentence', ); } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart new file mode 100644 index 00000000000..84722e691d1 --- /dev/null +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart @@ -0,0 +1,72 @@ +// Copyright 2025 The Flutter Authors +// 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 'dart:async'; + +import 'package:devtools_app_shared/ui.dart'; +import 'package:flutter/material.dart'; + +import 'utils/utils.dart'; + +class ReconnectingOverlay extends StatefulWidget { + const ReconnectingOverlay({super.key}); + + @override + State createState() => _ReconnectingOverlayState(); +} + +class _ReconnectingOverlayState extends State { + static const _countdownInterval = Duration(seconds: 1); + late final Timer _countdownTimer; + int _secondsUntilReconnection = 3; + + @override + void initState() { + super.initState(); + _countdownTimer = Timer.periodic(_countdownInterval, _onTick); + } + + @override + void dispose() { + _countdownTimer.cancel(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + return Container( + color: theme.colorScheme.surface.withValues(alpha: 0.5), + child: Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + const CircularProgressIndicator(), + const SizedBox(height: defaultSpacing), + Text( + _secondsUntilReconnection > 0 + ? 'Reconnecting in $_secondsUntilReconnection' + : 'Reconnecting...', + style: theme.textTheme.headlineMedium, + ), + ], + ), + ), + ); + } + + void _onTick(Timer timer) { + setState(() { + _secondsUntilReconnection--; + if (_secondsUntilReconnection == 0) { + timer.cancel(); + _reconnect(); + } + }); + } + + void _reconnect() { + forceReload(); + } +} diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_desktop.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_desktop.dart index c79da0f1bac..ff2333a299b 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_desktop.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_desktop.dart @@ -2,6 +2,10 @@ // 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. +void reloadIframe() { + // No-op for desktop platforms. +} + void addBlurListener() { // No-op for desktop platforms. } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_web.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_web.dart index 87b99146ce0..880e275da9c 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_web.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/_utils_web.dart @@ -6,6 +6,10 @@ import 'dart:js_interop'; import 'package:web/web.dart'; +void reloadIframe() { + window.location.reload(); +} + void addBlurListener() { window.addEventListener('blur', _onBlur.toJS); } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart index f33d9789fa5..fa3f1d7e3a7 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart @@ -78,6 +78,13 @@ RichText convertDartDocToRichText( return RichText(text: TextSpan(children: children)); } +/// Workaround to force reload the Property Editor when it disconnects. +/// +/// See https://github.com/flutter/devtools/issues/9028 for details. +void forceReload() { + reloadIframe(); +} + /// Workaround to prevent TextFields from holding onto focus when IFRAME-ed. /// /// See https://github.com/flutter/devtools/issues/8929 for details. From 95bee0f859c9c7db87c7de57422b05f7b48fccac Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 19 Mar 2025 12:57:23 -0700 Subject: [PATCH 2/4] Respond to PR comments --- .../timeline_events/timeline_events_view.dart | 30 ++++------ .../lib/src/shared/ui/common_widgets.dart | 55 +++++++++++++++++++ .../property_editor_controller.dart | 2 +- .../property_editor/reconnecting_overlay.dart | 31 +++++------ 4 files changed, 83 insertions(+), 35 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart index 3687b02c914..cf3288dcd4a 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart @@ -79,24 +79,18 @@ class _TimelineEventsTabViewState extends State _refreshingOverlay = OverlayEntry( maintainState: true, builder: (context) { - return Center( - child: Padding( - padding: const EdgeInsets.only(top: _overlayOffset), - child: RoundedOutlinedBorder( - clip: true, - child: Container( - width: width, - height: height, - color: theme.colorScheme.semiTransparentOverlayColor, - child: Center( - child: Text( - 'Refreshing the timeline...\n\n' - 'This may take a few seconds. Please do not\n' - 'refresh the page.', - textAlign: TextAlign.center, - style: theme.textTheme.titleMedium, - ), - ), + return DevToolsOverlay( + content: Container( + width: width, + height: height, + color: theme.colorScheme.semiTransparentOverlayColor, + child: Center( + child: Text( + 'Refreshing the timeline...\n\n' + 'This may take a few seconds. Please do not\n' + 'refresh the page.', + textAlign: TextAlign.center, + style: theme.textTheme.titleMedium, ), ), ), diff --git a/packages/devtools_app/lib/src/shared/ui/common_widgets.dart b/packages/devtools_app/lib/src/shared/ui/common_widgets.dart index fb8bf822149..20f5610ad8a 100644 --- a/packages/devtools_app/lib/src/shared/ui/common_widgets.dart +++ b/packages/devtools_app/lib/src/shared/ui/common_widgets.dart @@ -2283,3 +2283,58 @@ class _PositiveIntegerSettingState extends State ); } } + +/// Creates an overlay with the provided [content]. +/// +/// Set [fullScreen] to true to take up the entire screen. Otherwise, a +/// [maxSize] and [topOffset] can be provided to determine the overlay's size +/// and location. +class DevToolsOverlay extends StatelessWidget { + const DevToolsOverlay({ + super.key, + required this.content, + this.fullScreen = false, + this.maxSize, + this.topOffset, + }) : assert(maxSize != null || topOffset != null ? !fullScreen : true); + + final Widget content; + final bool fullScreen; + final Size? maxSize; + final double? topOffset; + + @override + Widget build(BuildContext context) { + final parentSize = MediaQuery.of(context).size; + + final overlayContent = Container( + width: _overlayWidth(parentSize), + height: _overlayHeight(parentSize), + color: Theme.of(context).colorScheme.semiTransparentOverlayColor, + child: Center(child: content), + ); + + return fullScreen + ? overlayContent + : Center( + child: Padding( + padding: EdgeInsets.only(top: topOffset ?? 0.0), + child: RoundedOutlinedBorder(clip: true, child: overlayContent), + ), + ); + } + + double _overlayWidth(Size parentSize) { + if (fullScreen) return parentSize.width; + final defaultWidth = parentSize.width - largeSpacing; + return maxSize != null ? min(maxSize!.width, defaultWidth) : defaultWidth; + } + + double _overlayHeight(Size parentSize) { + if (fullScreen) return parentSize.height; + final defaultHeight = parentSize.height - largeSpacing; + return maxSize != null + ? min(maxSize!.height, defaultHeight) + : defaultHeight; + } +} diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart index 5178d34d27a..572d1509e41 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart @@ -51,7 +51,7 @@ class PropertyEditorController extends DisposableController static const _editableArgsDebounceDuration = Duration(milliseconds: 600); - static const _checkConnectionInterval = Duration(seconds: 5); + static const _checkConnectionInterval = Duration(minutes: 1); @override void init() { diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart index 84722e691d1..66f9f3394b2 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:devtools_app_shared/ui.dart'; import 'package:flutter/material.dart'; +import '../../../shared/ui/common_widgets.dart'; import 'utils/utils.dart'; class ReconnectingOverlay extends StatefulWidget { @@ -36,22 +37,20 @@ class _ReconnectingOverlayState extends State { @override Widget build(BuildContext context) { final theme = Theme.of(context); - return Container( - color: theme.colorScheme.surface.withValues(alpha: 0.5), - child: Center( - child: Column( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - const CircularProgressIndicator(), - const SizedBox(height: defaultSpacing), - Text( - _secondsUntilReconnection > 0 - ? 'Reconnecting in $_secondsUntilReconnection' - : 'Reconnecting...', - style: theme.textTheme.headlineMedium, - ), - ], - ), + return DevToolsOverlay( + fullScreen: true, + content: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + const CircularProgressIndicator(), + const SizedBox(height: defaultSpacing), + Text( + _secondsUntilReconnection > 0 + ? 'Reconnecting in $_secondsUntilReconnection' + : 'Reconnecting...', + style: theme.textTheme.headlineMedium, + ), + ], ), ); } From 5acf0c4f33c948e8a2c57da3ddb86078d00228bd Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 19 Mar 2025 13:31:25 -0700 Subject: [PATCH 3/4] Fix overlay --- .../timeline_events/timeline_events_view.dart | 29 +++++-------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart index cf3288dcd4a..fd1d74d2a72 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart @@ -65,14 +65,6 @@ class _TimelineEventsTabViewState extends State } void _insertOverlay() { - final width = math.min( - _overlaySize.width, - MediaQuery.of(context).size.width - 2 * denseSpacing, - ); - final height = math.min( - _overlaySize.height, - MediaQuery.of(context).size.height - 2 * denseSpacing, - ); final theme = Theme.of(context); _refreshingOverlay?.remove(); Overlay.of(context).insert( @@ -80,19 +72,14 @@ class _TimelineEventsTabViewState extends State maintainState: true, builder: (context) { return DevToolsOverlay( - content: Container( - width: width, - height: height, - color: theme.colorScheme.semiTransparentOverlayColor, - child: Center( - child: Text( - 'Refreshing the timeline...\n\n' - 'This may take a few seconds. Please do not\n' - 'refresh the page.', - textAlign: TextAlign.center, - style: theme.textTheme.titleMedium, - ), - ), + topOffset: _overlayOffset, + maxSize: _overlaySize, + content: Text( + 'Refreshing the timeline...\n\n' + 'This may take a few seconds. Please do not\n' + 'refresh the page.', + textAlign: TextAlign.center, + style: theme.textTheme.titleMedium, ), ); }, From 422891927bc5def87cbf9398820c2a811e23420b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 19 Mar 2025 13:46:13 -0700 Subject: [PATCH 4/4] Clean up --- .../performance/panes/timeline_events/timeline_events_view.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart index fd1d74d2a72..1a44da58283 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart @@ -3,7 +3,6 @@ // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. import 'dart:async'; -import 'dart:math' as math; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart';