fix #2431 Progessbar Correct state transition logic #2441
Conversation
|
@PratyushSingh07 Can you review this..? |
|
Please resolve the conflicts. |
ghost
closed this
|
converting this PR to draft . You can make it ready for review when you solve the error |
ghost
marked this pull request as ready for review
|
@PratyushSingh07 Review this.... |
| _recentTransactionUiState.value = | ||
| RecentTransactionUiState.LoadMoreRecentTransactions(it.pageItems) | ||
| } else if (it.pageItems.isNotEmpty()) { | ||
| try { |
There was a problem hiding this comment.
why are you using try block ? catch api doesn't need to work with try
There was a problem hiding this comment.
Its giving me error to use finally without try
There was a problem hiding this comment.
there has to be a better alternative
There was a problem hiding this comment.
Or else i will just remove Try and finally set the state at the end to initial (thats works same as finally) what's Your view...?
@PratyushSingh07
| } | ||
| } finally { | ||
| _recentTransactionUiState.value = RecentTransactionUiState.Initial |
There was a problem hiding this comment.
any reason to use finally here ? This would mean that you would always want to execute this statement
There was a problem hiding this comment.
@PratyushSingh07 Yes...The issue Progress bar is persistent because the state remians in Loading...So we would set the state to intial state again
There was a problem hiding this comment.
are you certain that the error arises from the viewmodel and not from the fragment ?
There was a problem hiding this comment.
Yes! here are the log statements before finally..!
There was a problem hiding this comment.
And after finally @PratyushSingh07
| } | ||
|
|
There was a problem hiding this comment.
remove random spaces here as well as on line 60
| @@ -28,24 +28,34 @@ class RecentTransactionViewModel @Inject constructor(private val recentTransacti | |||
| loadRecentTransactions(offset, limit) | |||
| } | |||
|
|
|||
| private fun loadRecentTransactions(offset: Int, limit: Int) { | |||
| private fun loadRecentTransactions(offset: Int, limit: Int, refresh: Boolean = false) { | |||
There was a problem hiding this comment.
where are you using refresh variable ?
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
why do you have spaces here ?
Fixes #2431
Please Add Screenshots If there are any UI changes.
Screen_recording_20231105_094122.webm
Please make sure these boxes are checked before submitting your pull request - thanks!
Apply the
AndroidStyle.xmlstyle template to your code in Android Studio.Run the unit tests with
./gradlew checkto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.