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

Cupertino route/transition should block touch events #54090

Open
machinescream opened this issue Apr 6, 2020 · 12 comments · Fixed by #95757
Open

Cupertino route/transition should block touch events #54090

machinescream opened this issue Apr 6, 2020 · 12 comments · Fixed by #95757
Labels
a: fidelity Matching the OEM platforms better c: proposal A detailed proposal for a change to Flutter f: cupertino flutter/packages/flutter/cupertino repository f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. platform-ios iOS applications specifically team-design Owned by Design Languages team triaged-design Triaged by Design Languages team

Comments

@machinescream
Copy link

I think block any touch events during Cupertino transition is reasonable.

@VladyslavBondarenko
Copy link

Hi @Renesanse
could you describe and provide a minimal complete code sample to the cases when it is reasonable?

@VladyslavBondarenko VladyslavBondarenko added in triage Presently being triaged by the triage team waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds labels Apr 6, 2020
@machinescream
Copy link
Author

machinescream commented Apr 6, 2020

Hi @Renesanse
could you describe and provide a minimal complete code sample to the cases when it is reasonable?

Navigator.of(context).push(CupertinoPageRoute(builder: (_) => Container()));

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 6, 2020
@VladyslavBondarenko
Copy link

@Renesanse
it doesn't answer the question. Why do you think it is reasonable?

@VladyslavBondarenko VladyslavBondarenko added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 6, 2020
@machinescream
Copy link
Author

machinescream commented Apr 6, 2020

Because handling touch events during transitions and animations is bad practice. Users will be confused about actions in the app cause miss clicking happens during screen transitions. I am very surprised that you are don't know it and I need to explain to you why this is necessary.
In iOS touch events is prevented by default when screen transition in progress.

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 6, 2020
@VladyslavBondarenko VladyslavBondarenko added a: fidelity Matching the OEM platforms better f: cupertino flutter/packages/flutter/cupertino repository f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. c: proposal A detailed proposal for a change to Flutter and removed in triage Presently being triaged by the triage team labels Apr 7, 2020
@xster xster moved this from Awaiting triage to Engineer reviewed in iOS Platform - Cupertino widget review Apr 29, 2020
@xster xster changed the title Cupertino route/transition block touch events Cupertino route/transition should block touch events Apr 29, 2020
@xster
Copy link
Member

xster commented Apr 29, 2020

The premise is reasonable. We theoretically already do

. This might be a bug.

@chunhtai
Copy link
Contributor

chunhtai commented May 6, 2020

@xster This is actually not blocking anything, it only blocks when the route is offstage or in the middle of back swipe

@willlockwood
Copy link
Contributor

Just letting folks know that I'm putting up a change for this soon 👍🏻

@chunhtai
Copy link
Contributor

#104520
pr is reverted

@chunhtai
Copy link
Contributor

@willlockwood
Copy link
Contributor

Huh, @chunhtai what was the issue here? Anything I can help with?

@chunhtai
Copy link
Contributor

chunhtai commented May 25, 2022

I have not investigated what is wrong, but here is a more detailed stack trace

══╡ EXCEPTION CAUGHT BY FOUNDATION LIBRARY ╞════════════════════════════════════════════════════════
The following assertion was thrown while dispatching notifications for ValueNotifier<bool>:
setState() or markNeedsBuild() called during build.
This AnimatedBuilder widget cannot be marked as needing to build because the framework is already in
the process of building widgets. A widget can be marked as needing to be built during the build
phase only if one of its ancestors is currently building. This exception is allowed because the
framework builds parent widgets before children, which means a dirty descendant will always be
built. Otherwise, the framework might not visit this widget during this build phase.
The widget on which setState() or markNeedsBuild() was called was:
  AnimatedBuilder
The widget which was currently being built when the offending call was made was:
  Builder

When the exception was thrown, this was the stack:
#0      Element.markNeedsBuild.<anonymous closure> (package:flutter/src/widgets/framework.dart:4477)
#1      Element.markNeedsBuild (package:flutter/src/widgets/framework.dart:4492)
#2      State.setState (package:flutter/src/widgets/framework.dart:1129)
#3      _AnimatedState._handleChange (package:flutter/src/widgets/transitions.dart:127)
#4      ChangeNotifier.notifyListeners (package:flutter/src/foundation/change_notifier.dart:341)
#5      ValueNotifier.value= (package:flutter/src/foundation/change_notifier.dart:445)
#6      ModalRoute._maybeUpdateIgnorePointer (package:flutter/src/widgets/routes.dart:1429)
#7      ModalRoute._handleAnimationStatusChanged (package:flutter/src/widgets/routes.dart:1186)
#8      AnimationLocalStatusListenersMixin.notifyStatusListeners (package:flutter/src/animation/listener_helpers.dart:233)
#9      AnimationLocalStatusListenersMixin.notifyStatusListeners (package:flutter/src/animation/listener_helpers.dart:233)
#10     AnimationLocalStatusListenersMixin.notifyStatusListeners (package:flutter/src/animation/listener_helpers.dart:233)
#11     CompoundAnimation._maybeNotifyStatusListeners (package:flutter/src/animation/animations.dart:677)
#12     AnimationLocalStatusListenersMixin.notifyStatusListeners (package:flutter/src/animation/listener_helpers.dart:233)
#13     AnimationController._checkStatusChanged (package:flutter/src/animation/animation_controller.dart:815)
#14     AnimationController._animateToInternal (package:flutter/src/animation/animation_controller.dart:607)
#15     AnimationController.reverse (package:flutter/src/animation/animation_controller.dart:494)
#16     TransitionRoute.didPop (package:flutter/src/widgets/routes.dart:262)
#17     LocalHistoryRoute.didPop (package:flutter/src/widgets/routes.dart:682)
#18     _RouteEntry.handlePop (package:flutter/src/widgets/navigator.dart:2888)
#19     NavigatorState._flushHistoryUpdates (package:flutter/src/widgets/navigator.dart:3860)
#20     NavigatorState.pop (package:flutter/src/widgets/navigator.dart:4916)

@machinescream
Copy link
Author

I have implemented couple of months ago this route, you can take a look at this and try to replicate its behaviour. https://github.com/machinescream/right/blob/main/lib/src/r_route.dart

@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team labels Jul 8, 2023
@stuartmorgan stuartmorgan added team-design Owned by Design Languages team triaged-design Triaged by Design Languages team and removed team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team labels Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: fidelity Matching the OEM platforms better c: proposal A detailed proposal for a change to Flutter f: cupertino flutter/packages/flutter/cupertino repository f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. platform-ios iOS applications specifically team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
8 participants