Skip to content

Commit

Permalink
Add support for a multi-tab embedded view (flutter#7710)
Browse files Browse the repository at this point in the history
  • Loading branch information
kenzieschmoll authored May 7, 2024
1 parent aa81e8a commit 73489f9
Show file tree
Hide file tree
Showing 29 changed files with 310 additions and 123 deletions.
77 changes: 52 additions & 25 deletions packages/devtools_app/lib/src/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'dart:async';

import 'package:devtools_app_shared/shared.dart';
import 'package:devtools_app_shared/ui.dart';
import 'package:devtools_app_shared/utils.dart';
import 'package:flutter/foundation.dart';
Expand Down Expand Up @@ -55,6 +56,7 @@ import 'shared/query_parameters.dart';
import 'shared/routing.dart';
import 'shared/screen.dart';
import 'shared/ui/hover.dart';
import 'shared/utils.dart';
import 'standalone_ui/standalone_screen.dart';

// Assign to true to use a sample implementation of a conditional screen.
Expand Down Expand Up @@ -115,7 +117,7 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
// the IDE one (since the user can't access the preference, and the
// preference may have been set in an external window and differ from the
// IDE theme).
return ideTheme.embed ? ideTheme.isDarkMode : _isDarkThemeEnabledPreference;
return isEmbedded() ? ideTheme.isDarkMode : _isDarkThemeEnabledPreference;
}

bool _isDarkThemeEnabledPreference = true;
Expand Down Expand Up @@ -225,7 +227,7 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
return MaterialPage(
child: DevToolsScaffold.withChild(
key: const Key('not-found'),
embed: params.embed,
embedMode: params.embedMode,
child: PageNotFound(
page: page,
routerDelegate: routerDelegate,
Expand All @@ -241,7 +243,7 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
DevToolsNavigationState? __,
) {
final vmServiceUri = queryParams.vmServiceUri;
final embed = queryParams.embed;
final embedMode = queryParams.embedMode;
final hiddenScreens = queryParams.hiddenScreens;

// TODO(dantup): We should be able simplify this a little, removing params['page']
Expand All @@ -265,8 +267,14 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
],
builder: (_, __, child) {
final screens = _visibleScreens()
.where((p) => embed && page != null ? p.screenId == page : true)
.where((p) => !hiddenScreens.contains(p.screenId))
.where(
(s) => _maybeIncludeOnlyEmbeddedScreen(
s,
page: page,
embedMode: embedMode,
),
)
.where((s) => !hiddenScreens.contains(s.screenId))
.toList();
if (queryParams.hideExtensions) {
screens.removeWhere((s) => s is ExtensionScreen);
Expand All @@ -280,26 +288,28 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
return MultiProvider(
providers: _providedControllers(),
child: DevToolsScaffold(
embed: embed,
embedMode: embedMode,
page: page,
screens: screens,
actions: [
if (connectedToVmService) ...[
// Hide the hot reload button for Dart web apps, where the
// hot reload service extension is not avilable and where the
// [service.reloadServices] RPC is not implemented.
// TODO(https://github.com/flutter/devtools/issues/6441): find
// a way to show this for Dart web apps when supported.
if (!connectedToDartWebApp)
HotReloadButton(
callOnVmServiceDirectly: !connectedToFlutterApp,
),
// This button will hide itself based on whether the
// hot restart service is available for the connected app.
const HotRestartButton(),
],
...DevToolsScaffold.defaultActions(),
],
actions: isEmbedded()
? []
: [
if (connectedToVmService) ...[
// Hide the hot reload button for Dart web apps, where the
// hot reload service extension is not avilable and where the
// [service.reloadServices] RPC is not implemented.
// TODO(https://github.com/flutter/devtools/issues/6441): find
// a way to show this for Dart web apps when supported.
if (!connectedToDartWebApp)
HotReloadButton(
callOnVmServiceDirectly: !connectedToFlutterApp,
),
// This button will hide itself based on whether the
// hot restart service is available for the connected app.
const HotRestartButton(),
],
...DevToolsScaffold.defaultActions(),
],
),
);
},
Expand All @@ -309,12 +319,29 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
return connectedToVmService
? Initializer(
url: vmServiceUri,
allowConnectionScreenOnDisconnect: !embed,
allowConnectionScreenOnDisconnect: !embedMode.embedded,
builder: (_) => scaffoldBuilder(),
)
: scaffoldBuilder();
}

/// Helper function that will be used in a 'List.where' call to generate a
/// list of [Screen]s to pass to a [DevToolsScaffold].
///
/// When [embedMode] is [EmbedMode.embedOne], this method will return true
/// only when [screen] matches the specified [page]. Otherwise, this method
/// will return true for any [screen].
bool _maybeIncludeOnlyEmbeddedScreen(
Screen screen, {
required String? page,
required EmbedMode embedMode,
}) {
if (embedMode == EmbedMode.embedOne && page != null) {
return screen.screenId == page;
}
return true;
}

/// The pages that the app exposes.
Map<String, UrlParametersBuilder> get pages {
return _routes ??= {
Expand All @@ -323,7 +350,7 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
snapshotScreenId: (_, __, params, ___) {
return DevToolsScaffold.withChild(
key: UniqueKey(),
embed: params.embed,
embedMode: params.embedMode,
child: MultiProvider(
providers: _providedControllers(offline: true),
child: OfflineScreenBody(params.offlineScreenId, _screens),
Expand Down
26 changes: 15 additions & 11 deletions packages/devtools_app/lib/src/framework/scaffold.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:devtools_app_shared/shared.dart';
import 'package:devtools_app_shared/ui.dart';
import 'package:devtools_app_shared/utils.dart';
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -41,19 +42,19 @@ class DevToolsScaffold extends StatefulWidget {
required this.screens,
this.page,
List<Widget>? actions,
this.embed = false,
this.embedMode = EmbedMode.none,
}) : actions = actions ?? defaultActions();

DevToolsScaffold.withChild({
Key? key,
required Widget child,
bool embed = false,
EmbedMode embedMode = EmbedMode.none,
List<Widget>? actions,
}) : this(
key: key,
screens: [SimpleScreen(child)],
actions: actions,
embed: embed,
embedMode: embedMode,
);

static List<Widget> defaultActions({Color? color}) => [
Expand All @@ -70,7 +71,7 @@ class DevToolsScaffold extends StatefulWidget {
horizontalPadding.left,
isEmbedded() ? 2.0 : intermediateSpacing,
horizontalPadding.right,
isEmbedded() ? 0.0 : intermediateSpacing,
isEmbedded() ? 2.0 : intermediateSpacing,
);

// Note: when changing this value, also update `flameChartContainerOffset`
Expand All @@ -85,8 +86,11 @@ class DevToolsScaffold extends StatefulWidget {
/// The page being rendered.
final String? page;

/// Whether to render the embedded view (without the header).
final bool embed;
/// The type of embedding for DevTools.
///
/// This may result in rendering the DevTools without the top level tab bar.
/// See [EmbedMode].
final EmbedMode embedMode;

/// Actions that it's possible to perform in this Scaffold.
///
Expand Down Expand Up @@ -199,7 +203,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
// Send the page change info to the framework controller (it can then
// send it on to the devtools server, if one is connected).
frameworkController.notifyPageChange(
PageChangeEvent(screen.screenId, widget.embed),
PageChangeEvent(screen.screenId, widget.embedMode),
);

// Clear error count when navigating to a screen.
Expand Down Expand Up @@ -231,7 +235,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>

// Broadcast the initial page.
frameworkController.notifyPageChange(
PageChangeEvent(_currentScreen.screenId, widget.embed),
PageChangeEvent(_currentScreen.screenId, widget.embedMode),
);
}

Expand Down Expand Up @@ -310,7 +314,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
final showConsole =
serviceConnection.serviceManager.connectedAppInitialized &&
!offlineDataController.showingOfflineData.value &&
_currentScreen.showConsole(widget.embed);
_currentScreen.showConsole(widget.embedMode);

return DragAndDrop(
handleDrop: _importController.importData,
Expand All @@ -319,7 +323,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
context,
),
child: Scaffold(
appBar: widget.embed
appBar: widget.embedMode == EmbedMode.embedOne
? null
: PreferredSize(
preferredSize: Size.fromHeight(defaultToolbarHeight),
Expand Down Expand Up @@ -361,7 +365,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
),
bottomNavigationBar: StatusLine(
currentScreen: _currentScreen,
isEmbedded: widget.embed,
isEmbedded: widget.embedMode.embedded,
isConnected: serviceConnection.serviceManager.hasConnection &&
serviceConnection.serviceManager.connectedAppInitialized,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:async';

import 'package:codicon/codicon.dart';
import 'package:devtools_app_shared/shared.dart';
import 'package:devtools_app_shared/ui.dart';
import 'package:devtools_app_shared/utils.dart';
import 'package:flutter/foundation.dart';
Expand Down Expand Up @@ -45,7 +46,7 @@ class DebuggerScreen extends Screen {
static final id = ScreenMetaData.debugger.id;

@override
bool showConsole(bool embed) => true;
bool showConsole(EmbedMode embedMode) => true;

@override
ShortcutsConfiguration buildKeyboardShortcuts(BuildContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:async';
import 'dart:collection';

import 'package:devtools_app_shared/shared.dart';
import 'package:devtools_app_shared/ui.dart';
import 'package:devtools_app_shared/utils.dart';
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -37,7 +38,7 @@ class InspectorScreen extends Screen {
// There is not enough room to safely show the console in the embed view of
// the DevTools and IDEs have their own consoles.
@override
bool showConsole(bool embed) => !embed;
bool showConsole(EmbedMode embedMode) => !embedMode.embedded;

@override
String get docPageId => screenId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:devtools_app_shared/shared.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';

Expand All @@ -28,7 +29,7 @@ class MemoryScreen extends Screen {
// TODO(polina-c): when embedded and VSCode console features are implemented,
// should be in native console in VSCode
@override
bool showConsole(bool embed) => true;
bool showConsole(EmbedMode embedMode) => true;
}

class MemoryBody extends StatefulWidget {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ library;

import 'dart:async';

import 'package:devtools_app_shared/ui.dart';
import 'package:flutter/foundation.dart';
import 'package:js/js.dart';
import 'package:logging/logging.dart';
Expand Down Expand Up @@ -208,7 +207,7 @@ GtagEventDevTools _gtagEvent({
ide_launched: ideLaunched,
flutter_client_id: flutterClientId,
is_external_build: isExternalBuild.toString(),
is_embedded: ideTheme.embed.toString(),
is_embedded: isEmbedded().toString(),
g3_username: devToolsExtensionPoints.username(),
ide_launched_feature: ideLaunchedFeature,
// [PerformanceScreenMetrics]
Expand Down Expand Up @@ -273,7 +272,7 @@ GtagExceptionDevTools _gtagException(
ide_launched: _ideLaunched,
flutter_client_id: flutterClientId,
is_external_build: isExternalBuild.toString(),
is_embedded: ideTheme.embed.toString(),
is_embedded: isEmbedded().toString(),
g3_username: devToolsExtensionPoints.username(),
ide_launched_feature: ideLaunchedFeature,
// [PerformanceScreenMetrics]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be found
// in the LICENSE file.

import 'package:devtools_app_shared/ui.dart';
import 'package:flutter/services.dart';
import 'package:logging/logging.dart';

import '../../globals.dart';
import '../../utils.dart';
import '_copy_to_clipboard_desktop.dart'
if (dart.library.js_interop) '_copy_to_clipboard_web.dart';

Expand All @@ -31,7 +31,7 @@ Future<void> copyToClipboard(

if (successMessage != null) notificationService.push(successMessage);
} catch (e) {
if (ideTheme.embed) {
if (isEmbedded()) {
_log.warning(
'DevTools copy failed. This may be as a result of a known bug in VSCode. '
'See https://github.com/Dart-Code/Dart-Code/issues/4540 for more '
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import 'dart:developer';

import 'package:devtools_app_shared/service.dart';
import 'package:devtools_app_shared/service_extensions.dart';
import 'package:devtools_app_shared/ui.dart';
import 'package:devtools_app_shared/utils.dart';
import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
Expand Down Expand Up @@ -243,7 +242,7 @@ class InspectorService extends InspectorServiceBase {

// When DevTools is embedded, default hover eval mode to off.
@override
bool get hoverEvalModeEnabledByDefault => !ideTheme.embed;
bool get hoverEvalModeEnabledByDefault => !isEmbedded();

void onExtensionVmServiceReceived(Event e) {
if ('Flutter.Frame' == e.extensionKind) {
Expand Down
6 changes: 3 additions & 3 deletions packages/devtools_app/lib/src/shared/error_badge_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import '../service/vm_service_wrapper.dart';
import 'diagnostics/diagnostics_node.dart';
import 'globals.dart';
import 'primitives/listenable.dart';
import 'primitives/utils.dart';
import 'query_parameters.dart';

class ErrorBadgeManager extends DisposableController
with AutoDisposeControllerMixin {
Expand Down Expand Up @@ -92,8 +92,8 @@ class ErrorBadgeManager extends DisposableController
}

final queryParams =
devToolsQueryParams(devToolsUrlNode.getStringMember('value')!);
final inspectorRef = queryParams['inspectorRef'] ?? '';
DevToolsQueryParams.fromUrl(devToolsUrlNode.getStringMember('value')!);
final inspectorRef = queryParams.inspectorRef ?? '';

return InspectableWidgetError(errorMessage, inspectorRef);
}
Expand Down
Loading

0 comments on commit 73489f9

Please sign in to comment.