Skip to content

Conversation

@yaakovschectman
Copy link
Contributor

Add derived classes from the Darwin platform view base classes for MacOS. Functionality is largely the same as the UiKitView, but the two are decoupled and and can further diverge in the future as needed. Some unit tests remain skipped for now as the gesture recognizers for MacOS are not yet implemented.

#128519

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.

@yaakovschectman yaakovschectman added platform-mac Building on or for macOS specifically a: platform-views Embedding Android/iOS views in Flutter apps a: desktop Running on desktop labels Aug 15, 2023
@yaakovschectman yaakovschectman self-assigned this Aug 15, 2023
@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 15, 2023
@yaakovschectman yaakovschectman marked this pull request as ready for review August 15, 2023 21:46
@wanjm

This comment was marked as off-topic.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good - just a few nits here and there!

}) async {
assert(creationParams == null || creationParamsCodec != null);

// TODO(amirh): pass layoutDirection once the system channel supports it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be tied to a GitHub issue instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TODO was already present in the codebase, it has now just been copied into this new location where it is presumably also applicable. Should we create a new issue for this assuming one does not already exist?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep - assuming no such issue exists we should create one. We used to add them as they're written here, but Amir hasn't worked on Flutter in years and we've updated the style guide to require a TODO in the meantime. Probably worth replacing the iOS equivalent comment too while you're in there.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm modulo the TODO nit.

@cbracken cbracken requested a review from cyanglaz August 31, 2023 21:40
@cbracken
Copy link
Member

/cc @cyanglaz who's our iOS counterpart for platformviews.

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yaakovschectman yaakovschectman merged commit e6e44de into flutter:master Aug 31, 2023
@yaakovschectman yaakovschectman deleted the appkitview branch August 31, 2023 21:48
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop a: platform-views Embedding Android/iOS views in Flutter apps framework flutter/packages/flutter repository. See also f: labels. platform-mac Building on or for macOS specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants