Skip to content

[framework] fix slider regression due to touch slop changes - #103569

Merged
fluttergithubbot merged 1 commit into
flutter:masterfrom
jonahwilliams:foobar
May 12, 2022
Merged

fluttergithubbot merged 1 commit into
flutter:masterfrom
jonahwilliams:foobar

Conversation

@jonahwilliams

Copy link
Copy Markdown
Contributor

Fixes #103566

Due to changes in #97971

@flutter-dashboard flutter-dashboard Bot added p: material_ui material_ui package in flutter/packages 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
Copy Markdown
Contributor Author

@xu-baolin I'll trade you 😄

required TextDirection textDirection,
required bool hasFocus,
required bool hovering,
required DeviceGestureSettings gestureSettings,

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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, : )

@xu-baolin xu-baolin left a comment •

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
Member

👍 Glad to review!

@hicnar

This comment was marked as spam.

@hicnar

hicnar commented Jun 1, 2022

Copy link
Copy Markdown

@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

framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slider in Drawer Doesn't Slide

4 participants