fossasia / open-event-organizer-android

Open Event Mobile App for Organizers and Entry Managers https://play.google.com/store/apps/details?id=com.eventyay.organizer
Apache License 2.0
1.79k stars 490 forks source link

Remove any one of the two progress bars #705

Closed mooocer closed 6 years ago

mooocer commented 6 years ago

Is it a ?

Bug Report

Actual Behaviour

When swiped down to refresh, two progress bars appear simultaneously (one from the SwipeRefreshLayout, and other from the "included" progressbar_layout.xml)

Expected Behaviour

Only one of the progress bars should be displayed as double progress bars make the UI look odd.

Steps to reproduce it

Swipe down to refresh

Screenshot of the issue

doublerefresh

Would you like to work on the issue?

Yes

iamareebjamal commented 6 years ago

The problem is that a view only has one showProgress contract and we want a pull to refresh progress indicator, but it does not look good when loading items. What do you propose

mooocer commented 6 years ago

@iamareebjamal Can we use any of these?

  1. Use setRefreshing(false) in the OnRefeshListener's onRefresh() method. This one seems to be a concise and better solution.
  2. Change the drawable via reflection (not sure about this one).
  3. We can use a 3rd party library for swipeToRefresh.
iamareebjamal commented 6 years ago

Still can't understand how any of these will solve both of these being triggered. Can you elaborate?

mooocer commented 6 years ago

Yes. SwipeRefreshLayout's setRefreshing(false) is used to tell the widget that the loading has finished i.e. to hide the ProgressBar. The SwipeRefreshLayout's progress bar will be hidden immediately after the refresh is triggered. But the other ProgressBar will keep behaving normally.

This means that as you pull down, the small ProgressBar will be visible and move to show that fragment supports SwipeToRefresh. But once you swipe down fully and activate refresh, the smaller ProgressBar will be hidden immediately, and only the bigger (better looking) ProgressBar will be visible throughout the refresh.

The gif:

refresh

The other 2 solutions will replace the default drawable provided by SwipeToRefresh layout, and we can completely disable the manually triggered ProgressBar.

iamareebjamal commented 6 years ago

OK, sounds good. Looks good. What happens on a screen where there are no items?

mooocer commented 6 years ago

I found out that in the current state of the app if we swipe down in case of no items, the spinner doesn't appear to move. This is also a UI glitch as the user then won't know that fragment supports SwipeToRefresh feature (in case of no items), unless they accidently swipe down.

Otherwise, it works same as described above in case of no items as well.

iamareebjamal commented 6 years ago

OK, please go ahead and implement this

mooocer commented 6 years ago

Thanks! I'll send a PR right away.

mooocer commented 6 years ago

@iamareebjamal Please review!

sridharjajoo commented 6 years ago

@iamareebjamal This issue is similar one but the multiple progress bar here are present in the dashboard.