diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart index edb3b00eb32..ae506776fbb 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart @@ -85,8 +85,6 @@ class DevToolsApp extends StatefulWidget { /// /// This manages the route generation, and marshals URL query parameters into /// flutter route parameters. -// TODO(https://github.com/flutter/devtools/issues/1146): Introduce tests that -// navigate the full app. class DevToolsAppState extends State with AutoDisposeMixin { List get _screens => [ ..._originalScreens, @@ -100,7 +98,7 @@ class DevToolsAppState extends State with AutoDisposeMixin { /// [extensionService.availableExtensions] and verify tabs are added / removed /// propertly based on the enabled state of extensions. Iterable get _extensionScreens => - extensionService.availableExtensions.value.map( + extensionService.visibleExtensions.value.map( (e) => DevToolsScreen(ExtensionScreen(e)).screen, ); @@ -136,6 +134,11 @@ class DevToolsAppState extends State with AutoDisposeMixin { _clearCachedRoutes(); }); }); + addAutoDisposeListener(extensionService.visibleExtensions, () { + setState(() { + _clearCachedRoutes(); + }); + }); } addAutoDisposeListener(serviceManager.isolateManager.mainIsolate, () { @@ -256,6 +259,7 @@ class DevToolsAppState extends State with AutoDisposeMixin { listenables: [ preferences.vmDeveloperModeEnabled, extensionService.availableExtensions, + extensionService.visibleExtensions, ], builder: (_, __, child) { final screens = _visibleScreens() diff --git a/packages/devtools_app/lib/src/extensions/extension_screen.dart b/packages/devtools_app/lib/src/extensions/extension_screen.dart index d10040bfbeb..1f7e570ba61 100644 --- a/packages/devtools_app/lib/src/extensions/extension_screen.dart +++ b/packages/devtools_app/lib/src/extensions/extension_screen.dart @@ -6,20 +6,20 @@ import 'package:devtools_shared/devtools_extensions.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; -import '../shared/analytics/constants.dart' as gac; import '../shared/common_widgets.dart'; +import '../shared/globals.dart'; import '../shared/primitives/listenable.dart'; import '../shared/primitives/utils.dart'; import '../shared/screen.dart'; -import '../shared/theme.dart'; import 'embedded/controller.dart'; import 'embedded/view.dart'; +import 'extension_screen_controls.dart'; class ExtensionScreen extends Screen { ExtensionScreen(this.extensionConfig) : super.conditional( // TODO(kenz): we may need to ensure this is a unique id. - id: '${extensionConfig.name}-ext', + id: '${extensionConfig.name}_ext', title: extensionConfig.name.toSentenceCase(), icon: extensionConfig.icon, // TODO(kenz): support static DevTools extensions. @@ -100,10 +100,22 @@ class ExtensionView extends StatelessWidget { children: [ EmbeddedExtensionHeader(extension: extension), Expanded( - child: KeepAliveWrapper( - child: Center( - child: EmbeddedExtensionView(controller: controller), + child: ValueListenableBuilder( + valueListenable: extensionService.enabledStateListenable( + extension.name, ), + builder: (context, activationState, _) { + if (activationState == ExtensionEnabledState.enabled) { + return KeepAliveWrapper( + child: Center( + child: EmbeddedExtensionView(controller: controller), + ), + ); + } + return EnableExtensionPrompt( + extension: controller.extensionConfig, + ); + }, ), ), ], @@ -112,50 +124,6 @@ class ExtensionView extends StatelessWidget { } } -// TODO(kenz): add button to deactivate extension once activate / deactivate -// logic is hooked up. -class EmbeddedExtensionHeader extends StatelessWidget { - const EmbeddedExtensionHeader({super.key, required this.extension}); - - final DevToolsExtensionConfig extension; - - @override - Widget build(BuildContext context) { - final theme = Theme.of(context); - final extensionName = extension.name.toLowerCase(); - return AreaPaneHeader( - title: RichText( - text: TextSpan( - text: 'package:$extensionName extension', - style: theme.regularTextStyle.copyWith(fontWeight: FontWeight.bold), - children: [ - TextSpan( - text: ' (v${extension.version})', - style: theme.subtleTextStyle, - ), - ], - ), - ), - includeTopBorder: false, - roundedTopBorder: false, - rightPadding: defaultSpacing, - actions: [ - RichText( - text: LinkTextSpan( - link: Link( - display: 'Report an issue', - url: extension.issueTrackerLink, - gaScreenName: gac.extensionScreenId, - gaSelectedItemDescription: gac.extensionFeedback(extensionName), - ), - context: context, - ), - ), - ], - ); - } -} - extension ExtensionConfigExtension on DevToolsExtensionConfig { IconData get icon => IconData( materialIconCodePoint, diff --git a/packages/devtools_app/lib/src/extensions/extension_screen_controls.dart b/packages/devtools_app/lib/src/extensions/extension_screen_controls.dart new file mode 100644 index 00000000000..721c90d43a7 --- /dev/null +++ b/packages/devtools_app/lib/src/extensions/extension_screen_controls.dart @@ -0,0 +1,239 @@ +// Copyright 2023 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; + +import 'package:devtools_shared/devtools_extensions.dart'; +import 'package:flutter/material.dart'; + +import '../shared/analytics/constants.dart' as gac; +import '../shared/common_widgets.dart'; +import '../shared/dialogs.dart'; +import '../shared/globals.dart'; +import '../shared/routing.dart'; +import '../shared/theme.dart'; + +class EmbeddedExtensionHeader extends StatelessWidget { + const EmbeddedExtensionHeader({super.key, required this.extension}); + + final DevToolsExtensionConfig extension; + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + final extensionName = extension.displayName; + return AreaPaneHeader( + title: RichText( + text: TextSpan( + text: 'package:$extensionName extension', + style: theme.regularTextStyle.copyWith(fontWeight: FontWeight.bold), + children: [ + TextSpan( + text: ' (v${extension.version})', + style: theme.subtleTextStyle, + ), + ], + ), + ), + includeTopBorder: false, + roundedTopBorder: false, + rightPadding: defaultSpacing, + actions: [ + RichText( + text: LinkTextSpan( + link: Link( + display: 'Report an issue', + url: extension.issueTrackerLink, + gaScreenName: gac.extensionScreenId, + gaSelectedItemDescription: gac.extensionFeedback(extensionName), + ), + context: context, + ), + ), + ValueListenableBuilder( + valueListenable: + extensionService.enabledStateListenable(extension.displayName), + builder: (context, activationState, _) { + if (activationState == ExtensionEnabledState.enabled) { + return Padding( + padding: const EdgeInsets.only(left: denseSpacing), + child: DisableExtensionButton(extension: extension), + ); + } + return const SizedBox.shrink(); + }, + ), + ], + ); + } +} + +@visibleForTesting +class DisableExtensionButton extends StatelessWidget { + const DisableExtensionButton({super.key, required this.extension}); + + final DevToolsExtensionConfig extension; + + @override + Widget build(BuildContext context) { + return DevToolsButton.iconOnly( + icon: Icons.extension_off_outlined, + outlined: false, + tooltip: 'Disable extension', + gaScreen: gac.extensionScreenId, + gaSelection: gac.extensionDisable(extension.displayName), + onPressed: () => unawaited( + showDialog( + context: context, + builder: (_) => DisableExtensionDialog(extension: extension), + ), + ), + ); + } +} + +@visibleForTesting +class DisableExtensionDialog extends StatelessWidget { + const DisableExtensionDialog({super.key, required this.extension}); + + final DevToolsExtensionConfig extension; + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + return DevToolsDialog( + title: const DialogTitleText('Disable extension?'), + content: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + RichText( + text: TextSpan( + text: 'Are you sure you want to disable the ', + style: theme.regularTextStyle, + children: [ + TextSpan( + text: extension.displayName, + style: theme.fixedFontStyle, + ), + const TextSpan(text: ' extension?'), + ], + ), + ), + const SizedBox(height: denseSpacing), + RichText( + text: TextSpan( + text: 'You can always re-enable this extension later from the ', + style: theme.regularTextStyle, + children: [ + TextSpan( + text: 'DevTools Extensions ', + style: theme.boldTextStyle, + ), + WidgetSpan( + child: Icon( + Icons.extension_rounded, + size: defaultIconSize, + ), + ), + const TextSpan(text: ' menu.'), + ], + ), + ), + ], + ), + actions: [ + DialogTextButton( + onPressed: () { + unawaited( + extensionService.setExtensionEnabledState( + extension, + enable: false, + ), + ); + Navigator.of(context).pop(dialogDefaultContext); + DevToolsRouterDelegate.of(context) + .navigateHome(clearScreenParam: true); + }, + child: const Text('YES, DISABLE'), + ), + const DialogCancelButton(), + ], + ); + } +} + +class EnableExtensionPrompt extends StatelessWidget { + const EnableExtensionPrompt({ + super.key, + required this.extension, + }); + + final DevToolsExtensionConfig extension; + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + final extensionName = extension.displayName; + return Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + RichText( + text: TextSpan( + text: 'The ', + style: theme.regularTextStyle, + children: [ + TextSpan( + text: extension.name, + style: theme.fixedFontStyle, + ), + const TextSpan( + text: ' extension has not been enabled. Do you want to enable' + ' this extension?', + ), + ], + ), + ), + const SizedBox(height: defaultSpacing), + Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + DevToolsButton( + label: 'Enable', + gaScreen: gac.extensionScreenId, + gaSelection: gac.extensionEnable(extensionName), + elevatedButton: true, + onPressed: () { + unawaited( + extensionService.setExtensionEnabledState( + extension, + enable: true, + ), + ); + }, + ), + const SizedBox(width: defaultSpacing), + DevToolsButton( + label: 'No, hide this screen', + gaScreen: gac.extensionScreenId, + gaSelection: gac.extensionDisable(extensionName), + onPressed: () { + unawaited( + extensionService.setExtensionEnabledState( + extension, + enable: false, + ), + ); + DevToolsRouterDelegate.of(context) + .navigateHome(clearScreenParam: true); + }, + ), + ], + ), + ], + ), + ); + } +} diff --git a/packages/devtools_app/lib/src/extensions/extension_settings.dart b/packages/devtools_app/lib/src/extensions/extension_settings.dart index f6048253865..9dc6abe746d 100644 --- a/packages/devtools_app/lib/src/extensions/extension_settings.dart +++ b/packages/devtools_app/lib/src/extensions/extension_settings.dart @@ -181,20 +181,20 @@ class ExtensionSetting extends StatelessWidget { fillColor: theme.colorScheme.primary, selectedColor: theme.colorScheme.onPrimary, onPressed: (index) => buttonStates[index].onPressed(), - selectedStates: buttonStates - .map((option) => option.isSelected(enabledState)) - .toList(), - children: buttonStates - .map( - (option) => Padding( - padding: const EdgeInsets.symmetric( - vertical: densePadding, - horizontal: denseSpacing, - ), - child: Text(option.title), + selectedStates: [ + for (final state in buttonStates) + state.isSelected(enabledState), + ], + children: [ + for (final state in buttonStates) + Padding( + padding: const EdgeInsets.symmetric( + vertical: densePadding, + horizontal: denseSpacing, ), - ) - .toList(), + child: Text(state.title), + ), + ], ), ], ), diff --git a/packages/devtools_app/lib/src/shared/theme.dart b/packages/devtools_app/lib/src/shared/theme.dart index 6ebfcb5f5ff..c94e507b987 100644 --- a/packages/devtools_app/lib/src/shared/theme.dart +++ b/packages/devtools_app/lib/src/shared/theme.dart @@ -415,6 +415,9 @@ extension ThemeDataExtension on ThemeData { ), ); + TextStyle get boldTextStyle => + regularTextStyle.copyWith(fontWeight: FontWeight.bold); + TextStyle get subtleTextStyle => _fixBlurryText( TextStyle( color: colorScheme.subtleTextColor, diff --git a/packages/devtools_app/test/extensions/extension_screen_test.dart b/packages/devtools_app/test/extensions/extension_screen_test.dart index e7c618dac6b..71355f14c34 100644 --- a/packages/devtools_app/test/extensions/extension_screen_test.dart +++ b/packages/devtools_app/test/extensions/extension_screen_test.dart @@ -5,6 +5,8 @@ import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/extensions/embedded/view.dart'; import 'package:devtools_app/src/extensions/extension_screen.dart'; +import 'package:devtools_app/src/extensions/extension_screen_controls.dart'; +import 'package:devtools_shared/devtools_extensions.dart'; import 'package:devtools_test/devtools_test.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -13,18 +15,23 @@ import '../test_infra/test_data/extensions.dart'; void main() { const windowSize = Size(2000.0, 2000.0); - group('Extension screen', () { + group('$ExtensionScreen', () { late ExtensionScreen fooScreen; late ExtensionScreen barScreen; late ExtensionScreen providerScreen; - setUp(() { + setUp(() async { setGlobal(IdeTheme, IdeTheme()); setGlobal(PreferencesController, PreferencesController()); setGlobal(ServiceConnectionManager, ServiceConnectionManager()); fooScreen = ExtensionScreen(fooExtension); barScreen = ExtensionScreen(barExtension); providerScreen = ExtensionScreen(providerExtension); + + setGlobal( + ExtensionService, + await createMockExtensionServiceWithDefaults(testExtensions), + ); }); testWidgets('builds its tab', (WidgetTester tester) async { @@ -42,7 +49,7 @@ void main() { }); testWidgetsWithWindowSize( - 'renders as expected', + 'renders for unactivated state', windowSize, (tester) async { await tester.pumpWidget(wrap(Builder(builder: fooScreen.build))); @@ -54,7 +61,9 @@ void main() { ); expect(find.richTextContaining('(v1.0.0)'), findsOneWidget); expect(find.richTextContaining('Report an issue'), findsOneWidget); - expect(find.byType(EmbeddedExtensionView), findsOneWidget); + expect(find.byType(DisableExtensionButton), findsNothing); + expect(find.byType(EnableExtensionPrompt), findsOneWidget); + expect(find.byType(EmbeddedExtensionView), findsNothing); await tester.pumpWidget(wrap(Builder(builder: barScreen.build))); expect(find.byType(ExtensionView), findsOneWidget); @@ -65,7 +74,9 @@ void main() { ); expect(find.richTextContaining('(v2.0.0)'), findsOneWidget); expect(find.richTextContaining('Report an issue'), findsOneWidget); - expect(find.byType(EmbeddedExtensionView), findsOneWidget); + expect(find.byType(DisableExtensionButton), findsNothing); + expect(find.byType(EnableExtensionPrompt), findsOneWidget); + expect(find.byType(EmbeddedExtensionView), findsNothing); await tester.pumpWidget(wrap(Builder(builder: providerScreen.build))); expect(find.byType(ExtensionView), findsOneWidget); @@ -76,8 +87,106 @@ void main() { ); expect(find.richTextContaining('(v3.0.0)'), findsOneWidget); expect(find.richTextContaining('Report an issue'), findsOneWidget); + expect(find.byType(DisableExtensionButton), findsNothing); + expect(find.byType(EnableExtensionPrompt), findsOneWidget); + expect(find.byType(EmbeddedExtensionView), findsNothing); + }, + ); + + testWidgetsWithWindowSize( + 'renders for enabled state', + windowSize, + (tester) async { + await extensionService.setExtensionEnabledState( + fooExtension, + enable: true, + ); + + await tester.pumpWidget(wrap(Builder(builder: fooScreen.build))); + expect(find.byType(ExtensionView), findsOneWidget); + expect(find.byType(EmbeddedExtensionHeader), findsOneWidget); + expect( + find.richTextContaining('package:foo extension'), + findsOneWidget, + ); + expect(find.richTextContaining('(v1.0.0)'), findsOneWidget); + expect(find.richTextContaining('Report an issue'), findsOneWidget); + expect(find.byType(DisableExtensionButton), findsOneWidget); + expect(find.byType(EnableExtensionPrompt), findsNothing); expect(find.byType(EmbeddedExtensionView), findsOneWidget); }, ); + + testWidgetsWithWindowSize( + 'renders for disabled state', + windowSize, + (tester) async { + await extensionService.setExtensionEnabledState( + fooExtension, + enable: false, + ); + + await tester.pumpWidget(wrap(Builder(builder: fooScreen.build))); + expect(find.byType(ExtensionView), findsOneWidget); + expect(find.byType(EmbeddedExtensionHeader), findsOneWidget); + expect( + find.richTextContaining('package:foo extension'), + findsOneWidget, + ); + expect(find.richTextContaining('(v1.0.0)'), findsOneWidget); + expect(find.richTextContaining('Report an issue'), findsOneWidget); + expect(find.byType(DisableExtensionButton), findsNothing); + expect(find.byType(EnableExtensionPrompt), findsOneWidget); + expect(find.byType(EmbeddedExtensionView), findsNothing); + }, + ); + + testWidgetsWithWindowSize( + 'can enable and disable extension from screen', + windowSize, + (tester) async { + await tester.pumpWidget(wrap(Builder(builder: fooScreen.build))); + + expect( + extensionService.enabledStateListenable(fooExtension.name).value, + ExtensionEnabledState.none, + ); + expect(find.byType(EnableExtensionPrompt), findsOneWidget); + expect(find.byType(EmbeddedExtensionView), findsNothing); + expect(find.byType(DisableExtensionButton), findsNothing); + + await tester.tap( + find.descendant( + of: find.byType(DevToolsButton), + matching: find.text('Enable'), + ), + ); + await tester.pumpAndSettle(); + + expect( + extensionService.enabledStateListenable(fooExtension.name).value, + ExtensionEnabledState.enabled, + ); + expect(find.byType(EnableExtensionPrompt), findsNothing); + expect(find.byType(EmbeddedExtensionView), findsOneWidget); + expect(find.byType(DisableExtensionButton), findsOneWidget); + + await tester.tap(find.byType(DisableExtensionButton)); + await tester.pumpAndSettle(); + + expect(find.byType(DisableExtensionDialog), findsOneWidget); + + await tester.tap(find.text('YES, DISABLE')); + await tester.pumpAndSettle(); + + expect( + extensionService.enabledStateListenable(fooExtension.name).value, + ExtensionEnabledState.disabled, + ); + expect(find.byType(EnableExtensionPrompt), findsOneWidget); + expect(find.byType(EmbeddedExtensionView), findsNothing); + expect(find.byType(DisableExtensionButton), findsNothing); + }, + ); }); } diff --git a/packages/devtools_app/test/extensions/extension_settings_test.dart b/packages/devtools_app/test/extensions/extension_settings_test.dart index cdd67d0d310..ced8a66e03f 100644 --- a/packages/devtools_app/test/extensions/extension_settings_test.dart +++ b/packages/devtools_app/test/extensions/extension_settings_test.dart @@ -8,7 +8,6 @@ import 'package:devtools_shared/devtools_extensions.dart'; import 'package:devtools_test/devtools_test.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:mockito/mockito.dart'; import '../test_infra/matchers/matchers.dart'; import '../test_infra/test_data/extensions.dart'; @@ -16,49 +15,23 @@ import '../test_infra/test_data/extensions.dart'; void main() { late ExtensionSettingsDialog dialog; - final stubEnabledStates = >{}; - - Future setUpExtensionService( - List extensions, - // ignore: avoid-redundant-async, false positive - ) async { - final mockExtensionService = MockExtensionService(); - setGlobal(ExtensionService, mockExtensionService); - when(mockExtensionService.availableExtensions) - .thenReturn(ImmediateValueNotifier(extensions)); - - stubEnabledStates.clear(); - for (final e in extensions) { - stubEnabledStates[e.name.toLowerCase()] = - ValueNotifier(ExtensionEnabledState.none); - when(mockExtensionService.enabledStateListenable(e.name)) - .thenReturn(stubEnabledStates[e.name.toLowerCase()]!); - when(mockExtensionService.enabledStateListenable(e.name.toLowerCase())) - .thenReturn(stubEnabledStates[e.name.toLowerCase()]!); - when(mockExtensionService.setExtensionEnabledState(e, enable: true)) - .thenAnswer((_) async { - stubEnabledStates[e.name.toLowerCase()]!.value = - ExtensionEnabledState.enabled; - }); - when(mockExtensionService.setExtensionEnabledState(e, enable: false)) - .thenAnswer((_) async { - stubEnabledStates[e.name.toLowerCase()]!.value = - ExtensionEnabledState.disabled; - }); - } - } - group('$ExtensionSettingsDialog', () { setUp(() async { dialog = const ExtensionSettingsDialog(); - await setUpExtensionService(testExtensions); + setGlobal( + ExtensionService, + await createMockExtensionServiceWithDefaults(testExtensions), + ); setGlobal(IdeTheme, IdeTheme()); }); testWidgets( 'builds dialog with no available extensions', (WidgetTester tester) async { - await setUpExtensionService([]); + setGlobal( + ExtensionService, + await createMockExtensionServiceWithDefaults([]), + ); await tester.pumpWidget(wrap(dialog)); expect(find.text('DevTools Extensions'), findsOneWidget); expect( diff --git a/packages/devtools_shared/lib/src/extensions/extension_model.dart b/packages/devtools_shared/lib/src/extensions/extension_model.dart index db932dc3ff6..55c03dc4844 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_model.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_model.dart @@ -103,6 +103,8 @@ class DevToolsExtensionConfig implements Comparable { /// This code point should be part of the 'MaterialIcons' font family. final int materialIconCodePoint; + String get displayName => name.toLowerCase(); + Map toJson() => { nameKey: name, pathKey: path, diff --git a/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart b/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart index a34866d6c41..9a84ad76601 100644 --- a/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart +++ b/packages/devtools_test/lib/src/mocks/generated_mocks_factories.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'package:devtools_app/devtools_app.dart'; +import 'package:devtools_shared/devtools_extensions.dart'; import 'package:flutter/material.dart'; import 'package:mockito/mockito.dart'; import 'package:vm_service/vm_service.dart' hide TimelineEvent; @@ -181,3 +182,32 @@ MockLoggingController createMockLoggingControllerWithDefaults({ when(mockLoggingController.matchIndex).thenReturn(ValueNotifier(0)); return mockLoggingController; } + +Future createMockExtensionServiceWithDefaults( + List extensions, +) async { + final mockExtensionService = MockExtensionService(); + when(mockExtensionService.availableExtensions) + .thenReturn(ImmediateValueNotifier(extensions)); + + final _stubEnabledStates = >{}; + for (final e in extensions) { + _stubEnabledStates[e.name.toLowerCase()] = + ValueNotifier(ExtensionEnabledState.none); + when(mockExtensionService.enabledStateListenable(e.name)) + .thenReturn(_stubEnabledStates[e.name.toLowerCase()]!); + when(mockExtensionService.enabledStateListenable(e.name.toLowerCase())) + .thenReturn(_stubEnabledStates[e.name.toLowerCase()]!); + when(mockExtensionService.setExtensionEnabledState(e, enable: true)) + .thenAnswer((_) async { + _stubEnabledStates[e.name.toLowerCase()]!.value = + ExtensionEnabledState.enabled; + }); + when(mockExtensionService.setExtensionEnabledState(e, enable: false)) + .thenAnswer((_) async { + _stubEnabledStates[e.name.toLowerCase()]!.value = + ExtensionEnabledState.disabled; + }); + } + return mockExtensionService; +}