Skip to content
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

Wrong progress indicators with swipe/refresh in Bookmarks #3430

Closed
1 task done
nikclayton opened this issue Mar 10, 2023 · 5 comments · Fixed by #3474
Closed
1 task done

Wrong progress indicators with swipe/refresh in Bookmarks #3430

nikclayton opened this issue Mar 10, 2023 · 5 comments · Fixed by #3474

Comments

@nikclayton
Copy link
Contributor

Swiping to refresh in Bookmarks if they initially failed to load (e.g., poor network, or the server did not respond in time) shows both the "swipe to refresh" circular progress spinner, and the "loading content" progress spinner at the same time.

To reproduce:

  1. Open Tusky
  2. Enable airplane mode (to simulate poor network connection)
  3. Open bookmarks. Observe the sad elephant
  4. Disable airplane mode
  5. Swipe down to refresh
  6. Observe both progress spinners appear

@connyduck This is related to our discussion over in the Notifications PR. Which progress spinner should appear in this case? The swipe one, because the user has swiped, or the non-swipe one, because no content has been loaded yet?


  • Tusky Version: 21

  • I searched or browsed the repo’s other issues to ensure this is not a duplicate.

@connyduck
Copy link
Collaborator

Ideally, swipe to refresh is disabled and there is a "retry" button. Upon clicking it, full progress bar appears. Lists view seems to have it right.

This whole error/retry/refresh thing should be consolidated everywhere, right now we have different behaviors all over the place. Different elephants are shown, sometimes with, sometimes without retry button, some lists have swipe to refresh, some don't.

@Lakoja
Copy link
Collaborator

Lakoja commented Mar 21, 2023

I added a PR as a sort of discussion base.

I would say that everything (every list) should behave comparably.
(I guess from a code point that is everything that uses a MessageView.)

Meaning:

  • Everything has swipe-to-refresh
  • Everything has a retry button for failures
  • The only progress indicator is the one from the SwipeRefreshLayout

The last one probably contradicts our efforts in the thread view (two linear progress bars instead).
So at least that one is open to debate.

All the touched locations in the PR are quite different list implementations.
So from a consistent code perspective the question would be: How to abstract that?
Is there for example something like traits in Kotlin?

@connyduck
Copy link
Collaborator

Treating thread view differently is reasonable though? It can load partially and some parts may be cached, we don't have that anywhere else.

Regarding the other Activities/Fragments that show refreshable lists and errors, yes they all have very different implementations, we should tackle that eventually. But for now unifying their behavior is a nice quick win that is also noticebale for users, so we should do that.

@Lakoja
Copy link
Collaborator

Lakoja commented Mar 24, 2023

Treating thread view differently is reasonable though? It can load partially and some parts may be cached, we don't have that anywhere else.

I guess from a user's perspective they should still feel/be the same. (The user doesn't care that contents might be loaded differently here.)
Only in case of an error should there maybe be a respective hint: "This is cached, more couldn't be loaded" or similar.

@Lakoja
Copy link
Collaborator

Lakoja commented Mar 31, 2023

This issue was actually not fixed by the PR.
The discussion here is still open: Which progress bar(s) to use?
The PR only made it (more) consistent throughout the code base.

Lakoja added a commit to Lakoja/Tusky that referenced this issue Apr 24, 2023
nikclayton pushed a commit that referenced this issue Apr 30, 2023
On Account preferences > Filters

With the list hidden on an empty view the FAB is erroneouly placed on the top left of the screen.

(Introduced with #3430)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants