-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add MacOS AppKitView class. #132583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add MacOS AppKitView class. #132583
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
cbracken
left a comment
There was a problem hiding this 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Co-authored-by: Chris Bracken <[email protected]>
Co-authored-by: Chris Bracken <[email protected]>
Co-authored-by: Chris Bracken <[email protected]>
cbracken
left a comment
There was a problem hiding this 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.
|
/cc @cyanglaz who's our iOS counterpart for platformviews. |
cyanglaz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.