Remove denseMode setting and clean up dense UI (flutter#7086)

kenzieschmoll · Jan 23, 2024 · eca685b · eca685b
1 parent 59312ff
commit eca685b
Show file tree

Hide file tree

Showing 61 changed files with 118 additions and 164 deletions.
diff --git a/packages/analysis_options.yaml b/packages/analysis_options.yaml
@@ -218,7 +218,6 @@ dart_code_metrics:
     #    allowed-duplicated-chains: 2
     # - prefer-static-class
     - prefer-trailing-comma
-    - tag-name
     - always-remove-listener
     # - avoid-border-all Micro-optimization to avoid a const constructor.
     # - avoid-returning-widgets  This one is nice but has a lot of false positives.

diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart
@@ -119,9 +119,6 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
 
   bool _isDarkThemeEnabledPreference = true;
 
-  bool get denseModeEnabled => _denseModeEnabled;
-  bool _denseModeEnabled = false;
-
   final hoverCardController = HoverCardController();
 
   late ReleaseNotesController releaseNotesController;
@@ -171,13 +168,6 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
       });
     });
 
-    _denseModeEnabled = preferences.denseModeEnabled.value;
-    addAutoDisposeListener(preferences.denseModeEnabled, () {
-      setState(() {
-        _denseModeEnabled = preferences.denseModeEnabled.value;
-      });
-    });
-
     releaseNotesController = ReleaseNotesController();
   }
 

diff --git a/packages/devtools_app/lib/src/framework/settings_dialog.dart b/packages/devtools_app/lib/src/framework/settings_dialog.dart
@@ -53,14 +53,6 @@ class SettingsDialog extends StatelessWidget {
               gaItem: gac.darkTheme,
             ),
           ),
-          Flexible(
-            child: CheckboxSetting(
-              title: 'Use dense mode',
-              notifier: preferences.denseModeEnabled,
-              onChanged: preferences.toggleDenseMode,
-              gaItem: gac.denseMode,
-            ),
-          ),
           if (isExternalBuild && isDevToolsServerAvailable)
             Flexible(
               child: CheckboxSetting(

diff --git a/packages/devtools_app/lib/src/screens/debugger/codeview.dart b/packages/devtools_app/lib/src/screens/debugger/codeview.dart
@@ -63,8 +63,7 @@ class CodeView extends StatefulWidget {
   static const debuggerCodeViewVerticalScrollbarKey =
       Key('debuggerCodeViewVerticalScrollbarKey');
 
-  static double get rowHeight =>
-      isDense() ? scaleByFontFactor(16.0) : scaleByFontFactor(20.0);
+  static double get rowHeight => scaleByFontFactor(16.0);
 
   final CodeViewController codeViewController;
   final DebuggerController? debuggerController;

diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_breadcrumbs.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_breadcrumbs.dart
@@ -5,10 +5,10 @@
 import 'package:devtools_app_shared/ui.dart';
 import 'package:flutter/material.dart';
 
+import '../../shared/common_widgets.dart';
 import '../../shared/console/eval/inspector_tree.dart';
 import '../../shared/diagnostics_text_styles.dart';
 import '../../shared/primitives/utils.dart';
-import '../../shared/utils.dart';
 
 class InspectorBreadcrumbNavigator extends StatelessWidget {
   const InspectorBreadcrumbNavigator({
@@ -33,7 +33,7 @@ class InspectorBreadcrumbNavigator extends StatelessWidget {
 
     final breadcrumbs = _generateBreadcrumbs(items);
     return SizedBox(
-      height: isDense() ? 24 : 28,
+      height: Breadcrumb.height,
       child: Padding(
         padding: const EdgeInsets.symmetric(horizontal: 6),
         child: Row(

diff --git a/packages/devtools_app/lib/src/screens/inspector/layout_explorer/flex/flex.dart b/packages/devtools_app/lib/src/screens/inspector/layout_explorer/flex/flex.dart
@@ -294,10 +294,7 @@ class FlexLayoutExplorerWidgetState extends LayoutExplorerWidgetState<
               padding: const EdgeInsets.all(4.0),
               child: Text(
                 'Total Flex Factor: ${propertiesLocal.totalFlex.toInt()}',
-                textScaler: const TextScaler.linear(largeTextScaleFactor),
-                style: theme.boldTextStyle.copyWith(
-                  color: emphasizedTextColor,
-                ),
+                style: theme.regularTextStyleWithColor(emphasizedTextColor),
                 overflow: TextOverflow.ellipsis,
               ),
             ),
@@ -333,7 +330,6 @@ class FlexLayoutExplorerWidgetState extends LayoutExplorerWidgetState<
                       propertiesLocal.verticalDirectionDescription,
                       overflow: TextOverflow.ellipsis,
                       textAlign: TextAlign.center,
-                      textScaler: const TextScaler.linear(largeTextScaleFactor),
                       style: theme.regularTextStyleWithColor(
                         verticalTextColor(colorScheme),
                       ),
@@ -368,7 +364,6 @@ class FlexLayoutExplorerWidgetState extends LayoutExplorerWidgetState<
                     propertiesLocal.horizontalDirectionDescription,
                     overflow: TextOverflow.ellipsis,
                     textAlign: TextAlign.center,
-                    textScaler: const TextScaler.linear(largeTextScaleFactor),
                     style: theme.regularTextStyleWithColor(
                       horizontalTextColor(colorScheme),
                     ),

diff --git a/packages/devtools_app/lib/src/screens/inspector/layout_explorer/ui/theme.dart b/packages/devtools_app/lib/src/screens/inspector/layout_explorer/ui/theme.dart
@@ -38,7 +38,6 @@ double get heightOnlyIndicatorSize => scaleByFontFactor(72.0);
 double get minWidthToDisplayWidthInsideArrow => scaleByFontFactor(200.0);
 double get minHeightToDisplayHeightInsideArrow => scaleByFontFactor(200.0);
 
-const largeTextScaleFactor = 1.2;
 const smallTextScaleFactor = 0.8;
 
 /// Height for limiting asset image (selected one in the drop down).

diff --git a/packages/devtools_app/lib/src/screens/network/network_request_inspector.dart b/packages/devtools_app/lib/src/screens/network/network_request_inspector.dart
@@ -22,19 +22,6 @@ class NetworkRequestInspector extends StatelessWidget {
   static const _responseTabTitle = 'Response';
   static const _cookiesTabTitle = 'Cookies';
 
-  // TODO(kenz): remove these keys and use a text finder to lookup widgets in test.
-
-  @visibleForTesting
-  static const overviewTabKey = Key(_overviewTabTitle);
-  @visibleForTesting
-  static const headersTabKey = Key(_headersTabTitle);
-  @visibleForTesting
-  static const responseTabKey = Key(_responseTabTitle);
-  @visibleForTesting
-  static const cookiesTabKey = Key(_cookiesTabTitle);
-  @visibleForTesting
-  static const noRequestSelectedKey = Key('No Request Selected');
-
   final NetworkController controller;
 
   DevToolsTab _buildTab({required String tabName, Widget? trailing}) {
@@ -55,8 +42,7 @@ class NetworkRequestInspector extends StatelessWidget {
               ? Center(
                   child: Text(
                     'No request selected',
-                    key: NetworkRequestInspector.noRequestSelectedKey,
-                    style: Theme.of(context).textTheme.titleLarge,
+                    style: Theme.of(context).regularTextStyle,
                   ),
                 )
               : AnalyticsTabbedView(
@@ -119,9 +105,7 @@ class NetworkRequestInspector extends StatelessWidget {
             ),
           if (data.hasCookies)
             (
-              tab: _buildTab(
-                tabName: NetworkRequestInspector._cookiesTabTitle,
-              ),
+              tab: _buildTab(tabName: NetworkRequestInspector._cookiesTabTitle),
               tabView: HttpRequestCookiesView(data),
             ),
         ],

diff --git a/packages/devtools_app/lib/src/screens/profiler/profiler_status.dart b/packages/devtools_app/lib/src/screens/profiler/profiler_status.dart
@@ -5,6 +5,7 @@
 import 'package:devtools_app_shared/ui.dart';
 import 'package:flutter/material.dart';
 
+import '../../shared/common_widgets.dart';
 import 'cpu_profiler_controller.dart';
 
 class CpuProfilerDisabled extends StatelessWidget {
@@ -14,19 +15,22 @@ class CpuProfilerDisabled extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return Center(
-      child: Column(
-        mainAxisAlignment: MainAxisAlignment.center,
-        children: <Widget>[
-          const Text('CPU profiler is disabled.'),
-          Padding(
-            padding: const EdgeInsets.all(8.0),
-            child: ElevatedButton(
-              onPressed: controller.enableCpuProfiler,
-              child: const Text('Enable profiler'),
+    return DefaultTextStyle(
+      style: Theme.of(context).regularTextStyle,
+      child: Center(
+        child: Column(
+          mainAxisAlignment: MainAxisAlignment.center,
+          children: <Widget>[
+            const Text('CPU profiler is disabled.'),
+            Padding(
+              padding: const EdgeInsets.all(8.0),
+              child: ElevatedButton(
+                onPressed: controller.enableCpuProfiler,
+                child: const Text('Enable profiler'),
+              ),
             ),
-          ),
-        ],
+          ],
+        ),
       ),
     );
   }
@@ -37,21 +41,24 @@ class EmptyAppStartUpProfile extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return const Center(
-      child: Column(
-        mainAxisAlignment: MainAxisAlignment.center,
-        children: [
-          Text(
-            'There are no app start up samples available.',
-            textAlign: TextAlign.center,
-          ),
-          SizedBox(height: denseSpacing),
-          Text(
-            'To avoid this, try to open the DevTools CPU profiler '
-            'sooner after starting your app.',
-            textAlign: TextAlign.center,
-          ),
-        ],
+    return DefaultTextStyle(
+      style: Theme.of(context).regularTextStyle,
+      child: const Center(
+        child: Column(
+          mainAxisAlignment: MainAxisAlignment.center,
+          children: [
+            Text(
+              'There are no app start up samples available.',
+              textAlign: TextAlign.center,
+            ),
+            SizedBox(height: denseSpacing),
+            Text(
+              'To avoid this, try to open the DevTools CPU profiler '
+              'sooner after starting your app.',
+              textAlign: TextAlign.center,
+            ),
+          ],
+        ),
       ),
     );
   }
@@ -62,9 +69,7 @@ class EmptyProfileView extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return const Center(
-      child: Text('No CPU samples recorded.'),
-    );
+    return const CenteredMessage('No CPU samples recorded.');
   }
 }
 
@@ -73,28 +78,30 @@ class ProfileRecordingInstructions extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    const stopRow = Row(
-      mainAxisAlignment: MainAxisAlignment.center,
-      children: [
-        Text('Click the stop button '),
-        Icon(Icons.stop),
-        Text(' to end the recording.'),
-      ],
-    );
-    return const Center(
-      child: Column(
-        mainAxisAlignment: MainAxisAlignment.center,
-        children: [
-          Row(
-            mainAxisAlignment: MainAxisAlignment.center,
-            children: [
-              Text('Click the record button '),
-              Icon(Icons.fiber_manual_record),
-              Text(' to start recording CPU samples.'),
-            ],
-          ),
-          stopRow,
-        ],
+    return DefaultTextStyle(
+      style: Theme.of(context).regularTextStyle,
+      child: Center(
+        child: Column(
+          mainAxisAlignment: MainAxisAlignment.center,
+          children: [
+            Row(
+              mainAxisAlignment: MainAxisAlignment.center,
+              children: [
+                const Text('Click the record button '),
+                Icon(Icons.fiber_manual_record, size: defaultIconSize),
+                const Text(' to start recording CPU samples.'),
+              ],
+            ),
+            Row(
+              mainAxisAlignment: MainAxisAlignment.center,
+              children: [
+                const Text('Click the stop button '),
+                Icon(Icons.stop, size: defaultIconSize),
+                const Text(' to end the recording.'),
+              ],
+            ),
+          ],
+        ),
       ),
     );
   }

diff --git a/packages/devtools_app/lib/src/shared/preferences.dart b/packages/devtools_app/lib/src/shared/preferences.dart
@@ -32,8 +32,6 @@ class PreferencesController extends DisposableController
       ValueNotifier<bool>(Logger.root.level == verboseLoggingLevel);
   static const _verboseLoggingStorageId = 'verboseLogging';
 
-  final denseModeEnabled = ValueNotifier<bool>(false);
-
   InspectorPreferencesController get inspector => _inspector;
   final _inspector = InspectorPreferencesController();
 
@@ -63,12 +61,6 @@ class PreferencesController extends DisposableController
       storage.setValue('ui.vmDeveloperMode', '${vmDeveloperModeEnabled.value}');
     });
 
-    value = await storage.getValue('ui.denseMode');
-    toggleDenseMode(value == 'true');
-    addAutoDisposeListener(denseModeEnabled, () {
-      storage.setValue('ui.denseMode', '${denseModeEnabled.value}');
-    });
-
     await _initVerboseLogging();
 
     await inspector.init();
@@ -128,13 +120,6 @@ class PreferencesController extends DisposableController
       verboseLoggingEnabled.value = enableVerboseLogging;
     }
   }
-
-  /// Change the value for the dense mode setting.
-  void toggleDenseMode(bool? enableDenseMode) {
-    if (enableDenseMode != null) {
-      denseModeEnabled.value = enableDenseMode;
-    }
-  }
 }
 
 class InspectorPreferencesController extends DisposableController

diff --git a/packages/devtools_app/lib/src/shared/ui/tab.dart b/packages/devtools_app/lib/src/shared/ui/tab.dart
@@ -61,6 +61,14 @@ class DevToolsTab extends Tab {
   final String gaId;
 
   final Widget? trailing;
+
+  @override
+  Widget build(BuildContext context) {
+    return DefaultTextStyle(
+      style: Theme.of(context).textTheme.titleSmall!,
+      child: super.build(context),
+    );
+  }
 }
 
 /// A combined [TabBar] and [TabBarView] implementation that tracks tab changes

diff --git a/packages/devtools_app/lib/src/shared/utils.dart b/packages/devtools_app/lib/src/shared/utils.dart
@@ -33,11 +33,6 @@ void debugLogger(String message) {
   );
 }
 
-// TODO(kenz): remove concept of dense mode. Use dense values by default.
-bool isDense() {
-  return preferences.denseModeEnabled.value || isEmbedded();
-}
-
 bool isEmbedded() => ideTheme.embed;
 
 extension VmExtension on VM {

diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
@@ -12,10 +12,11 @@ To learn more about DevTools, check out the
 
 * Improved overall usability by making the DevTools UI more dense. This
 significantly improves the user experience when using DevTools embedded in
-an IDE. (#7030)[https://github.com/flutter/devtools/pull/7030]
+an IDE. - [#7030](https://github.com/flutter/devtools/pull/7030)
+* Removed the "Dense mode" setting. - [#7086](https://github.com/flutter/devtools/pull/7086)
 * Added support for filtering with regular expressions in the Logging, Network, and CPU profiler
-pages - (#7027)[https://github.com/flutter/devtools/pull/7027]
-* Add a DevTools server interaction for getting the DTD uri. - (#7054)[https://github.com/flutter/devtools/pull/7054]
+pages - [#7027](https://github.com/flutter/devtools/pull/7027)
+* Add a DevTools server interaction for getting the DTD uri. - [#7054](https://github.com/flutter/devtools/pull/7054)
 
 ## Inspector updates
 

diff --git a/packages/devtools_app/test/debugger/debugger_screen_paused_test.dart b/packages/devtools_app/test/debugger/debugger_screen_paused_test.dart
@@ -20,7 +20,7 @@ import '../test_infra/utils/debugger_utils.dart';
 import '../test_infra/utils/test_utils.dart';
 
 void main() {
-  const windowSize = Size(2500.0, 1500.0);
+  const windowSize = Size(2500.0, 1200.0);
 
   late FakeServiceConnectionManager fakeServiceConnection;
   late MockScriptManager scriptManager;

diff --git a/packages/devtools_app/test/inspector/inspector_integration_test.dart b/packages/devtools_app/test/inspector/inspector_integration_test.dart
@@ -41,8 +41,6 @@ void main() {
 
   setUp(() async {
     await env.setupEnvironment();
-    await storage.setValue('ui.denseMode', 'true');
-    preferences.toggleDenseMode(true);
   });
 
   tearDownAll(() async {