Apply first round of reviews

material-foundation · May 30, 2022 · 3980e88 · 3980e88
1 parent dbc15a3
commit 3980e88
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 1.4.0
+- Add support for Windows' accent color ([#43](https://github.com/material-foundation/material-dynamic-color-flutter/pull/43F))
+  - Rename `DynamicColorPlugin.getControlAccentColor` to `DynamicColorPlugin.getAccentColor`
+  - Rename `DynamicColorTestingUtils.setMockDynamicColors`'s argument `controlAccentColor` to `accentColor`
+
 ## 1.3.0
 
 - Add support for macOS' control accent color ([#42](https://github.com/material-foundation/material-dynamic-color-flutter/pull/42))

diff --git a/README.md b/README.md
@@ -49,6 +49,8 @@ import 'package:dynamic_color/dynamic_color.dart';
 See this [complete example] for obtaining dynamic colors, creating
 harmonized color schemes, and harmonizing custom colors.
 
+See [accent color example] for obtaining the macOS/Windows accent color.
+
 <a href="https://material-foundation.github.io/material-dynamic-color-flutter/example/build/web/">
 <img src="https://user-images.githubusercontent.com/6655696/152188934-35e58f5c-2a3c-41af-8d49-faabb1701dcc.png" width="400" /> </a>
 
@@ -74,3 +76,4 @@ flutter build web
 [examples]: https://github.com/material-foundation/material-dynamic-color-flutter/tree/main/example/lib/
 [harmonization.dart]: https://github.com/material-foundation/material-dynamic-color-flutter/blob/main/lib/src/harmonization.dart
 [hosted on github]: https://material-foundation.github.io/material-dynamic-color-flutter/example/build/web/
+[accent color example]: https://github.com/material-foundation/material-dynamic-color-flutter/blob/main/example/lib/accent_color.dart
diff --git a/example/lib/control_accent_color.dart → example/lib/accent_color.dart b/example/lib/control_accent_color.dart → example/lib/accent_color.dart
@@ -2,25 +2,25 @@ import 'package:dynamic_color/dynamic_color.dart';
 import 'package:dynamic_color_example/common.dart';
 import 'package:flutter/material.dart';
 
-class ControlAccentColorExample extends StatelessWidget {
-  const ControlAccentColorExample({Key? key}) : super(key: key);
+class AccentColorExample extends StatelessWidget {
+  const AccentColorExample({Key? key}) : super(key: key);
 
-  static const title = 'Control accent color (macOS and Windows)';
+  static const title = 'Accent color (macOS and Windows)';
 
   @override
   Widget build(BuildContext context) {
     return FutureBuilder<Color?>(
-      future: DynamicColorPlugin.getControlAccentColor(),
+      future: DynamicColorPlugin.getAccentColor(),
       builder: (context, snapshot) {
         if (snapshot.connectionState == ConnectionState.done) {
           final color = snapshot.data;
           return color == null
               ? const Text(
-                  "Control accent color isn't supported on this platform",
+                  "Accent color isn't supported on this platform",
                 )
               : Column(
                   children: [
-                    ColoredSquare(color, 'Control Accent Color'),
+                    ColoredSquare(color, 'Accent color'),
                   ],
                 );
         } else {

diff --git a/example/lib/main.dart b/example/lib/main.dart
@@ -2,7 +2,7 @@ import 'package:flutter/material.dart';
 
 import 'common.dart';
 import 'complete_example.dart';
-import 'control_accent_color.dart';
+import 'accent_color.dart';
 import 'core_palette_visualization.dart';
 import 'dynamic_color_builder_example.dart';
 import 'get_core_palette_example.dart';
@@ -45,8 +45,8 @@ class ExampleApp extends StatelessWidget {
                   widget: CorePaletteVisualization(),
                 ),
                 const _ExampleAppButton(
-                  title: ControlAccentColorExample.title,
-                  widget: ControlAccentColorExample(),
+                  title: AccentColorExample.title,
+                  widget: AccentColorExample(),
                 ),
                 const _ExampleAppButton(
                   title: HarmonizationExample.title,

diff --git a/lib/src/dynamic_color_builder.dart b/lib/src/dynamic_color_builder.dart
@@ -20,7 +20,7 @@ import 'dynamic_color_plugin.dart';
 ///    for obtaining dynamic colors and creating a harmonized color scheme
 ///  * [DynamicColorPlugin.getCorePalette] for requesting the [CorePalette]
 ///    directly, asynchronously.
-///  * [DynamicColorPlugin.getControlAccentColor] for requesting the accent [Color]
+///  * [DynamicColorPlugin.getAccentColor] for requesting the accent [Color]
 ///    [ColorScheme] directly, asynchronously.
 class DynamicColorBuilder extends StatefulWidget {
   const DynamicColorBuilder({
@@ -75,28 +75,27 @@ class DynamicColorBuilderState extends State<DynamicColorBuilder> {
     }
 
     try {
-      final Color? controlAccentColor =
-          await DynamicColorPlugin.getControlAccentColor();
+      final Color? accentColor = await DynamicColorPlugin.getAccentColor();
 
       // Likewise above.
       if (!mounted) return;
 
-      if (controlAccentColor == null) {
-        debugPrint('Got null control accent color.');
+      if (accentColor == null) {
+        debugPrint('Got null accent color.');
       } else {
         setState(() {
           _light = ColorScheme.fromSeed(
-            seedColor: controlAccentColor,
+            seedColor: accentColor,
             brightness: Brightness.light,
           );
           _dark = ColorScheme.fromSeed(
-            seedColor: controlAccentColor,
+            seedColor: accentColor,
             brightness: Brightness.dark,
           );
         });
       }
     } on PlatformException {
-      debugPrint('Failed to obtain control accent color.');
+      debugPrint('Failed to obtain accent color.');
     }
   }
 

diff --git a/lib/src/dynamic_color_plugin.dart b/lib/src/dynamic_color_plugin.dart
@@ -17,7 +17,7 @@ class DynamicColorPlugin {
   static const methodName = 'getCorePalette';
 
   /// A method name that the macOS plugin listens for.
-  static const controlAccentColorMethodName = 'getControlAccentColor';
+  static const accentColorMethodName = 'getAccentColor';
 
   /// Returns the Android OS' dynamic colors asynchronously in a [CorePalette].
   ///
@@ -33,18 +33,19 @@ class DynamicColorPlugin {
     return result == null ? null : CorePalette.fromList(result.toList());
   }
 
-  /// Returns the macOS' control accent color asynchronously as a [Color].
+  /// Returns the OS' accent color asynchronously as a [Color].
   ///
-  /// Supported since macOS 10.14 (Mojave) and Windows 10/11.
+  /// Supported on macOS starting with 10.14 (Mojave) and on Windows with
+  /// Windows 10.
   ///
   /// See also:
   ///
-  /// * [Apple's introduction to macos accent color](https://developer.apple.com/design/human-interface-guidelines/macos/overview/whats-new-in-macos/#app-accent-colors)
+  /// * [Apple's introduction to macOS accent color](https://developer.apple.com/design/human-interface-guidelines/macos/overview/whats-new-in-macos/#app-accent-colors)
   /// * [macOS's NSColor.controlAccentColor documentation](https://developer.apple.com/documentation/appkit/nscolor/3000782-controlaccentcolor)
   /// * [Windows' accent color](https://docs.microsoft.com/en-us/windows/apps/design/style/color#accent-color)
   /// * [Change colors in Windows](https://support.microsoft.com/en-us/windows/change-colors-in-windows-d26ef4d6-819a-581c-1581-493cfcc005fe)
-  static Future<Color?> getControlAccentColor() async {
-    final result = await channel.invokeMethod(controlAccentColorMethodName);
+  static Future<Color?> getAccentColor() async {
+    final result = await channel.invokeMethod(accentColorMethodName);
     return result == null ? null : Color(result);
   }
 }
diff --git a/lib/test_utils.dart b/lib/test_utils.dart
@@ -18,7 +18,7 @@ class DynamicColorTestingUtils {
   @visibleForTesting
   static void setMockDynamicColors({
     CorePalette? colorPalette,
-    Color? controlAccentColor,
+    Color? accentColor,
   }) {
     DynamicColorPlugin.channel
         .setMockMethodCallHandler((MethodCall methodCall) async {
@@ -27,8 +27,8 @@ class DynamicColorTestingUtils {
             ? Int64List.fromList(colorPalette.asList())
             : null;
       } else if (methodCall.method ==
-          DynamicColorPlugin.controlAccentColorMethodName) {
-        return controlAccentColor?.value;
+          DynamicColorPlugin.accentColorMethodName) {
+        return accentColor?.value;
       }
     });
     addTearDown(() {

diff --git a/macos/Classes/DynamicColorPlugin.swift b/macos/Classes/DynamicColorPlugin.swift
@@ -10,7 +10,7 @@ public class DynamicColorPlugin: NSObject, FlutterPlugin {
 
   public func handle(_ call: FlutterMethodCall, result: @escaping FlutterResult) {
     switch call.method {
-    case "getControlAccentColor":
+    case "getAccentColor":
       if #available(macOS 10.14, *) {
         if let color = NSColor.controlAccentColor.usingColorSpace(.sRGB) {
           var r: CGFloat = 0

diff --git a/macos/dynamic_color.podspec b/macos/dynamic_color.podspec
@@ -3,8 +3,8 @@ Pod::Spec.new do |s|
   s.authors          = 'Material Flutter team + Ebrahim Byagowi'
   s.license          = 'BSD-3-Clause'
   s.homepage         = 'https://github.com/material-foundation/material-dynamic-color-flutter'
-  s.summary          = 'Retrieves control accent color'
-  s.version          = '0.0.1'
+  s.summary          = 'Retrieves accent color'
+  s.version          = '0.0.2'
   s.source           = { :path => '.' }
   s.source_files     = 'Classes/**/*'
   s.dependency 'FlutterMacOS'

diff --git a/pubspec.yaml b/pubspec.yaml
@@ -1,6 +1,6 @@
 name: dynamic_color
 description: A Flutter package to create Material color schemes based on a platform's implementation of dynamic color.
-version: 1.3.0
+version: 1.4.0
 repository: https://github.com/material-foundation/material-dynamic-color-flutter
 
 environment:

diff --git a/test/dynamic_color_builder_test.dart b/test/dynamic_color_builder_test.dart
@@ -10,7 +10,7 @@ void main() {
   setUp(() {
     DynamicColorTestingUtils.setMockDynamicColors(
       colorPalette: SampleCorePalettes.green,
-      controlAccentColor: Colors.amber,
+      accentColor: Colors.amber,
     );
   });
 
@@ -38,10 +38,10 @@ void main() {
     expect(container.color, const Color(0xff286b2a));
   });
 
-  testWidgets('DynamicColorBuilder is correct on macOS',
+  testWidgets('DynamicColorBuilder is correct on macOS/Windows',
       (WidgetTester tester) async {
     DynamicColorTestingUtils.setMockDynamicColors(
-      controlAccentColor: Colors.amber,
+      accentColor: Colors.amber,
     );
     await tester.pumpWidget(dynamicColorBuilder());
     await tester.pumpAndSettle();

diff --git a/test/dynamic_color_plugin_test.dart b/test/dynamic_color_plugin_test.dart
@@ -25,20 +25,20 @@ void main() {
     expect(colors, equals(null));
   });
 
-  test('getControlAccentColor', () async {
+  test('getAccentColor', () async {
     const color = Color.fromARGB(12, 24, 123, 53);
 
     DynamicColorTestingUtils.setMockDynamicColors(
-      controlAccentColor: color,
+      accentColor: color,
     );
-    final result = await DynamicColorPlugin.getControlAccentColor();
+    final result = await DynamicColorPlugin.getAccentColor();
     expect(result, color);
   });
 
-  test('getControlAccentColor returns null', () async {
-    DynamicColorTestingUtils.setMockDynamicColors(controlAccentColor: null);
+  test('getAccentColor returns null', () async {
+    DynamicColorTestingUtils.setMockDynamicColors(accentColor: null);
 
-    final colors = await DynamicColorPlugin.getControlAccentColor();
+    final colors = await DynamicColorPlugin.getAccentColor();
     expect(colors, equals(null));
   });
 }
diff --git a/windows/dynamic_color_plugin.cpp b/windows/dynamic_color_plugin.cpp
@@ -40,7 +40,7 @@ DynamicColorPlugin::~DynamicColorPlugin() {}
 void DynamicColorPlugin::HandleMethodCall(
     const flutter::MethodCall<flutter::EncodableValue> &method_call,
     std::unique_ptr<flutter::MethodResult<flutter::EncodableValue>> result) {
-  if (method_call.method_name().compare("getControlAccentColor") == 0) {
+  if (!method_call.method_name().compare("getAccentColor")) {
     int64_t argbColor = 0;
     DWORD abgr = 0;
     DWORD resultSize = sizeof (abgr);