Clean up Logging page top bar (#6281)

![](https://media.giphy.com/media/3ov9k3Pq54ky2rNyp2/giphy-downsized.gif) Fixes #4921 https://github.com/flutter/devtools/assets/1386322/ebe4f959-72b3-4d02-a755-a1bbef7fb577 # Open question The structured errors are saved in the service, not in the local preferences. Should we have a way to somehow let the user know that this setting is saved in their app?
flutter · Aug 30, 2023 · 86bbc8a · 86bbc8a
1 parent 3b8a462
commit 86bbc8a
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 20 deletions.
diff --git a/packages/devtools_app/lib/src/screens/logging/logging_screen.dart b/packages/devtools_app/lib/src/screens/logging/logging_screen.dart
@@ -21,6 +21,7 @@ import '../../shared/utils.dart';
 import '_log_details.dart';
 import '_logs_table.dart';
 import 'logging_controller.dart';
+import 'shared/constants.dart';
 
 /// Presents logs from the connected app.
 class LoggingScreen extends Screen {
@@ -123,13 +124,15 @@ class _LoggingScreenState extends State<LoggingScreenBody>
           onPressed: controller.clear,
           gaScreen: gac.logging,
           gaSelection: gac.clear,
+          minScreenWidthForTextBeforeScaling: loggingMinVerboseWidth,
         ),
         const Spacer(),
-        const StructuredErrorsToggle(),
         const SizedBox(width: denseSpacing),
         // TODO(kenz): fix focus issue when state is refreshed
         SearchField<LoggingController>(
-          searchFieldWidth: wideSearchFieldWidth,
+          searchFieldWidth: isScreenWiderThan(context, loggingMinVerboseWidth)
+              ? wideSearchFieldWidth
+              : defaultSearchFieldWidth,
           searchController: controller,
           searchFieldEnabled: hasData,
         ),
@@ -145,6 +148,20 @@ class _LoggingScreenState extends State<LoggingScreenBody>
               .joinWithTrailing('\n'),
           tooltip: 'Copy filtered logs',
         ),
+        const SizedBox(width: denseSpacing),
+        SettingsOutlinedButton(
+          gaScreen: gac.logging,
+          gaSelection: gac.loggingSettings,
+          tooltip: 'Logging Settings',
+          onPressed: () {
+            unawaited(
+              showDialog(
+                context: context,
+                builder: (context) => const LoggingSettingsDialog(),
+              ),
+            );
+          },
+        ),
       ],
     );
   }
@@ -189,3 +206,32 @@ class _LoggingScreenState extends State<LoggingScreenBody>
     );
   }
 }
+
+class LoggingSettingsDialog extends StatelessWidget {
+  const LoggingSettingsDialog({super.key});
+
+  @override
+  Widget build(BuildContext context) {
+    final theme = Theme.of(context);
+    return DevToolsDialog(
+      title: const DialogTitleText('Logging Settings'),
+      content: SizedBox(
+        width: defaultDialogWidth,
+        child: Column(
+          mainAxisSize: MainAxisSize.min,
+          crossAxisAlignment: CrossAxisAlignment.start,
+          children: [
+            ...dialogSubHeader(
+              theme,
+              'General',
+            ),
+            const StructuredErrorsToggle(),
+          ],
+        ),
+      ),
+      actions: const [
+        DialogCloseButton(),
+      ],
+    );
+  }
+}
diff --git a/packages/devtools_app/lib/src/screens/logging/shared/constants.dart b/packages/devtools_app/lib/src/screens/logging/shared/constants.dart
@@ -0,0 +1,5 @@
+// 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.
+
+const loggingMinVerboseWidth = 650.0;
diff --git a/packages/devtools_app/lib/src/service/service_extension_widgets.dart b/packages/devtools_app/lib/src/service/service_extension_widgets.dart
@@ -184,9 +184,10 @@ class ServiceExtensionButton extends StatelessWidget {
       child: Container(
         height: defaultButtonHeight,
         padding: EdgeInsets.symmetric(
-          horizontal: includeText(context, minScreenWidthForTextBeforeScaling)
-              ? defaultSpacing
-              : 0.0,
+          horizontal:
+              isScreenWiderThan(context, minScreenWidthForTextBeforeScaling)
+                  ? defaultSpacing
+                  : 0.0,
         ),
         child: ImageIconLabel(
           ServiceExtensionIcon(extensionState: extensionState),

diff --git a/packages/devtools_app/lib/src/shared/analytics/constants.dart b/packages/devtools_app/lib/src/shared/analytics/constants.dart
@@ -68,6 +68,7 @@ const enableOnDeviceInspector = 'enableOnDeviceInspector';
 const showOnDeviceInspector = 'showInspector';
 const treeNodeSelection = 'treeNodeSelection';
 const inspectorSettings = 'inspectorSettings';
+const loggingSettings = 'loggingSettings';
 const refreshPubRoots = 'refreshPubRoots';
 
 enum HomeScreenEvents {

diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
@@ -27,6 +27,8 @@ TODO: Remove this section if there are not any general updates.
 TODO: Remove this section if there are not any general updates.
 
 ## Logging updates
+* Improved responsiveness of the top bar in the Logging page - [#6281](https://github.com/flutter/devtools/pull/6281)
+
 * Add the ability to copy filtered logs - [#6260](https://github.com/flutter/devtools/pull/6260)
 ![Logger tab copy](images/logger_copy.png "Logger tab copy")
 ## App size tool updates

diff --git a/packages/devtools_app/test/logging/logging_screen_test.dart b/packages/devtools_app/test/logging/logging_screen_test.dart
@@ -108,7 +108,7 @@ void main() {
         expect(find.byType(ClearButton), findsOneWidget);
         expect(find.byType(TextField), findsOneWidget);
         expect(find.byType(DevToolsFilterButton), findsOneWidget);
-        expect(find.byType(StructuredErrorsToggle), findsOneWidget);
+        expect(find.byType(SettingsOutlinedButton), findsOneWidget);
       },
     );
 
@@ -151,7 +151,15 @@ void main() {
           serviceConnection,
         );
         await pumpLoggingScreen(tester);
-        Switch toggle = tester.widget(find.byType(Switch));
+
+        await tester.tap(find.byType(SettingsOutlinedButton));
+        await tester.pump();
+        Switch toggle = tester.widget(
+          find.descendant(
+            of: find.byType(StructuredErrorsToggle),
+            matching: find.byType(Switch),
+          ),
+        );
         expect(toggle.value, false);
 
         serviceConnection.serviceManager.serviceExtensionManager

diff --git a/packages/devtools_app_shared/lib/src/ui/common.dart b/packages/devtools_app_shared/lib/src/ui/common.dart
@@ -404,7 +404,7 @@ class DevToolsButton extends StatelessWidget {
       tooltipPadding: tooltipPadding,
       child: SizedBox(
         height: defaultButtonHeight,
-        width: !includeText(context, minScreenWidthForTextBeforeScaling)
+        width: !isScreenWiderThan(context, minScreenWidthForTextBeforeScaling)
             ? buttonMinWidth
             : null,
         child: outlined
@@ -613,7 +613,7 @@ final class ImageIconLabel extends StatelessWidget {
       children: [
         icon,
         // TODO(jacobr): animate showing and hiding the text.
-        if (includeText(context, unscaledMinIncludeTextWidth))
+        if (isScreenWiderThan(context, unscaledMinIncludeTextWidth))
           Padding(
             padding: const EdgeInsets.only(left: 8.0),
             child: Text(text),
@@ -655,7 +655,7 @@ final class MaterialIconLabel extends StatelessWidget {
           ),
         // TODO(jacobr): animate showing and hiding the text.
         if (label != null &&
-            includeText(context, minScreenWidthForTextBeforeScaling))
+            isScreenWiderThan(context, minScreenWidthForTextBeforeScaling))
           Padding(
             padding: EdgeInsets.only(
               left: iconData != null ? denseSpacing : 0.0,

diff --git a/packages/devtools_app_shared/lib/src/ui/theme/theme.dart b/packages/devtools_app_shared/lib/src/ui/theme/theme.dart
@@ -487,18 +487,15 @@ const defaultCurve = Curves.easeInOutCubic;
 CurvedAnimation defaultCurvedAnimation(AnimationController parent) =>
     CurvedAnimation(curve: defaultCurve, parent: parent);
 
-/// Measures the screen size to determine whether text should be included for
-/// the value of [minScreenWidthForTextBeforeScaling].
-///
-/// Returns false if the screen width is smaller than minimum size required to
-/// include text [minScreenWidthForTextBeforeScaling].
-bool includeText(
+/// Measures the screen size to determine whether it is strictly larger
+/// than [width], scaled to the current font factor.
+bool isScreenWiderThan(
   BuildContext context,
-  double? minScreenWidthForTextBeforeScaling,
+  double? width,
 ) {
-  return minScreenWidthForTextBeforeScaling == null ||
+  return width == null ||
       MediaQuery.of(context).size.width >
-          scaleByFontFactor(minScreenWidthForTextBeforeScaling);
+          scaleByFontFactor(width);
 }
 
 ButtonStyle denseAwareOutlinedButtonStyle(
@@ -532,7 +529,7 @@ ButtonStyle _generateButtonStyle({
   required ButtonStyle buttonStyle,
   double? minScreenWidthForTextBeforeScaling,
 }) {
-  if (!includeText(context, minScreenWidthForTextBeforeScaling)) {
+  if (!isScreenWiderThan(context, minScreenWidthForTextBeforeScaling)) {
     buttonStyle = buttonStyle.copyWith(
       padding: MaterialStateProperty.resolveWith<EdgeInsets>((_) {
         return EdgeInsets.zero;