search

andrasferenczi / vitrina-legacy

A companion app for Muzei that fetches images from Reddit
24 stars 9 forks source link

Keep the latest images when refreshing #5

Closed amartini closed 4 years ago

amartini commented 4 years ago

Hi, as we discussed I went ahead and made the refresh work with keeping the most recent versions by filtering, sorting by time and limiting each subreddit. Please take a look and let me know what you think.

amartini commented 4 years ago

Just realized i changed the last line and forgot to change back (return images.map { it }), sorry bout that

andrasferenczi commented 4 years ago

Thank you for the PR, but overall this significantly changes the behavior of the app.

I explained briefly in this comment how it needs to be implemented and I still don't see any other way.

One big issues with this change is that if you disable shuffle in the settings and you find the maximum number of posts from the first subreddit, you will never see other posts from subreddit.

To put it simply: refreshing twice one after another should give a different result, as you meant to see new pictures, but this change would mean you keep loading the same thing many times (as it does not keep track of history).

amartini commented 4 years ago

I see what you mean. I made this because previously you said there would be the problem that newer posts wouldn't show, but I solved this by sorting by date before limiting. For me, when I refresh, and shuffle is off, I want to see the newest posts. If there's nothing new, the list should remain the same. I think this is a better behavior than having only one image left, but I understand if this is not the intended behavior. Another option would be keeping today behavior by deleting old entries, but re-adding some of them if the result size is less than 15.

andrasferenczi commented 4 years ago

I solved this by sorting by date before limiting

I think that this should be solved at the API level. Currently the app is querying by hot and not by new. The reason being is that some subreddits can have too many low rated posts.

If there's nothing new, the list should remain the same.

Don't forget that with this pull request, if you have let's say /r/EarthPorn listed as first with a low upvote count, you will never see other posts from subreddits that come after that. 15 images can always be server from that subreddit and later coming ones will never be served.

The current (released) code works, because it keeps track of the posts served, so when you keep reloading it, it will run out of images from /r/EarthPorn eventually. Deleting the memory would break that functionality.

Another option would be keeping today behavior by deleting old entries, but re-adding some of them if the result size is less than 15.

I'll see about that, but I am afraid Muzei might download the images again, haven't checked their code.

andrasferenczi commented 4 years ago

This has been solved in #9 . I am closing this.