Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Mac trackpad gestures #31593

Merged
dkwingsmt merged 5 commits into from
Apr 29, 2022
Merged

Mac trackpad gestures #31593

dkwingsmt merged 5 commits into from
Apr 29, 2022

Conversation

@moffatman
Copy link
Contributor

@moffatman moffatman commented Feb 21, 2022 •
edited
Loading

Start sending PointerPanZoom events from macOS when trackpad is used

Fixes flutter/flutter#23604

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] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

@flutter-dashboard
Copy link

flutter-dashboard bot commented Feb 21, 2022

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@cbracken cbracken self-requested a review March 4, 2022 22:20
@chinmaygarde
Copy link
Member

chinmaygarde commented Mar 17, 2022

@cbracken Ping?

justinmc
justinmc reviewed Mar 30, 2022
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

I tried this on my Macbook along with the framework PR (flutter/flutter#89944) using my demo app and it worked very well 👍 .

Would it be possible to write a unit test for this in FlutterViewControllerTest.mm?

@zanderso
Copy link
Member

zanderso commented Mar 31, 2022

From PR review triage: It looks like the next step on this one is for @moffatman to respond to the feedback.

@moffatman moffatman force-pushed the trackpad_gestures_mac branch 2 times, most recently from e75450c to 6ab5db4 Compare April 2, 2022 18:20
@moffatman moffatman requested a review from justinmc April 5, 2022 23:07
@chinmaygarde
Copy link
Member

chinmaygarde commented Apr 14, 2022

Ping @justinmc.

dkwingsmt
dkwingsmt suggested changes Apr 14, 2022
View reviewed changes
@moffatman moffatman force-pushed the trackpad_gestures_mac branch from 6ab5db4 to 4ba1e68 Compare April 18, 2022 06:51
@chinmaygarde
Copy link
Member

chinmaygarde commented Apr 21, 2022

@dkwingsmt I believe this is good for another review.

@justinmc justinmc mentioned this pull request Apr 27, 2022
Closed
justinmc
justinmc approved these changes Apr 27, 2022
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍 But @dkwingsmt should verify that the tests look ok.

Mostly just a bunch of nitpick comments on missing periods from me.

dkwingsmt
dkwingsmt approved these changes Apr 29, 2022
View reviewed changes
Copy link
Contributor

@dkwingsmt dkwingsmt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM except for the periods.

@moffatman moffatman force-pushed the trackpad_gestures_mac branch from 327ba84 to 98765b2 Compare April 29, 2022 04:22
cbracken
cbracken approved these changes Apr 29, 2022
View reviewed changes
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.

This looks great with one tiny nit.

@dkwingsmt dkwingsmt merged commit e470a96 into flutter:main Apr 29, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 29, 2022
@engine-flutter-autoroll
e470a96 Mac trackpad gestures (flutter/engine#31593)
bcc36e9
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 29, 2022
@engine-flutter-autoroll
e470a96 Mac trackpad gestures (flutter/engine#31593)
8c950ca
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 29, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 29, 2022
@engine-flutter-autoroll
e470a96 Mac trackpad gestures (flutter/engine#31593)
977777e
@Piinks Piinks mentioned this pull request Jun 13, 2022
Closed
@likang likang mentioned this pull request Aug 14, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@cbracken cbracken cbracken approved these changes

@justinmc justinmc justinmc approved these changes

@dkwingsmt dkwingsmt dkwingsmt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs tests platform-macos

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Should support trackpad gestures from host

6 participants

@moffatman @chinmaygarde @zanderso @cbracken @justinmc @dkwingsmt