Skip to content

[material_ui] Handle unknown swipe edges in predictive back transitions - #12502

Open
muhammadkamel wants to merge 1 commit into
flutter:mainfrom
muhammadkamel:fix/material-ui-predictive-back-swipe-edge
Open

[material_ui] Handle unknown swipe edges in predictive back transitions#12502
muhammadkamel wants to merge 1 commit into
flutter:mainfrom
muhammadkamel:fix/material-ui-predictive-back-swipe-edge

Conversation

@muhammadkamel

Copy link
Copy Markdown
Contributor

Ports the Material side of flutter/flutter#185140 after the Material/Cupertino decoupling.

PredictiveBackPageTransitionsBuilder now treats unknown or button-triggered swipe edges as a no-direction fallback, matching Android BackEvent.EDGE_NONE. _ is used instead of SwipeEdge.none because that enum value is not in the Flutter SDK this package currently compiles against; it also keeps the switch exhaustive when the SDK gains new SwipeEdge values.

Adds coverage for right-edge swipes and for button-triggered back events (Offset.zero + progress: 0.0), which handleStartBackGesture already filters via isButtonEvent.

The RangeError in PredictiveBackEvent.fromMap for swipeEdge: 2 still lives in flutter/services and is fixed in flutter/flutter#191306. That crash cannot be fixed in this package.

Fixes flutter/flutter#186168

Pre-Review Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under.
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under.
  • All existing and new tests are passing.

Treat unknown or button-triggered swipe edges as a no-direction fallback
so the switch stays exhaustive when the Flutter SDK adds SwipeEdge values.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates PredictiveBackPageTransitionsBuilder to treat unknown or button-triggered swipe edges as a no-direction fallback, and adds tests for right-edge swipes and button-triggered back events. The review feedback suggests simplifying the switch expression by removing the redundant SwipeEdge.left case, and using a more precise assertion in the tests to verify that the page shifts in the correct direction.

Comment on lines 460 to +467
SwipeEdge.left => Offset(xShift, _getYShiftPosition(screenSize.height)),
SwipeEdge.right => Offset(-xShift, _getYShiftPosition(screenSize.height)),
null => Offset(xShift, _getYShiftPosition(screenSize.height)),
// Button-triggered back (Android BackEvent.EDGE_NONE) has no swipe
// direction. Button events are filtered out in handleStartBackGesture,
// so this is a safe fallback matching the null (no event) case.
// `_` keeps the switch exhaustive if additional SwipeEdge values
// are added in the Flutter SDK.
_ => Offset(xShift, _getYShiftPosition(screenSize.height)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since SwipeEdge.left and the wildcard _ fallback both return the exact same offset Offset(xShift, _getYShiftPosition(screenSize.height)), we can simplify the switch expression by removing the redundant SwipeEdge.left case and letting the wildcard pattern handle it.

          SwipeEdge.right => Offset(-xShift, _getYShiftPosition(screenSize.height)),
          // Button-triggered back (Android BackEvent.EDGE_NONE) has no swipe
          // direction. Button events are filtered out in handleStartBackGesture,
          // so this is a safe fallback matching the null (no event) case.
          // The wildcard pattern keeps the switch exhaustive if additional SwipeEdge
          // values are added in the Flutter SDK, and also handles SwipeEdge.left.
          _ => Offset(xShift, _getYShiftPosition(screenSize.height)),

// is driven by the transition animation and will be non-zero (the exact
// direction depends on which layer the tween is applied to).
final Offset pageBOffset = tester.getTopLeft(find.text('page b'));
expect(pageBOffset.dx, isNot(0.0));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For a right-edge swipe, the page is expected to shift to the left (negative dx). Using lessThan(0.0) instead of isNot(0.0) provides a more precise and robust assertion, ensuring the page shifts in the correct direction.

Suggested change
expect(pageBOffset.dx, isNot(0.0));
expect(pageBOffset.dx, lessThan(0.0));

@github-actions github-actions Bot added triage-framework Should be looked at in framework triage p: material_ui labels Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: material_ui triage-framework Should be looked at in framework triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Android] PredictiveBackEvent.fromMap does not guard against invalid swipeEdge value

1 participant