Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

Fixes #103566

Due to changes in #97971

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels May 12, 2022
@jonahwilliams jonahwilliams marked this pull request as ready for review May 12, 2022 03:52
@jonahwilliams jonahwilliams requested a review from xu-baolin May 12, 2022 03:54
@jonahwilliams
Copy link
Contributor Author

@xu-baolin I'll trade you 😄

required TextDirection textDirection,
required bool hasFocus,
required bool hovering,
required DeviceGestureSettings gestureSettings,
Copy link
Member

Choose a reason for hiding this comment

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

Should not this.gestureSettings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the only thing I do with gestureSettings is pass it to the gesture detectors below, we don't necessarily need a field to store the settings on this render object. The gesture detectors themselves are late and so aren't re-created outside of the constructor

Copy link
Member

Choose a reason for hiding this comment

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

oh, you are right, : )

Copy link
Member

@xu-baolin xu-baolin left a comment

Choose a reason for hiding this comment

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

LGTM

Further, we may need to troubleshoot all related gestures and add gesture settings for them, what do you think?
image

@jonahwilliams
Copy link
Contributor Author

yes, an audit is in order. We might be able to speed things up by adding an assert if a gesture is received and gesture settings hasn't been set. I could also look into a breaking change process to make it required to prevent this from happening again.

@fluttergithubbot fluttergithubbot merged commit 4960492 into flutter:master May 12, 2022
@jonahwilliams jonahwilliams deleted the foobar branch May 12, 2022 06:14
@xu-baolin
Copy link
Member

👍 Glad to review!

@hicnar

This comment was marked as spam.

@hicnar
Copy link

hicnar commented Jun 1, 2022

@jonahwilliams When will that fix make its way to a stable release?

jonahwilliams pushed a commit to jonahwilliams/flutter that referenced this pull request Jun 1, 2022
itsjustkevin pushed a commit that referenced this pull request Jun 1, 2022
…egressions (#105141)

* [framework] fix slider regression due to touch slop changes (#103569)

* fix `SliverReorderableList` not work on Android platform bug (#103406)

Co-authored-by: xubaolin <[email protected]>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slider in Drawer Doesn't Slide

4 participants