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

Revert "Add new pointer signal for cancelling scroll inertia"#34535

Merged
dkwingsmt merged 1 commit intomainfrom
revert-34402-inertia
Jul 8, 2022
Merged

Revert "Add new pointer signal for cancelling scroll inertia"#34535
dkwingsmt merged 1 commit intomainfrom
revert-34402-inertia

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Jul 8, 2022 •
edited
Loading

Reverts #34402

Internal build failure: (b/238385004)


ERROR: third_party/dart/flutter/lib/src/gestures/converter.dart

55: .map((ui.PointerData datum) {                            The body might complete normally, causing 'null' to be returned, but the return
                                                             type, 'PointerEvent', is a potentially non-nullable type.
                                                             #body_might_complete_normally

66: switch (datum.signalKind ?? ui.PointerSignalKind.none) { (warning) Missing case clause for 'scrollInertiaCancel'.
                                                             #missing_enum_constant_in_switch

Error: A non-null value must be returned since the return type 'PointerEvent' doesn't allow null.
 - 'PointerEvent' is from 'package:flutter/src/gestures/events.dart' 
        .map((ui.PointerData datum) {
             ^

@dkwingsmt dkwingsmt requested a review from moffatman July 8, 2022 11:22
@flutter-dashboard flutter-dashboard bot added the embedder Related to the embedder API label Jul 8, 2022
@moffatman
Copy link
Contributor

moffatman commented Jul 8, 2022

Why would it fail, is this build just using an older framework, or it's some more strict rules?

@dkwingsmt dkwingsmt merged commit 088f925 into main Jul 8, 2022
@dkwingsmt dkwingsmt deleted the revert-34402-inertia branch July 8, 2022 11:45
@dkwingsmt
Copy link
Contributor Author

dkwingsmt commented Jul 8, 2022

@moffatman I guess Google's internal codebase uses a different (stricter) rule set. Do these errors make sense to you?

@moffatman
Copy link
Contributor

moffatman commented Jul 8, 2022

@dkwingsmt Those errors look like what you would get if you didn't have flutter/flutter#106906. I can't be sure since I can't see the code at the difference points. Maybe I should have waited longer before merging this engine change for framework to propagate?

@dkwingsmt
Copy link
Contributor Author

dkwingsmt commented Jul 8, 2022 •
edited
Loading

Just checked and confirmed that the code at that time had not applied flutter/flutter#106906. I think it's because the framework changes had not been rolled into the internal codebase but the engine roller was also trying based on the internal codebase.

Unfortunately the framework roller is experiencing difficulty and is 8 days behind. It doesn't necessarily mean flutter/flutter#106906 needs another 8 days to roll in. Can you create a reland PR? I'll keep an eye on the rolling status of flutter/flutter#106906.

This was referenced Jul 8, 2022
Closed
Merged
@moffatman
Copy link
Contributor

moffatman commented Jul 8, 2022

@dkwingsmt I made a reland PR here: #34537. Also included updates to android and web implementations which were forgotten in the original PR.

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 8, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 8, 2022
@engine-flutter-autoroll
088f925 Revert "Add new pointer signal for cancelling scroll inertia (f…
42594aa 
…lutter#34402)" (flutter/engine#34535)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@moffatman moffatman moffatman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

embedder Related to the embedder API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dkwingsmt @moffatman