Initial Commit for PlaformChannel Sample App #453

AyushBherwani1998 · 2020-06-01T12:26:35Z

Description

Adds a MethodChannel Demo with tests.

GIF

@RedBrogdon @domesticmouse I have few doubts regarding the current commit. I'll state them below

Currently, for testing the functionality of the counter, I need to write an integration test. Should I write an integration test or just unit test which Mocks the increment and decrement methods using setMockMethodCallHandler? I can also use the provider package since my logic is already separated out and in the test, I can create a Mock.
Currently, I have separated the logic for the MethodChannel and method calls in the class named Counter. Is that how we are looking to implement the demos? If yes, then is there any project structure I should follow?

RedBrogdon

@AyushBherwani1998, this is a great start. I've added a lot of comments (and, I should warn you, there's probably a few more to come 😄 ), but that's mostly just because this is the first PR and an especially big one. Once you get going on this app, we'll know each other better and the process will speed up.

Let me know what you think and when you're ready for another look!

RedBrogdon · 2020-06-02T00:41:26Z

platform_channels/README.md

@@ -0,0 +1,3 @@
+# Platform Channel Samples


This file needs to get bigger and more detailed. Take a look at the README in jsonexample for a model:

https://github.com/flutter/samples/blob/master/jsonexample/README.md

I was thinking to add a detailed README after the project. As of now, I have improved it taking the base from other samples in the repo.

The new stuff looks great. The only additional suggestion I have is to add a line just before "Goals" that says "This sample is currently being built. Not all platforms and functionality are in place."

RedBrogdon · 2020-06-02T00:41:40Z

platform_channels/analysis_options.yaml

+    - unnecessary_brace_in_string_interps
+    - unnecessary_getters_setters
+    - unnecessary_new
+    - unnecessary_statements


nit: Throw a newline on this.

RedBrogdon · 2020-06-02T00:44:51Z

platform_channels/android/app/src/profile/AndroidManifest.xml

@@ -0,0 +1,7 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.example.platform_channels">


We generally use "dev.flutter" or "dev.flutter.example" as the org. You should be able to run:

flutter create --org="dev.flutter" .

in the project directory to change it.

RedBrogdon · 2020-06-02T00:51:47Z

platform_channels/lib/main.dart

+}
+
+// List of all the demos with their titles and routes.
+final demoList = [


Typically we'd make a simple data class for this rather than using maps, so you'd have List<DemoData> rather than List<Map<String, String>>.

RedBrogdon · 2020-06-02T00:52:38Z

platform_channels/lib/main.dart

+      appBar: AppBar(
+        title: Text('Platform Channel Sample'),
+      ),
+      body: SingleChildScrollView(


Did you consider using a ListView here? While it's unlikely to matter in this case, ListView is significantly more performant than a scrolling Column.

I did consider using a ListView, but I think we won't have many DemoTile's on the screen. I'm using SingleChildScrollView just to be on safer side in case if user is testing in small device.

Discussed offline, and we decided to go with a ListView.

RedBrogdon · 2020-06-02T05:34:20Z

platform_channels/lib/src/method_channel_demo.dart

+  @override
+  void initState() {
+    super.initState();
+    count = 0;


You can also just initialize count to 0 right in its declaration and get ride of this override.

RedBrogdon · 2020-06-02T05:35:14Z

platform_channels/lib/src/method_channel_demo.dart

+        title: const Text('MethodChannel Demo'),
+      ),
+      floatingActionButton: Row(
+        mainAxisAlignment: MainAxisAlignment.start,


Start is the default, so you can actually leave this out.

RedBrogdon · 2020-06-02T05:37:13Z

platform_channels/lib/src/method_channel_demo.dart

+      appBar: AppBar(
+        title: const Text('MethodChannel Demo'),
+      ),
+      floatingActionButton: Row(


I believe that if one is going to have two FABs, they should be a in a Column. At least that's the example they show on the Material site:

https://material.io/components/buttons-floating-action-button

platform_channels/lib/src/method_channel_demo.dart

AyushBherwani1998 · 2020-06-02T12:28:28Z

@AyushBherwani1998, this is a great start. I've added a lot of comments (and, I should warn you, there's probably a few more to come 😄 ), but that's mostly just because this is the first PR and an especially big one. Once you get going on this app, we'll know each other better and the process will speed up.

Let me know what you think and when you're ready for another look!

I was expecting comments for the feedback and reviews to help me better understand how the samples are maintained 😄 . I'll push the changes soon.

RedBrogdon

This is really close!

RedBrogdon · 2020-06-03T04:19:23Z

platform_channels/README.md

@@ -0,0 +1,3 @@
+# Platform Channel Samples


The new stuff looks great. The only additional suggestion I have is to add a line just before "Goals" that says "This sample is currently being built. Not all platforms and functionality are in place."

RedBrogdon · 2020-06-03T04:19:58Z

platform_channels/README.md

+## Goals
+
+* Demonstrate how to use `MethodChannel` to invoke platform methods.
+* Demonstrate how to use `EventChannel` to listen continous value changes across the platforms.


nit: "to listen to continuous value changes from the platform."

RedBrogdon · 2020-06-03T04:21:19Z

platform_channels/android/app/src/main/kotlin/dev/flutter/platform_channels/MainActivity.kt

-
-                    if (call.argument<Int>("count") != null) {
-                        count = call.argument<Int>("count")!!
+                    val count: Int? = call.argument<Int>("count")


This looks great. Much tighter code.

RedBrogdon · 2020-06-03T04:21:57Z

platform_channels/lib/main.dart

+      appBar: AppBar(
+        title: Text('Platform Channel Sample'),
+      ),
+      body: SingleChildScrollView(


Discussed offline, and we decided to go with a ListView.

platform_channels/lib/src/counter_method_channel.dart

RedBrogdon · 2020-06-03T04:24:24Z

platform_channels/lib/src/method_channel_demo.dart

-/// It has two [FloatingActionButton]s and a [Text] widget to display the value of
-/// [count].
+/// It has two [FloatingActionButton]s to increment and decrement the value of
+/// [count], and a [Text] widget to display the value it.


nit: "...to display its value."

RedBrogdon · 2020-06-03T04:24:44Z

platform_channels/lib/src/method_channel_demo.dart

-          FloatingActionButton(
-            heroTag: 'addition',
-            child: Icon(Icons.add),
+      floatingActionButton: Builder(


Nice refactor. Builder has saved me a lot of keys. :)

RedBrogdon · 2020-06-03T04:25:09Z

platform_channels/lib/src/method_channel_demo.dart

-          FloatingActionButton(
-            heroTag: 'subtract',
-            child: Icon(Icons.remove),
+                // Whenever users presses the FloatingActionButton, it invokes


nit: "...users press the..."

RedBrogdon · 2020-06-03T04:26:07Z

platform_channels/test/src/method_channel_demo_test.dart

 import 'package:flutter_test/flutter_test.dart';
 import 'package:platform_channels/src/method_channel_demo.dart';

 void main() {
+  TestWidgetsFlutterBinding.ensureInitialized();


This shouldn't be necessary. Try taking it out and see if everything still works. :)

RedBrogdon · 2020-06-03T04:35:45Z

platform_channels/lib/src/method_channel_demo.dart

+                onPressed: () {
+                  Counter.increment(counterValue: count).then((value) {
+                    setState(() => count = value);
+                  }).catchError((dynamic error) {
+                    showErrorMessage(
+                      context,
+                      error.message as String,
+                    );
+                  });
+                },


I believe I had a comment in this section earlier, but just in case: typically I would expect to see an async method here. Generally we recommend using async/await over the Futures API whenever possible.

The logic can all stay the same; it's just a syntactical change:

onPressed: () async { try { final value = await Counter.increment(counterValue: count); setState(() => count = value); } catch (error) { showErrorMessage( context, error.message as String, ); } },

I missed it in the previous commit after doing the changes.

…98/samples into platform_channel

AyushBherwani1998 · 2020-06-03T09:57:48Z

The recent commit updates the UI of MethodChannel Demo since having two FloatingActionButtons is not always encouraged.

New UI

RedBrogdon

LGTM other than the one suggestion about shuffling some test code around. Let me know what you want to do there, and then we can merge this.

platform_channels/test/src/method_channel_demo_test.dart

AyushBherwani1998 added 4 commits June 1, 2020 17:06

initial commit, adds MethodChannel demo

19dbb41

add project in CI script

f3fb698

updated README

1b61dbe

improves formatting

c6354be

RedBrogdon requested changes Jun 2, 2020

View reviewed changes

AyushBherwani1998 added 5 commits June 2, 2020 18:23

improves tests, and code quality

4c6256e

adds new line in analysis_options

572e802

changes org name

f28d8e5

changes org name

40f5142

updates README

22ab4f3

AyushBherwani1998 requested a review from RedBrogdon June 2, 2020 14:16

RedBrogdon requested changes Jun 3, 2020

View reviewed changes

AyushBherwani1998 added 3 commits June 3, 2020 15:18

updates UI, and test

89d734c

Merge branch 'platform_channel' of https://github.com/AyushBherwani19…

ee98dee

…98/samples into platform_channel

updates README

4c58378

AyushBherwani1998 requested a review from RedBrogdon June 3, 2020 09:59

RedBrogdon approved these changes Jun 4, 2020

View reviewed changes

platform_channels/test/src/method_channel_demo_test.dart Show resolved Hide resolved

improves test

3b4495e

RedBrogdon approved these changes Jun 4, 2020

View reviewed changes

RedBrogdon merged commit acf700c into flutter:master Jun 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial Commit for PlaformChannel Sample App #453

Initial Commit for PlaformChannel Sample App #453

AyushBherwani1998 commented Jun 1, 2020 •

edited

Loading

RedBrogdon left a comment

RedBrogdon Jun 2, 2020

AyushBherwani1998 Jun 2, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 2, 2020

RedBrogdon Jun 2, 2020

RedBrogdon Jun 2, 2020

RedBrogdon Jun 2, 2020

AyushBherwani1998 Jun 2, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 2, 2020

RedBrogdon Jun 2, 2020

RedBrogdon Jun 2, 2020

AyushBherwani1998 commented Jun 2, 2020

RedBrogdon left a comment

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

RedBrogdon Jun 3, 2020

AyushBherwani1998 Jun 3, 2020

AyushBherwani1998 commented Jun 3, 2020

RedBrogdon left a comment

		@@ -0,0 +1,7 @@
		<manifest xmlns:android="http://schemas.android.com/apk/res/android"
		package="com.example.platform_channels">

Initial Commit for PlaformChannel Sample App #453

Initial Commit for PlaformChannel Sample App #453

Conversation

AyushBherwani1998 commented Jun 1, 2020 • edited Loading

Description

GIF

RedBrogdon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AyushBherwani1998 commented Jun 2, 2020

RedBrogdon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AyushBherwani1998 commented Jun 3, 2020

New UI

RedBrogdon left a comment

Choose a reason for hiding this comment

AyushBherwani1998 commented Jun 1, 2020 •

edited

Loading