Skip to content

Conversation

@reyhaan
Copy link

@reyhaan reyhaan commented Jul 20, 2024

No description provided.

Copy link

@nivisi nivisi left a comment

Choose a reason for hiding this comment

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

I am not sure if it's the best solution, but it looks promising.

///
/// If no curve is provided it falls back to `decelerateEasing`.
final Curve? animationCurve;
final Curve? secondAnimationCurve;
Copy link

Choose a reason for hiding this comment

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

I'd recommend naming it reverseAnimationCurve

Comment on lines 70 to +72
/// If no curve is provided it falls back to `decelerateEasing`.
final Curve? animationCurve;
final Curve? secondAnimationCurve;
Copy link

Choose a reason for hiding this comment

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

We should also provide documentation what it is used for.

}

Curve get _defaultCurve => widget.animationCurve ?? _decelerateEasing;
Curve get _defaultSecondCurve => widget.secondAnimationCurve ?? _decelerateEasing;
Copy link

Choose a reason for hiding this comment

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

Imo should return _defaultCurve.flipped as a fallback

Comment on lines +376 to 378
final animationValue = (_dismissUnderway && !isDragging ? secondAnimationCurve : animationCurve).transform(
widget.animationController.value,
);
Copy link

Choose a reason for hiding this comment

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

Suggested change
final animationValue = (_dismissUnderway && !isDragging ? secondAnimationCurve : animationCurve).transform(
widget.animationController.value,
);
final effectiveCurve = _dismissUnderway && !isDragging
? secondAnimationCurve
: animationCurve;
final animationValue = effectiveCurve.transform(
widget.animationController.value,
);

@nivisi
Copy link

nivisi commented Aug 13, 2024

Hi sir @jamesblasco mind taking a look at this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants