-
Notifications
You must be signed in to change notification settings - Fork 573
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
Smoother HomeUI Scroll and Animation #170
Smoother HomeUI Scroll and Animation #170
Conversation
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 @Vishwa-Raghavendra your PR is good! I have some minor comments but overall that's an elegant and optimal solution. I'll merge it, right away! Thanks for the contribution!
@@ -28,4 +28,7 @@ object Constants { | |||
"https://github.com/ILIYANGERMANOV/ivy-wallet#contributors-see-graph" | |||
|
|||
const val USER_INACTIVITY_TIME_LIMIT = 60 //Time in seconds | |||
|
|||
const val SWIPE_DOWN_THRESHOLD_OPEN_MORE_MENU = 200 |
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.
This constant is local for HomeTab and isn't global => it shouldn't be here.
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.
Nvm, this is okay to be a Constant as it is used in more than one place.
@@ -28,4 +28,7 @@ object Constants { | |||
"https://github.com/ILIYANGERMANOV/ivy-wallet#contributors-see-graph" | |||
|
|||
const val USER_INACTIVITY_TIME_LIMIT = 60 //Time in seconds | |||
|
|||
const val SWIPE_DOWN_THRESHOLD_OPEN_MORE_MENU = 200 | |||
const val SWIPE_HORIZONTAL_THRESHOLD = 250 |
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.
This constant is local for HomeMoreMenu and isn't global => it shouldn't be here.
@@ -170,6 +176,8 @@ private fun BoxWithConstraintsScope.UI( | |||
} | |||
var expanded by remember { mutableStateOf(false) } | |||
|
|||
val showHideBalanceRow = remember { mutableStateOf(false) } |
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.
hideBalanceRow
might be a better name
@@ -339,11 +364,47 @@ fun HomeTransactionsLazyColumn( | |||
) { | |||
val ivyContext = LocalIvyContext.current | |||
|
|||
val nestedScrollConnection = remember { |
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.
That's nice solution!
Pull Request (PR) Checklist
Please check if your pull request fulfills the following requirements:
develop
branch.bundle exec fastlane ui_tests
and all tests are passingsuccessfully.
Put an
x
in the boxes that apply.[x]
Pull Request Type
Please check the type of change your PR introduces:
Put an
x
in the boxes that apply.Does this PR closes any GitHub Issues?
Check Ivy Wallet Issues.
What's changed?
Video Preview -> https://user-images.githubusercontent.com/42895647/143041982-be14ea76-720f-45da-b75c-216762902459.mp4
This PR template style was inspired by ionic-framework.