Merge download adapters and performance improvements #1145
Conversation
This could honestly be a horrible way to impliment this (or maybe you don't want it at all) but figured I would give it a try as I often am used to you being able to on other apps so often out of habbit try like this and of course does not work. This also seems to fix an issue where downloaded icon becomes mixed with in progress and downloaded overlapping on scroll if you have inprogress downloads as well as size becoming inaccurate while scrolling (it used the wrong file's size sometimes) which seems to be fixed with `setItemViewCacheSize`. `setHasFixedSize` also improves performance by reducing the need to continue re-requesting layout when it does not need to (testing did not show any issues with it anyway, but not sure if this is the best way to do either of these things. Known issues: * Does not work for series/individual episodes * Delete icon UI needs improvements TODO (for further download improvements; maybe in follow-ups): * Show more information (IE synopsis/descriptions) for downloaded content * Further performance improvements for UI threads etc... If the entire concept of this idea is undesired I suppose we can close it and I will just do some of the other things to improve performance etc... (IE when having a moderate amount of downloads scroll is very laggy which this also seems to at least improve quite a lot.
You now have to click the delete icon. This was a UX choice so you don't inadvertently swipe and create unexpected popups
… and other cleanup We can use adapter.notifyItemRemoved and remove it from the cardList in adapater directly since we do understand the position in this case. It works better because it does not require reloading everything and causes less bugs with the view model. I am not sure if this was the proper way to do this. If not I can revert the change to this method It also removes the need for a callback in DownloadButtonSetup
Luna712
changed the title
|
I tested this now and it seems to work with pretty significant performance improvements. For one the updateList in view model was always refreshing data causing it to sometimes refresh multiple times on a single request which this also appears to correct (at least visually stops the items from refreshing unless they actually have updates to refresh) additionally the merginf of the adapters was just done as a step towards the multi download select because it allows more common code rather than a bunch of duplicate stuff to work around the difference in adapters. So it just makes future expansion a bit easiser. If you don't want it done like this I can do it another way if you have better suggestions. Overall this should be ready for review though now. |
|
Okay so further testing, this actually seems to fix a lot more problems then I even intended.
|
|
Okay my new version should be done. I decided to go further with this PR and make things even better and more organized so it's easier to work on my next project: multi-delete downloads. Making the code cleaner and easier to manage helps make that a bit easier as well... |
Fixes
Cleanup/Refactoring
Performance Profiling
Both use the same downloads:
NOTE: this could just be coincidence though