Skip to content

Redo "Remove pressure customization from some pointer events" #30874

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.

Sign up for GitHub

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

Jump to bottom
Merged
dkwingsmt merged 6 commits into from
May 3, 2019
Merged

Redo "Remove pressure customization from some pointer events" #30874

dkwingsmt merged 6 commits into from
May 3, 2019

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Apr 10, 2019 •
edited
Loading

Description

This PR removes parameter pressure from the constructors of the following classes:

  • PointerAddedEvent
  • PointerRemovedEvent
  • PointerHoverEvent
  • PointerEnterEvent
  • PointerExitEvent
  • PointerUpEvent See below
  • PointerCancelEvent

It changes converter so that the aforementioned events will not be passed with datum.pressure.

It also changes PointerEnterEvent.fromMouseEvent and PointerExitEvent.fromMouseEvent into redirecting to their unnamed constructors, which drops the arguments down and pressure from the source event.

For reasoning, migration, and discussion, see #30412.

About redo

A first attempt #30414 was found to break devicelab and was reverted. Through investigation, it was found out that events with PointerChange.up can also contain non-zero pressure (see touchEvents which are real recordings).

As a fix, PointerUpEvent has kept its pressure parameter, and will receive datum.pressure when dispatched by event converter. Other changes are unchanged.

I have tested all devicelabs on an android device and it has passed all tests.

Related Issues

Tests

It's not related to any tests.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

@goderbauer goderbauer added framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Apr 10, 2019
@dkwingsmt
Copy link
Contributor Author

dkwingsmt commented Apr 10, 2019 •
edited
Loading

An event in bin/cache/pkg/goldens/dev/integration_tests/assets_for_android_views/assets/touchEvents contains PointerChange.up with non-zero pressure. This file is recorded from real events though.

[embedded_android_views_integration_test] [STDOUT] stdout: [        ] I/flutter (12419): EVENT PointerData(timeStamp: 147:24:11.703000, change: PointerChange.up, kind: PointerDeviceKind.touch, signalKind: PointerSignalKind.none, device: 1, physicalX: 375.0, physicalY: 455.0, buttons: 0, pressure: 0.1411764770746231, pressureMin: 0.0, pressureMax: 1.0, distance: 0.0, distanceMax: 0.0, size: 0.1411764770746231, radiusMajor: 36.0, radiusMinor: 36.0, radiusMin: 0.0, radiusMax: 0.0, orientation: 0.0, tilt: 0.0, platformData: 0, scrollDeltaX: 0.0, scrollDeltaY: 0.0)

@dkwingsmt dkwingsmt changed the title WIP: Redo "Remove pressure customization from some pointer events" Redo "Remove pressure customization from some pointer events" Apr 10, 2019
@dkwingsmt dkwingsmt requested a review from goderbauer April 11, 2019 19:46
@dkwingsmt dkwingsmt added the c: API break Backwards-incompatible API changes label Apr 12, 2019
Hixie
Hixie reviewed Apr 19, 2019
View reviewed changes
Hixie
Hixie reviewed Apr 19, 2019
View reviewed changes
@dkwingsmt dkwingsmt mentioned this pull request Apr 20, 2019
Open
dkwingsmt
dkwingsmt commented Apr 20, 2019
View reviewed changes
Copy link
Contributor Author

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

Fixed the 2 issues

@dkwingsmt dkwingsmt requested a review from Hixie April 20, 2019 00:50
@dnfield
Copy link
Contributor

dnfield commented Apr 24, 2019

This has some merge conflicts now that need to be updated.

@dnfield
Copy link
Contributor

dnfield commented May 1, 2019

Is this waiting for a re-review? @Hixie @goderbauer @dkwingsmt

@dkwingsmt
Copy link
Contributor Author

dkwingsmt commented May 1, 2019

@dnfield Yes. May I have a re-review? @Hixie

@Hixie
Copy link
Contributor

Hixie commented May 3, 2019

LGTM

@dkwingsmt dkwingsmt merged commit 7beb09e into master May 3, 2019
@dkwingsmt dkwingsmt deleted the revert-30873-revert-30414-remove-hover-pressure branch May 3, 2019 06:05
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@goderbauer goderbauer Awaiting requested review from goderbauer

@Hixie Hixie Awaiting requested review from Hixie

Assignees

No one assigned

Labels

c: API break Backwards-incompatible API changes framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Some event kinds shouldn't allow non-zero pressure

5 participants

@dkwingsmt @dnfield @Hixie @goderbauer @googlebot