[flutter_tools] add toggle `b` and service extension to change platform brightness #59571

jonahwilliams · 2020-06-16T16:21:54Z

Description

A frequent request from the last Flutter developer survey was for an easier method of testing light/dark mode changes. Currently, a user needs to manually change the theme settings or adjust phone settings to see the difference. Instead we should add a toggle from the CLI, and eventually devtools/Intellij/Vscode that allows developers to override the current setting.

Fixes #59495

Adds flutter.ext.brightnessOverride service protocol which either queries the current platform brightness, or overrides it to a new value. This accepts either Brightness.light or Brightness.dark as a value.

Adds a CLI toggle b which allows the setting to be toggled manually.

Requires an update to the MediaQuery, to conditionally use a debug override when not in release mode

jonahwilliams · 2020-06-16T16:23:52Z

packages/flutter/lib/src/widgets/media_query.dart

  /// window's metrics change. For example, see
  /// [WidgetsBindingObserver.didChangeMetrics] or [Window.onMetricsChanged].
-  MediaQueryData.fromWindow(ui.Window window)
+  MediaQueryData.fromWindow(ui.Window window, { ui.Brightness platformBrightness })


@goderbauer thoughts on this API change? It seems likely to be non-breaking. I could instead create a mocked window ... but this seemed more straightforward

Instead of adding the extra slightly awkward one-off API, could you just do the following in app.dart?

MediaQueryData data = MediaQueryData.fromWindow(WidgetsBinding.instance.window); if (!kReleaseMode) { data = data.copyWith(platformBrightness: debugBrightnessOverride); }

Ahh, I missed the existing copyWith method - that seems more straightforward

goderbauer · 2020-06-16T18:02:36Z

packages/flutter/lib/src/foundation/debug.dart

  return value.toStringAsFixed(1);
 }
+
+/// A brightness setting that overrides [ui.Brightness] when used through the [BindingBase].


How do you use it "through the [BindingBase]"?

As implemented, it appears to be a debug setting in the widget layer, determining what brightness the MediaQuery of WidgetApp inserts into the tree, no?

I guess it isn't really true - maybe "Used by MediaQuery to override the platform brightness?"

Sounds better, we should probably move it to widgets/debug.dart then since its more a debug setting of that layer?

I wasn't sure, the window is exposed through BindingBase which is where the Brightness comes from. If one was hypothetically depending only on foundation, it would not be usable if the brightness was in the widget layer

Ah, that's fair. If we add some more documentation to this property here I am OK with leaving it where it as.

goderbauer

approach seems fine.

goderbauer · 2020-06-16T18:50:55Z

packages/flutter/lib/src/foundation/debug.dart

 }
+
+/// A setting that can be used to override the platform [Brightness] exposed
+/// from [BindingBase.window]


nitty nit: missing a .

goderbauer · 2020-06-16T18:51:58Z

packages/flutter/lib/src/foundation/debug.dart

+/// A setting that can be used to override the platform [Brightness] exposed
+/// from [BindingBase.window]
+///
+/// For an example of using [debugBrightnessOverride], see


Maybe just say "See also:" since that's our style. You could incorporate this half-sentence into the explanation after the [WidgetsApp], which below.

goderbauer · 2020-06-16T18:52:38Z

packages/flutter/lib/src/foundation/binding.dart

        name: 'exit',
        callback: _exitApplication,
      );
-      registerServiceExtension(


Is this just no longer a thing - or why remove it?

its not a thing

jmagman · 2020-06-16T22:03:24Z

packages/flutter/test/foundation/service_extensions_test.dart

+    result = await binding.testExtension('brightnessOverride', <String, String>{});
+    final String brightnessValue = result['value'] as String;
+
+    expect(brightnessValue, 'Brightness.light');


Can dark be tested?

I can't actually change the value in this test or I'll trigger reassemble and break the test framework. I will update the integration test

the integration test now confirms both brightness settings and that a bogus value does not change anything

jmagman · 2020-06-16T22:05:08Z

packages/flutter_tools/lib/src/base/command_help.dart

+  CommandHelpOption _b;
+  CommandHelpOption get b => _b ??= _makeOption(
+    'b',
+    'Toggle the platform brightness setting.',


I didn't know what "brightness" meant, maybe 'Toggle the platform brightness (dark and light mode).'?

Good idea, fixed

jmagman

Impressed how quickly you did this based on survey feedback!

jmagman · 2020-06-17T01:04:17Z

packages/flutter_tools/test/integration.shard/vmservice_integration_test.dart

+        isolateId: isolate.id,
+        args: <String, String>{
+          'value': 'dark',
+        }


Can you add a comment that this is purposely bogus? It looks like a bug in the test.

jonahwilliams · 2020-06-17T01:11:22Z

Thanks! its not my first rodeo 🐮 🥍 😄

jmagman · 2020-06-17T01:13:30Z

Thanks! its not my first rodeo 🐮 🥍 😄

I said "impressed" not "surprised". 😉

goderbauer

LGTM

goderbauer · 2020-06-17T21:41:42Z

packages/flutter/lib/src/foundation/binding.dart

+            await reassembleApplication();
+          }
+          return <String, dynamic>{
+            'value': (debugBrightnessOverride ?? window.platformBrightness)


nitty nit: just leave the .toString() on this line? Looks so lonely on the next.

Gave it some friends

goderbauer · 2020-06-17T21:42:15Z

packages/flutter/lib/src/foundation/debug.dart

+///
+/// See also:
+///
+///  * [WidgetsApp], which uses the [debugBrightnessOverride] setting to construct a [MediaQueryData].


maybe add that it only uses it in debug mode.

…rm brightness (flutter#59571) A frequent request from the last Flutter developer survey was for an easier method of testing light/dark mode changes. Currently, a user needs to manually change the theme settings or adjust phone settings to see the difference. Instead we should add a toggle from the CLI, and eventually devtools/Intellij/Vscode that allows developers to override the current setting. Fixes flutter#59495 Adds flutter.ext.brightnessOverride service protocol which either queries the current platform brightness, or overrides it to a new value. This accepts either Brightness.light or Brightness.dark as a value. Adds a CLI toggle b which allows the setting to be toggled manually. Requires an update to the MediaQuery, to conditionally use a debug override when not in release mode

jonahwilliams added 2 commits June 16, 2020 09:13

[flutter_tool] support a command line toggle for platform brightness

8670fad

add fix for web

590f35d

fluttergithubbot added framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Jun 16, 2020

googlebot added the cla: yes label Jun 16, 2020

Jonah Williams added 2 commits June 16, 2020 09:22

Update binding.dart

3ee90ef

Update app.dart

3479213

jonahwilliams commented Jun 16, 2020

View reviewed changes

jonahwilliams and others added 2 commits June 16, 2020 10:50

add testing of brightness toggle

07496cf

Update vmservice.dart

5b67d93

jonahwilliams marked this pull request as ready for review June 16, 2020 17:52

jonahwilliams changed the title ~~[flutter_tools][wip] add toggle b and service protocol to change platform brightness~~ [flutter_tools] add toggle b and service extension to change platform brightness Jun 16, 2020

jonahwilliams removed the work in progress; do not review label Jun 16, 2020

goderbauer reviewed Jun 16, 2020

View reviewed changes

devoncarew mentioned this pull request Jun 16, 2020

expose a brightness / dark mode toggle option? flutter/devtools#2090

Closed

give another pass on the comment

528bc55

goderbauer reviewed Jun 16, 2020

View reviewed changes

Update debug.dart

6a6d847

jonahwilliams requested a review from jmagman June 16, 2020 19:31

fix media query test that had null values

39e3356

jmagman reviewed Jun 16, 2020

View reviewed changes

jonahwilliams added 3 commits June 16, 2020 15:48

add more deets to command help

2606ec3

confirm window is set to dark mode

a978dcb

more integration test

25e64b7

jmagman approved these changes Jun 17, 2020

View reviewed changes

Update vmservice_integration_test.dart

0048506

jonahwilliams requested a review from goderbauer June 17, 2020 01:16

goderbauer reviewed Jun 17, 2020

View reviewed changes

Jonah Williams added 2 commits June 17, 2020 14:46

give toString some friends

6999c84

Update debug.dart

e5b8623

jonahwilliams merged commit e1f4cfb into flutter:master Jun 18, 2020

jonahwilliams deleted the add_toggle_brightness branch June 18, 2020 17:32

DanTup mentioned this pull request Aug 3, 2020

Add a command to toggle brightness Dart-Code/Dart-Code#2697

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021

[flutter_tools] add toggle b and service extension to change platform brightness #59571

[flutter_tools] add toggle b and service extension to change platform brightness #59571

Uh oh!

Conversation

jonahwilliams commented Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Jun 17, 2020

Uh oh!

jmagman commented Jun 17, 2020

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[flutter_tools] add toggle `b` and service extension to change platform brightness #59571

[flutter_tools] add toggle `b` and service extension to change platform brightness #59571

jonahwilliams commented Jun 16, 2020 •

edited

Loading