Skip to content

Conversation

@justinmc
Copy link
Contributor

ScaleUpdateDetails is supposed to be a superset of DragUpdateDetails, but it is missing the delta param. This PRs adds it.

Closes #43833

@justinmc justinmc self-assigned this Jun 21, 2021
@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Jun 21, 2021
@google-cla google-cla bot added the cla: yes label Jun 21, 2021
@justinmc justinmc changed the title (WIP) Add delta param to scaleupdate so it matches dragupdate Add delta param to scaleupdate so it matches dragupdate Jun 22, 2021
@justinmc justinmc marked this pull request as ready for review June 22, 2021 22:48
@justinmc justinmc requested a review from LongCatIsLooong June 22, 2021 22:48
@justinmc
Copy link
Contributor Author

CC @marcglasberg as this was an old request of yours.

@Piinks Piinks added a: quality A truly polished experience f: gestures flutter/packages/flutter/gestures repository. labels Jun 23, 2021
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

Flutter_LGTM

/// receiver since the previous update.
///
/// Defaults to zero if not specified in the constructor.
final Offset delta;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should this be added to toString?

Copy link
Contributor

Choose a reason for hiding this comment

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

What does "pointer" refer to in the doc? Shouldn't a scale gesture have 2 pointers? From the implementation, it should be the position change of the focal point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this, I should have waited for your review. You already saw my new PR but for the record it's at #85718.

localFocalPoint: PointerEvent.transformPosition(_lastTransform, _currentFocalPoint),
rotation: _computeRotationFactor(),
pointerCount: _pointerQueue.length,
delta: _currentFocalPoint - _initialFocalPoint,
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Jul 1, 2021

Choose a reason for hiding this comment

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

The doc says delta will be in the coordinate space of the event receiver but here it looks like these 2 are pre-transformed (global) coordinates (because there's a localFocalPoint that has a different value)?

@marcglasberg
Copy link

@justinmc Thanks for pinging me. I appreciate it.

CC @marcglasberg as this was an old request of yours.

@justinmc justinmc deleted the scale-delta branch July 1, 2021 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: quality A truly polished experience f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GestureDetector: ScaleUpdateDetails should also contain a delta since scale is a superset of pan.

5 participants