Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gesture disambiguation fix #7

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 21 additions & 50 deletions lib/src/cupertino_page_route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -628,40 +628,23 @@ class _CupertinoBackGestureDetector<T> extends StatefulWidget {
class _CupertinoBackGestureDetectorState<T> extends State<_CupertinoBackGestureDetector<T>> {
_CupertinoBackGestureController<T> _backGestureController;

HorizontalDragGestureRecognizer _recognizer;

@override
void initState() {
super.initState();
_recognizer = HorizontalDragGestureRecognizer(debugOwner: this)
..onStart = _handleDragStart
..onUpdate = _handleDragUpdate
..onEnd = _handleDragEnd
..onCancel = _handleDragCancel;
}

@override
void dispose() {
_recognizer.dispose();
super.dispose();
}

void _handleDragStart(DragStartDetails details) {
assert(mounted);
assert(_backGestureController == null);
_backGestureController = widget.onStartPopGesture();
void _handleDragStart(DragStartDetails details, double dragAreaWidth) {
if (details.localPosition.dx < dragAreaWidth && widget.enabledCallback()) {
assert(mounted);
assert(_backGestureController == null);
_backGestureController = widget.onStartPopGesture();
}
}

void _handleDragUpdate(DragUpdateDetails details) {
assert(mounted);
assert(_backGestureController != null);
_backGestureController.dragUpdate(_convertToLogical(details.primaryDelta / context.size.width));
_backGestureController?.dragUpdate(
_convertToLogical(details.primaryDelta / context.size.width));
}

void _handleDragEnd(DragEndDetails details) {
assert(mounted);
assert(_backGestureController != null);
_backGestureController.dragEnd(_convertToLogical(details.velocity.pixelsPerSecond.dx / context.size.width));
_backGestureController?.dragEnd(_convertToLogical(details.velocity.pixelsPerSecond.dx / context.size.width));
_backGestureController = null;
}

Expand All @@ -673,11 +656,6 @@ class _CupertinoBackGestureDetectorState<T> extends State<_CupertinoBackGestureD
_backGestureController = null;
}

void _handlePointerDown(PointerDownEvent event) {
if (widget.enabledCallback())
_recognizer.addPointer(event);
}

double _convertToLogical(double value) {
switch (Directionality.of(context)) {
case TextDirection.rtl:
Expand Down Expand Up @@ -706,21 +684,14 @@ class _CupertinoBackGestureDetectorState<T> extends State<_CupertinoBackGestureD
MediaQuery.of(context).padding.left :
MediaQuery.of(context).padding.right;
dragAreaWidth = max(dragAreaWidth, _backGestureWidth);
return Stack(
fit: StackFit.passthrough,
children: <Widget>[
widget.child,
PositionedDirectional(
start: 0.0,
width: dragAreaWidth,
top: 0.0,
bottom: 0.0,
child: Listener(
onPointerDown: _handlePointerDown,
behavior: HitTestBehavior.translucent,
),
),
],
return GestureDetector(
onHorizontalDragEnd: (details) => _handleDragEnd(details),
onHorizontalDragStart: (details) =>
_handleDragStart(details, dragAreaWidth),
onHorizontalDragUpdate: (details) =>
_handleDragUpdate(details),
onHorizontalDragCancel: _handleDragCancel,
child: widget.child,
);
}
}
Expand All @@ -746,7 +717,7 @@ class _CupertinoBackGestureController<T> {
@required this.controller,
}) : assert(navigator != null),
assert(controller != null) {
navigator.didStartUserGesture();
//navigator.didStartUserGesture();
Copy link
Owner

@PiN73 PiN73 Jan 25, 2021

Choose a reason for hiding this comment

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

Can you clarify which use-case do you have with keyboard which needs this fix?

Copy link
Author

@theSharpestTool theSharpestTool Jan 26, 2021

Choose a reason for hiding this comment

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

@PiN73 If there are two pages with autofocused text in each. If the user navigates back from one page to another by swiping back, the field will lose autofocus. It was critical in my project

Copy link
Owner

Choose a reason for hiding this comment

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

I think we shouldn't disable these callbacks as it can break other things. If you want to allow focusing during user gesture, I think it would be better to patch _shouldIgnoreFocusRequest in widgets/routes.dart making it always return false whatever the value of userGestureInProgress is

Copy link
Owner

Choose a reason for hiding this comment

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

As widgets/routes.dart isn't a part of this package, I'm going to keep the original behavior

Copy link
Author

@theSharpestTool theSharpestTool Jan 30, 2021

Choose a reason for hiding this comment

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

I think we shouldn't disable these callbacks as it can break other things. If you want to allow focusing during user gesture, I think it would be better to patch _shouldIgnoreFocusRequest in widgets/routes.dart making it always return false whatever the value of userGestureInProgress is

What things it can break?

}

final AnimationController controller;
Expand Down Expand Up @@ -805,12 +776,12 @@ class _CupertinoBackGestureController<T> {
// depends on userGestureInProgress.
AnimationStatusListener animationStatusCallback;
animationStatusCallback = (AnimationStatus status) {
navigator.didStopUserGesture();
//navigator.didStopUserGesture();
controller.removeStatusListener(animationStatusCallback);
};
controller.addStatusListener(animationStatusCallback);
} else {
navigator.didStopUserGesture();
//navigator.didStopUserGesture();
}
}
}
Expand Down Expand Up @@ -1149,4 +1120,4 @@ Future<T> showCupertinoDialog<T>({
useRootNavigator: useRootNavigator,
routeSettings: routeSettings,
);
}
}