search

amugofjava / anytime_podcast_player

Simple, easy to use Podcast player app written in Flutter and Dart.
BSD 3-Clause "New" or "Revised" License
377 stars 99 forks source link

Refresh the podcast on Podcast Details page creation #51

Closed ademar111190 closed 2 years ago

ademar111190 commented 3 years ago

Add a minor convenience to the user, whenever the user opens the podcast detail it silently updates the list of episodes.

Based on this suggestion: https://github.com/breez/breezmobile/issues/456

How it looks like

https://user-images.githubusercontent.com/1225438/123450993-ab049a80-d5b3-11eb-8930-d92b04815276.mp4

amugofjava commented 3 years ago

Hi @ademar111190,

Thanks for the pull request. Background/auto refresh is something I am looking into. There are a couple of issues with the podcast load routine which has stopped me calling _handleRefresh() during page load.

  1. If you try to perform a load when you are offline or have no connection an error message is displayed. Calling refresh on page load when offline causes the error message to be displayed at the start of the episode list which looks a little odd (see screenshot). When performing a silent load it also needs to silently fail.
  2. The load routine cannot be cancelled once started. This means that if you go into one podcast and the load starts, then you go to another podcast before the first has finished loading the first set of episodes returned will be displayed in the second podcast until the second load has completed (see video).

Both issues shouldn't be too hard to resolve, but as it stands I feel calling refresh on load may give a poorer user experience than having to refresh manually in the short term.

https://user-images.githubusercontent.com/5526902/123471563-e328cf00-d5ed-11eb-8918-caef2b7e7887.mp4

ademar111190 commented 3 years ago

Thank you for the feedback @amugofjava

I'll think in something that updates without those undesirables side effects.

kingonly commented 3 years ago

@amugofjava this is something Breez users keep straggling with. I get feedback on this issue on a daily basis. Both issues you've raised are valid, but if we could fix #2 and keep the manual refresh in can of no connectivity, I think it's (by far) a better UX than what we have today.

roeierez commented 3 years ago

@ademar111190 @amugofjava AFAICT the podcast_bloc is responsible for loading the podcast and updating the UI via the streams here: https://github.com/amugofjava/anytime_podcast_player/blob/master/lib/bloc/podcast/podcast_bloc.dart#L103 By the time the podcastService.loadPodcast is finished the new episodes are saved safely in the database without affective the UI. The following lines are responsible for updating the streams with the podcast and new episodes so in order to prevent wrong UI update we can check if the current podcast (in the _podcastStream) is the one that we are refreshing, if not just skip updating the streams. will that solve the seconds issue?

For the first issue we can pass a flag (background: true) to the Feed entity when calling handleRefresh. The podcast_bloc will fail silently in case of errors (network error, etc...). I think it will solve the first issue.

LMK your thoughts.

amugofjava commented 3 years ago

Hi @roeierez,

Yes, that is exactly my thinking for both points. This seems to be the simplest solution and shouldn't be difficult to implement. In addition, I am also thinking of adding settings to set the background refresh to be Off, On (always), On (Wifi-Only). I will also add a cache time - i.e. don't bother refreshing again if you did so only 5 minutes ago; podcasts won't necessarily update that frequently. If I make all these options configurable settings then you can use or ignore them how to feel best suits Breez.

ademar111190 commented 3 years ago

Hi, thanks for the feedback and the insights.

I think I did solve both issues with my last changes. May you guys can check it one more time?

I was thinking about some settings too, it doesn't seem to be difficult, but I don't have much clue about how to design it properly and keep a good experience for the user.

roeierez commented 3 years ago

I think I did solve both issues with my last changes. May you guys can check it one more time?

Look good to me.

amugofjava commented 3 years ago

Thanks @ademar111190, this looks good to me too. I will add the settings around your changes and push the update to master. Thanks for your PR.

amugofjava commented 3 years ago

I have found one small issue @ademar111190 in that because the load process is now being called twice in the initState, if you load a podcast by URL (i.e. not one already saved in the database) it results in the RSS feed being downloaded and processed twice. I'll work on resolving that.

amugofjava commented 3 years ago

This is the current state of the auto-refresh. It needs finalising and thorough testing, but I think this solution will work. The stuttery-ness of the video is not a reflection on Anytime, just the poor recording ability of the emulator running a debug build.

https://user-images.githubusercontent.com/5526902/125112132-ab865080-e0de-11eb-8e02-c47596889891.mp4

kingonly commented 3 years ago

@amugofjava great UI. One suggestion: add a spinner on top of the podcast image while new episodes are loading so people understand something is going on.