-
Notifications
You must be signed in to change notification settings - Fork 58
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
Fix position offset do not work when movingOnWindowChange is false #100
Fix position offset do not work when movingOnWindowChange is false #100
Conversation
Hi @AlexV525 Could you help me to review this PR ? Thanks! |
lib/src/widget/container.dart
Outdated
if (movingOnWindowChange != true) { | ||
return Padding( | ||
padding: offsetPadding ?? EdgeInsets.zero, | ||
child: w, | ||
); | ||
} |
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.
Can we continue to use the AnimatedPadding
below instead of returning a new Padding
widget?
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.
@AlexV525 Sorry for being late. It's reusing AnimatedPadding
now.
Thanks for your suggestion!
lib/src/widget/container.dart
Outdated
final EdgeInsets windowInsets; | ||
if (movingOnWindowChange) { | ||
windowInsets = EdgeInsets.only( | ||
bottom: MediaQueryData.fromWindow(ui.window).viewInsets.bottom, |
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.
Sorry for being late again (the notification was flushed in my queue). Can we get rid of .fromWindow? It's been deprecated IIRC.
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.
Hi @AlexV525, sorry for the late. Replace the deprecated method fromWindow
to fromView
. Could you help me review it again ? Thanks!
The PR looks good overall. Could you add some tests? |
Hi @AlexV525 Test added. Pls help review. Thanks. |
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.
Thanks for your contribution!
Hi @AlexV525 , I'm pushed a new change which fix the error in Github action. |
The
position
parameter is not working when setmovingOnWindowChange = false
.Sometimes I want it to be a bit higher than the bottom center. And after I set the offset value just find it not work.
Resolve #63