-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
fixes cupertino page transition leak #147133
Merged
polina-c
merged 3 commits into
flutter:master
from
Dimilkalathiya:fix_cupertino_page_transition_leak
Apr 23, 2024
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -378,56 +378,105 @@ class CupertinoPage<T> extends Page<T> { | |
/// | ||
/// The page slides in from the right and exits in reverse. It also shifts to the left in | ||
/// a parallax motion when another page enters to cover it. | ||
class CupertinoPageTransition extends StatelessWidget { | ||
class CupertinoPageTransition extends StatefulWidget { | ||
/// Creates an iOS-style page transition. | ||
/// | ||
const CupertinoPageTransition({ | ||
super.key, | ||
required this.primaryRouteAnimation, | ||
required this.secondaryRouteAnimation, | ||
required this.child, | ||
required this.linearTransition, | ||
}); | ||
|
||
/// The widget below this widget in the tree. | ||
final Widget child; | ||
|
||
/// * `primaryRouteAnimation` is a linear route animation from 0.0 to 1.0 | ||
/// when this screen is being pushed. | ||
final Animation<double> primaryRouteAnimation; | ||
|
||
/// * `secondaryRouteAnimation` is a linear route animation from 0.0 to 1.0 | ||
/// when another screen is being pushed on top of this one. | ||
final Animation<double> secondaryRouteAnimation; | ||
|
||
/// * `linearTransition` is whether to perform the transitions linearly. | ||
/// Used to precisely track back gesture drags. | ||
CupertinoPageTransition({ | ||
super.key, | ||
required Animation<double> primaryRouteAnimation, | ||
required Animation<double> secondaryRouteAnimation, | ||
required this.child, | ||
required bool linearTransition, | ||
}) : _primaryPositionAnimation = | ||
(linearTransition | ||
? primaryRouteAnimation | ||
: CurvedAnimation( | ||
parent: primaryRouteAnimation, | ||
final bool linearTransition; | ||
|
||
@override | ||
State<CupertinoPageTransition> createState() => _CupertinoPageTransitionState(); | ||
} | ||
|
||
class _CupertinoPageTransitionState extends State<CupertinoPageTransition> { | ||
|
||
|
||
// When this page is coming in to cover another page. | ||
late Animation<Offset> _primaryPositionAnimation; | ||
// When this page is becoming covered by another page. | ||
late Animation<Offset> _secondaryPositionAnimation; | ||
// Shadow of page which is coming in to cover another page. | ||
late Animation<Decoration> _primaryShadowAnimation; | ||
// Curve of primary page which is coming in to cover another page. | ||
CurvedAnimation? _primaryPositionCurve; | ||
// Curve of secondary page which is becoming covered by another page. | ||
CurvedAnimation? _secondaryPositionCurve; | ||
// Curve of primary page's shadow. | ||
CurvedAnimation? _primaryShadowCurve; | ||
|
||
@override | ||
void initState() { | ||
super.initState(); | ||
_init(); | ||
} | ||
|
||
@override | ||
void didUpdateWidget(covariant CupertinoPageTransition oldWidget) { | ||
super.didUpdateWidget(oldWidget); | ||
_disposeCurve(); | ||
polina-c marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_init(); | ||
} | ||
|
||
@override | ||
void dispose() { | ||
super.dispose(); | ||
_disposeCurve(); | ||
} | ||
|
||
void _disposeCurve() { | ||
_primaryPositionCurve?.dispose(); | ||
_secondaryPositionCurve?.dispose(); | ||
_primaryShadowCurve?.dispose(); | ||
} | ||
|
||
void _init() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we invoke this method not only in the beginning, but also when widget is updated, meay be better name will be something like |
||
_primaryPositionAnimation = | ||
(widget.linearTransition | ||
? widget.primaryRouteAnimation | ||
: _primaryPositionCurve = CurvedAnimation( | ||
parent: widget.primaryRouteAnimation, | ||
curve: Curves.fastEaseInToSlowEaseOut, | ||
reverseCurve: Curves.fastEaseInToSlowEaseOut.flipped, | ||
) | ||
).drive(_kRightMiddleTween), | ||
).drive(_kRightMiddleTween); | ||
_secondaryPositionAnimation = | ||
(linearTransition | ||
? secondaryRouteAnimation | ||
: CurvedAnimation( | ||
parent: secondaryRouteAnimation, | ||
(widget.linearTransition | ||
? widget.secondaryRouteAnimation | ||
: _secondaryPositionCurve = CurvedAnimation( | ||
parent: widget.secondaryRouteAnimation, | ||
curve: Curves.linearToEaseOut, | ||
reverseCurve: Curves.easeInToLinear, | ||
) | ||
).drive(_kMiddleLeftTween), | ||
).drive(_kMiddleLeftTween); | ||
_primaryShadowAnimation = | ||
(linearTransition | ||
? primaryRouteAnimation | ||
: CurvedAnimation( | ||
parent: primaryRouteAnimation, | ||
(widget.linearTransition | ||
? widget.primaryRouteAnimation | ||
: _secondaryPositionCurve = CurvedAnimation( | ||
parent: widget.primaryRouteAnimation, | ||
curve: Curves.linearToEaseOut, | ||
) | ||
).drive(_CupertinoEdgeShadowDecoration.kTween); | ||
|
||
// When this page is coming in to cover another page. | ||
final Animation<Offset> _primaryPositionAnimation; | ||
// When this page is becoming covered by another page. | ||
final Animation<Offset> _secondaryPositionAnimation; | ||
final Animation<Decoration> _primaryShadowAnimation; | ||
|
||
/// The widget below this widget in the tree. | ||
final Widget child; | ||
} | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
|
@@ -442,7 +491,7 @@ class CupertinoPageTransition extends StatelessWidget { | |
textDirection: textDirection, | ||
child: DecoratedBoxTransition( | ||
decoration: _primaryShadowAnimation, | ||
child: child, | ||
child: widget.child, | ||
), | ||
), | ||
); | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should linearTransition be here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually
primaryRouteAnimation
will also change whenlinearTransition
gets switched so i thought not make things more complicated, but it seems adding it would be good fail-safe, so added in last commit :)