mobile-wallet icon indicating copy to clipboard operation
mobile-wallet copied to clipboard

Swipe to refresh not working

Open devansh-299 opened this issue 4 years ago • 8 comments

Summary:

The SwipeRefreshLayout doesn't work for most of the fragments in the application. Currently we are using the same BaseFragment SwipeRefreshLayout for all the fragments and this also leads to unnecessary fetching of data from the server when the user just wants to refresh just a particular page

(For illustration - I took HistoryFragment) Steps to reproduce:

  • Turn off mobile data/Wifi
  • Go to history fragment -> Error state appears
  • Turn on mobile data/Wifi
  • Swipe to refresh -> Observe error state still appears even though transactions have been fetched

Expected behavior:

The error state should have disappeared and refreshed transaction history should have been displayed

Device and Android version: Xiaomi PocoF1 Android 9 Stock rom

Screen Recording: GIF-200305_224337

devansh-299 avatar Mar 05 '20 17:03 devansh-299

@devansh-299 @luckyman20 @naman14 This is the problem with the app in general, swipe refresh does not work anywhere in the app. Because of how the app is designed all these different fragments (which are a subclass of BaseFragment)share the same swipe layout and as view pager loads two different fragment at once the swipe refresh listener of the visible fragment gets overridden by the listener of the next fragment. I think we should should modify the issue to cover the whole app not only HistoryFragment.

dubeyaayush07 avatar Mar 06 '20 06:03 dubeyaayush07

@devansh-299 @luckyman20 @naman14 This is the problem with the app in general, swipe refresh does not work anywhere in the app. Because of how the app is designed all these different fragments (which are a subclass of BaseFragment)share the same swipe layout and as view pager loads two different fragment at once the swipe refresh listener of the visible fragment gets overridden by the listener of the next fragment. I think we should should modify the issue to cover the whole app not only HistoryFragment.

@dubeyaayush07 yeah, you are completely right. I think there's a need to use separate SwipeRefreshLayout for each fragment. What do you think?

devansh-299 avatar Mar 06 '20 06:03 devansh-299

@devansh-299 Yeah that seems like the way to go but that would entail changing the basic architecture of the app (modifying MainActivity) maybe we could find another fix like in InvoiceFragment's setupSwipeRefresh a check to see if swipeLayout is enabled or not and proceed only if it is not enabled but then we will have to find some way to disable it too when going back. We could also try using setVisibleHint function but it is deprecated in androidX also duplicating the same swipeRefreshLayout seems redundant.

dubeyaayush07 avatar Mar 06 '20 06:03 dubeyaayush07

@devansh-299 Yeah that seems like the way to go but that would entail changing the basic architecture of the app (modifying MainActivity) maybe we could find another fix like in InvoiceFragment's setupSwipeRefresh a check to see if swipeLayout is enabled or not and proceed only if it is not enabled but then we will have to find some way to disable it too when going back. We could also try using setVisibleHint function but it is deprecated in androidX also duplicating the same swipeRefreshLayout seems redundant.

I will try to come up with a solution soon as I can, I have my exam tomorrow and after that I will try to work on this and revert back. If you can find one please suggest

devansh-299 avatar Mar 06 '20 07:03 devansh-299

@devansh-299 Yeah that seems like the way to go but that would entail changing the basic architecture of the app (modifying MainActivity) maybe we could find another fix like in InvoiceFragment's setupSwipeRefresh a check to see if swipeLayout is enabled or not and proceed only if it is not enabled but then we will have to find some way to disable it too when going back. We could also try using setVisibleHint function but it is deprecated in androidX also duplicating the same swipeRefreshLayout seems redundant.

I will try to come up with a solution soon as I can, I have my exam tomorrow and after that I will try to work on this and revert back. If you can find one please suggest

@dubeyaayush07 I have made a PR addressing this issue. What are your thoughts?

devansh-299 avatar Mar 08 '20 10:03 devansh-299

@devansh-299 Really great work but have you checked by turning internet on/off? Because all these fragments still share the same swipeRefreshLayout instance (of the MainActivity) so enabling it in one place enables it in every fragment that is why I think it should not work

dubeyaayush07 avatar Mar 08 '20 12:03 dubeyaayush07

Thanks @dubeyaayush07 and refreshing after switching off data is working but this did help me finding another bug GIF-200308_190443

devansh-299 avatar Mar 08 '20 13:03 devansh-299

Can I work this issue?Is it still valid?

haran2248 avatar Jan 21 '21 19:01 haran2248