Store the logging details format in user preferences (#8446)

packages/devtools_app/lib/src/screens/logging/_log_details.dart

@@ -5,9 +5,12 @@
 import 'dart:async';
 
 import 'package:devtools_app_shared/ui.dart';
+import 'package:devtools_app_shared/utils.dart';
 import 'package:flutter/material.dart';
 
 import '../../shared/common_widgets.dart';
+import '../../shared/globals.dart';
+import '../../shared/preferences/preferences.dart';
 import 'logging_controller.dart';
 
 class LogDetails extends StatefulWidget {
@@ -23,18 +26,15 @@ class LogDetails extends StatefulWidget {
 }
 
 class _LogDetailsState extends State<LogDetails>
-    with SingleTickerProviderStateMixin {
+    with AutoDisposeMixin<LogDetails>, SingleTickerProviderStateMixin {
   String? _lastDetails;
   late final ScrollController scrollController;
 
-  // TODO(kenz): store this as a setting in logging preferences instead of in
-  // this state class.
-  bool showDetailsAsText = true;
-
   @override
   void initState() {
     super.initState();
     scrollController = ScrollController();
+    addAutoDisposeListener(preferences.logging.detailsFormat);
     unawaited(_computeLogDetails());
   }
 
@@ -79,18 +79,14 @@ class _LogDetailsState extends State<LogDetails>
     return DevToolsAreaPane(
       header: _LogDetailsHeader(
         log: log,
-        showDetailsAsText: showDetailsAsText,
-        onDetailsFormatPressed: (value) {
-          setState(() {
-            showDetailsAsText = value;
-          });
-        },
+        format: preferences.logging.detailsFormat.value,
       ),
       child: Scrollbar(
         controller: scrollController,
         child: SingleChildScrollView(
           controller: scrollController,
-          child: showDetailsAsText
+          child: preferences.logging.detailsFormat.value ==
+                  LoggingDetailsFormat.text
               ? Padding(
                   padding: const EdgeInsets.all(denseSpacing),
                   child: SelectableText(
@@ -106,15 +102,10 @@ class _LogDetailsState extends State<LogDetails>
 }
 
 class _LogDetailsHeader extends StatelessWidget {
-  const _LogDetailsHeader({
-    required this.log,
-    required this.showDetailsAsText,
-    required this.onDetailsFormatPressed,
-  });
+  const _LogDetailsHeader({required this.log, required this.format});
 
   final LogData? log;
-  final bool showDetailsAsText;
-  final void Function(bool) onDetailsFormatPressed;
+  final LoggingDetailsFormat format;
 
   @override
   Widget build(BuildContext context) {
@@ -127,10 +118,7 @@ class _LogDetailsHeader extends StatelessWidget {
       includeTopBorder: false,
       roundedTopBorder: false,
       actions: [
-        LogDetailsFormatButton(
-          showDetailsAsText: showDetailsAsText,
-          onPressed: onDetailsFormatPressed,
-        ),
+        LogDetailsFormatButton(format: format),
         const SizedBox(width: densePadding),
         CopyToClipboardControl(
           dataProvider: dataProvider,
@@ -143,29 +131,28 @@ class _LogDetailsHeader extends StatelessWidget {
 
 @visibleForTesting
 class LogDetailsFormatButton extends StatelessWidget {
-  const LogDetailsFormatButton({
-    super.key,
-    required this.showDetailsAsText,
-    required this.onPressed,
-  });
+  const LogDetailsFormatButton({super.key, required this.format});
 
-  final bool showDetailsAsText;
-  final void Function(bool) onPressed;
+  final LoggingDetailsFormat format;
 
   static const viewAsJsonTooltip = 'View as JSON';
   static const viewAsRawTextTooltip = 'View as raw text';
 
   @override
   Widget build(BuildContext context) {
+    final currentlyUsingTextFormat = format == LoggingDetailsFormat.text;
     final tooltip =
-        showDetailsAsText ? viewAsJsonTooltip : viewAsRawTextTooltip;
-    return showDetailsAsText
+        currentlyUsingTextFormat ? viewAsJsonTooltip : viewAsRawTextTooltip;
+    void togglePreference() =>
+        preferences.logging.detailsFormat.value = format.opposite();
+
+    return currentlyUsingTextFormat
         ? Padding(
             // This padding aligns this button with the copy button.
             padding: const EdgeInsets.only(bottom: borderPadding),
             child: SmallAction(
               tooltip: tooltip,
-              onPressed: () => onPressed(!showDetailsAsText),
+              onPressed: togglePreference,
               child: Text(
                 ' { } ',
                 style: Theme.of(context).regularTextStyle,
@@ -175,7 +162,7 @@ class LogDetailsFormatButton extends StatelessWidget {
         : ToolbarAction(
             icon: Icons.text_fields,
             tooltip: tooltip,
-            onPressed: () => onPressed(!showDetailsAsText),
+            onPressed: togglePreference,
             size: defaultIconSize,
           );
   }

packages/devtools_app/lib/src/shared/analytics/constants/_logging_constants.dart

@@ -7,4 +7,5 @@ part of '../constants.dart';
 /// Logging event constants specific for logging screen.
 enum LoggingEvents {
   changeRetentionLimit,
+  changeDetailsFormat,
 }

packages/devtools_app/lib/src/shared/preferences/_logging_preferences.dart

@@ -8,12 +8,22 @@ class LoggingPreferencesController extends DisposableController
     with AutoDisposeControllerMixin {
   final retentionLimitTitle = 'Limit for the number of logs retained.';
 
+  // TODO(kenz): remove the retention limit setting if we cannot apply this
+  // functionality to the existing logging page, since the logging V2 code may
+  // be removed.
   /// The number of logs to retain on the logging table.
   final retentionLimit = ValueNotifier<int>(_defaultRetentionLimit);
 
+  /// The [LoggingDetailsFormat] to use when displaying a log in the log details
+  /// view.
+  final detailsFormat =
+      ValueNotifier<LoggingDetailsFormat>(_defaultDetailsFormat);
+
   static const _defaultRetentionLimit = 3000;
+  static const _defaultDetailsFormat = LoggingDetailsFormat.text;
 
   static const _retentionLimitStorageId = 'logging.retentionLimit';
+  static const _detailsFormatStorageId = 'logging.detailsFormat';
 
   Future<void> init() async {
     retentionLimit.value =
@@ -27,13 +37,45 @@ class LoggingPreferencesController extends DisposableController
           _retentionLimitStorageId,
           retentionLimit.value.toString(),
         );
-
         ga.select(
           gac.logging,
           gac.LoggingEvents.changeRetentionLimit.name,
           value: retentionLimit.value,
         );
       },
     );
+
+    final detailsFormatValueFromStorage =
+        await storage.getValue(_detailsFormatStorageId);
+    detailsFormat.value = LoggingDetailsFormat.values.firstWhereOrNull(
+          (value) => detailsFormatValueFromStorage == value.name,
+        ) ??
+        _defaultDetailsFormat;
+
+    addAutoDisposeListener(
+      detailsFormat,
+      () {
+        storage.setValue(_detailsFormatStorageId, detailsFormat.value.name);
+        ga.select(
+          gac.logging,
+          gac.LoggingEvents.changeDetailsFormat.name,
+          value: detailsFormat.value.index,
+        );
+      },
+    );
+  }
+}
+
+enum LoggingDetailsFormat {
+  json,
+  text;
+
+  LoggingDetailsFormat opposite() {
+    switch (this) {
+      case LoggingDetailsFormat.json:
+        return LoggingDetailsFormat.text;
+      case LoggingDetailsFormat.text:
+        return LoggingDetailsFormat.json;
+    }
   }
 }

packages/devtools_app/lib/src/shared/preferences/preferences.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'dart:convert';
 
+import 'package:collection/collection.dart';
 import 'package:devtools_app_shared/ui.dart';
 import 'package:devtools_app_shared/utils.dart';
 import 'package:flutter/foundation.dart';

packages/devtools_app/test/shared/preferences_controller_test.dart

@@ -400,6 +400,57 @@ void main() {
     });
   });
 
+  group('$LoggingPreferencesController', () {
+    late LoggingPreferencesController controller;
+    late FlutterTestStorage storage;
+
+    setUp(() async {
+      setGlobal(Storage, storage = FlutterTestStorage());
+      controller = LoggingPreferencesController();
+      await controller.init();
+    });
+
+    test('has expected default values', () {
+      expect(controller.detailsFormat.value, LoggingDetailsFormat.text);
+    });
+
+    test('stores values and reads them on init', () async {
+      storage.values.clear();
+
+      // Remember original values.
+      final detailsFormat = controller.detailsFormat.value;
+
+      // Flip the values in controller.
+      controller.detailsFormat.value = detailsFormat.opposite();
+
+      // Check the values are stored.
+      expect(storage.values, hasLength(1));
+
+      // Reload the values from storage.
+      await controller.init();
+
+      // Check they did not change back to default.
+      expect(
+        controller.detailsFormat.value,
+        detailsFormat.opposite(),
+      );
+
+      // Flip the values in storage.
+      for (final key in storage.values.keys) {
+        storage.values[key] = detailsFormat.name;
+      }
+
+      // Reload the values from storage.
+      await controller.init();
+
+      // Check they flipped values are loaded.
+      expect(
+        controller.detailsFormat.value,
+        detailsFormat,
+      );
+    });
+  });
+
   group('$PerformancePreferencesController', () {
     late PerformancePreferencesController controller;
     late FlutterTestStorage storage;