[macOS] Add platform_channel sample/test #123141

cbracken · 2023-03-21T18:26:56Z

Adds a macOS implementation of the platform_channel example, demonstrating method channels and event channels with a battery power plugin.

Adds platform_channel_sample_test_macos macOS host test to verify the sample works.

Issue: #79204

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

examples/platform_channel/macos/Runner/BatteryLevelPlugin.swift

examples/platform_channel/macos/Runner/MainFlutterWindow.swift

loic-sharma

LGTM but consider getting approval from someone with more Swift expertise.

loic-sharma · 2023-03-22T21:34:06Z

Do you know why the platform_channel_swift example was split from platform_channel? Should we merge them? They seem to have similar battery logic: https://github.com/flutter/flutter/blob/master/examples/platform_channel_swift/ios/Runner/AppDelegate.swift

cbracken · 2023-03-22T22:52:09Z

This code serves two primary purposes:

Demonstrate how to use platform channels
Test that they work

To the first point, having a separate Swift example made sense when the iOS default was Obj-C but we were migrating towards a Swift-based future. Now that the default for iOS plugins is Swift, it makes sense to simply replace the Obj-C example with the Swift code and delete the extra Swift example.

To the second point, the goal is an integration test verifying method and event channels; the language is irrelevant, so Swift-only should be sufficient.

/cc @jmagman for thoughts on that end. If you think it makes sense to keep an Obj-C version around, I'd suggest the Swift implementation be moved to the main platform_channel example and a separate iOS-only Obj-C example created, but to be honest I don't think it adds much value; most Obj-C developers will be able to translate the example themselves.

yaakovschectman

LGTM but I am not very familiar with Swift or MacOS

stuartmorgan-g · 2023-03-23T13:43:05Z

If you think it makes sense to keep an Obj-C version around, I'd suggest the Swift implementation be moved to the main platform_channel example and a separate iOS-only Obj-C example created

+1 to doing this if we keep both. (I should probably do the same with Android, moving Java out and making the primary example Kotlin.)

but to be honest I don't think it adds much value; most Obj-C developers will be able to translate the example themselves.

We also link to this as the runnable version of https://docs.flutter.dev/development/platform-integration/platform-channels#example (updating that page was the initial motivation for me wanting the macOS version of this code). If we remove the Obj-C example here we'll need to at least update the text in that info box; we could also re-evaluate whether we still also want the Obj-C example on that page.

examples/platform_channel/macos/Runner/BatteryLevelPlugin.swift

examples/platform_channel/macos/Runner/MainFlutterWindow.swift

examples/platform_channel/macos/Runner/PowerSource.swift

Adds a macOS implementation of the platform_channel example, demonstrating method channels and event channels with a battery power plugin. Adds platform_channel_sample_test_macos macOS host test to verify the sample works. Issue: flutter#79204

For consistency with rest of examples.

examples/platform_channel/macos/Runner/Configs/AppInfo.xcconfig

examples/platform_channel/macos/RunnerTests/RunnerTests.swift

)

jmagman · 2023-03-23T21:03:36Z

@hellohuanlin would you mind reviewing this post-merge? @cbracken can follow up with changes you suggest.

* cbdee52 Roll Flutter Engine from 59acb5362098 to 12c822327825 (3 revisions) (flutter/flutter#123330) * 897e3db Inject the gstatic CanvasKit CDN URL by default in `flutter build web` (flutter/flutter#122772) * 5a36bdd Stop serving Observatory by default (flutter/flutter#122419) * b212e7b implement Iterator and Comparable instead of extending them (flutter/flutter#123282) * af6029c c0fbe5a53 Roll Dart SDK from 9256fffbd5af to e8e045620234 (1 revision) (flutter/engine#40561) (flutter/flutter#123336) * 9dec4fb FIX: NavigationDrawer hover/focus/pressed does not use indicatorShape (flutter/flutter#123325) * 4907464 20ab040cd Roll Skia from ce5ff5cc03ce to c42320d53714 (2 revisions) (flutter/engine#40565) (flutter/flutter#123340) * 28d40a4 Roll Packages from 75491e9 to 0826798 (5 revisions) (flutter/flutter#123342) * 674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141) * 7b7af9f roll packages (flutter/flutter#123339) * 3179875 replace some ._() constructors with class modifiers (flutter/flutter#122765) * 7f41ab2 Fix (insert|move|remove)RenderObjectChild methods in base class (flutter/flutter#123276) * fccca49 Refactor buildOverscrollIndicator (flutter/flutter#123246) * 11bbce1 6b0933e74 [web] Add `dart:js_interop` to `_embedder.yaml`. (flutter/engine#40545) (flutter/flutter#123347) * 716d252 Remove prefer_const_constructors ignores (flutter/flutter#123284) * 100cf21 Prefer enum over class. (flutter/flutter#123312) * 5ef9b84 Expose toggle to textfield's opacity animation. (flutter/flutter#122474) * d79f3aa Roll Flutter Engine from 6b0933e74965 to bdce896fb64f (2 revisions) (flutter/flutter#123351)

hellohuanlin

Just a few comments. Let me know if any questions and I'm more than happy to assist (anything Swift related is my favorite!)

Some comments might be outdated since you may have fixed it in separate PRs

examples/platform_channel/macos/Runner/MainFlutterWindow.swift

examples/platform_channel/macos/Runner/PowerSource.swift

This adds a macOS section to the example implementation instructions, following the same structure as the existing examples, and using the code from flutter/flutter#123141 This is the macOS equivalent of #7328 and #8127 Fixes flutter/flutter#79204

This adds a macOS section to the example implementation instructions, following the same structure as the existing examples, and using the code from flutter/flutter#123141 This is the macOS equivalent of #7328 and #8127 Fixes flutter/flutter#79204 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/master/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.

)

) * cbdee52 Roll Flutter Engine from 59acb5362098 to 12c822327825 (3 revisions) (flutter/flutter#123330) * 897e3db Inject the gstatic CanvasKit CDN URL by default in `flutter build web` (flutter/flutter#122772) * 5a36bdd Stop serving Observatory by default (flutter/flutter#122419) * b212e7b implement Iterator and Comparable instead of extending them (flutter/flutter#123282) * af6029c c0fbe5a53 Roll Dart SDK from 9256fffbd5af to e8e045620234 (1 revision) (flutter/engine#40561) (flutter/flutter#123336) * 9dec4fb FIX: NavigationDrawer hover/focus/pressed does not use indicatorShape (flutter/flutter#123325) * 4907464 20ab040cd Roll Skia from ce5ff5cc03ce to c42320d53714 (2 revisions) (flutter/engine#40565) (flutter/flutter#123340) * 28d40a4 Roll Packages from 75491e9 to 0826798 (5 revisions) (flutter/flutter#123342) * 674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141) * 7b7af9f roll packages (flutter/flutter#123339) * 3179875 replace some ._() constructors with class modifiers (flutter/flutter#122765) * 7f41ab2 Fix (insert|move|remove)RenderObjectChild methods in base class (flutter/flutter#123276) * fccca49 Refactor buildOverscrollIndicator (flutter/flutter#123246) * 11bbce1 6b0933e74 [web] Add `dart:js_interop` to `_embedder.yaml`. (flutter/engine#40545) (flutter/flutter#123347) * 716d252 Remove prefer_const_constructors ignores (flutter/flutter#123284) * 100cf21 Prefer enum over class. (flutter/flutter#123312) * 5ef9b84 Expose toggle to textfield's opacity animation. (flutter/flutter#122474) * d79f3aa Roll Flutter Engine from 6b0933e74965 to bdce896fb64f (2 revisions) (flutter/flutter#123351)

cbracken requested a review from stuartmorgan-g March 21, 2023 18:26

flutter-dashboard bot added d: examples Sample code and demos c: contributor-productivity Team-specific productivity, code health, technical debt. labels Mar 21, 2023

cbracken commented Mar 21, 2023

View reviewed changes

examples/platform_channel/macos/Runner/BatteryLevelPlugin.swift Outdated Show resolved Hide resolved

cbracken force-pushed the macos-platform-channel branch 2 times, most recently from 9db4ce0 to f0c4e9a Compare March 21, 2023 18:46

cbracken changed the title ~~[macOS] Add macOS platform_channel test~~ [macOS] Add platform_channel sample/test Mar 21, 2023

cbracken requested review from loic-sharma and yaakovschectman March 22, 2023 16:44

loic-sharma reviewed Mar 22, 2023

View reviewed changes

examples/platform_channel/macos/Runner/MainFlutterWindow.swift Outdated Show resolved Hide resolved

loic-sharma approved these changes Mar 22, 2023

View reviewed changes

cbracken force-pushed the macos-platform-channel branch from 01b28ae to a8669a6 Compare March 22, 2023 23:03

yaakovschectman approved these changes Mar 23, 2023

View reviewed changes

stuartmorgan-g reviewed Mar 23, 2023

View reviewed changes

examples/platform_channel/macos/Runner/BatteryLevelPlugin.swift Outdated Show resolved Hide resolved

examples/platform_channel/macos/Runner/MainFlutterWindow.swift Outdated Show resolved Hide resolved

examples/platform_channel/macos/Runner/PowerSource.swift Outdated Show resolved Hide resolved

cbracken added 4 commits March 23, 2023 09:55

[macOS] Add macOS platform_channel test

c6fc68e

Adds a macOS implementation of the platform_channel example, demonstrating method channels and event channels with a battery power plugin. Adds platform_channel_sample_test_macos macOS host test to verify the sample works. Issue: flutter#79204

Format

cbde303

Extract handler class

f8a3c55

Add comment on manual registration

860daad

cbracken force-pushed the macos-platform-channel branch from a8669a6 to 1ed8098 Compare March 23, 2023 16:56

Merge BatteryPlugin into MainFlutterWindow

fd0591b

For consistency with rest of examples.

cbracken force-pushed the macos-platform-channel branch from 1ed8098 to fd0591b Compare March 23, 2023 16:59

cbracken merged commit 674ff15 into flutter:master Mar 23, 2023

cbracken deleted the macos-platform-channel branch March 23, 2023 18:56

cbracken mentioned this pull request Mar 23, 2023

[macOS] Battery check in platform_channel sample #123354

Merged

8 tasks

jmagman reviewed Mar 23, 2023

View reviewed changes

examples/platform_channel/macos/Runner/Configs/AppInfo.xcconfig Show resolved Hide resolved

examples/platform_channel/macos/RunnerTests/RunnerTests.swift Show resolved Hide resolved

engine-flutter-autoroll mentioned this pull request Mar 23, 2023

Manual roll Flutter from 250a6c294264 to d79f3aab0fa8 (18 revisions) flutter/packages#3535

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 23, 2023

674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141

1d27967

)

HansMuller mentioned this pull request Mar 24, 2023

Flutter Framework Team: Notable Commits #121415

Closed

hellohuanlin reviewed Mar 25, 2023

View reviewed changes

This was referenced Mar 28, 2023

i123643 print java version gradle #123644

Merged

Add Java-Gradle-AGP validation to flutter analyze #123916

Merged

stuartmorgan-g mentioned this pull request Apr 5, 2023

Add macOS to platform channel example flutter/website#8489

Merged

3 tasks

reidbaker mentioned this pull request Apr 5, 2023

Update getGradleVersion to ignore commented out lines #124260

Merged

8 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141

5af80b1

)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

674ff15 [macOS] Add platform_channel sample/test (flutter/flutter#123141

e6d9814

)

[macOS] Add platform_channel sample/test #123141

[macOS] Add platform_channel sample/test #123141

Uh oh!

Conversation

cbracken commented Mar 21, 2023

Pre-launch Checklist

Uh oh!

Uh oh!

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

loic-sharma commented Mar 22, 2023

Uh oh!

cbracken commented Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yaakovschectman left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmagman commented Mar 23, 2023

Uh oh!

hellohuanlin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cbracken commented Mar 22, 2023 •

edited

Loading

stuartmorgan-g commented Mar 23, 2023 •

edited

Loading