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

Macgills/2.5 downloader #1170

Merged
merged 20 commits into from
May 23, 2019
Merged

Macgills/2.5 downloader #1170

merged 20 commits into from
May 23, 2019

Conversation

macgills
Copy link
Contributor

@macgills macgills commented May 9, 2019

Fixes #263 #597 #343 #340 #658 #668 #712 #703 #826 #830 #883 #889 #933 #907 #988 #975 #987 #990 #1007 #860
Changes: A new Downloader utilising Android's built-in DownloadManager was created, to facilitate this a total rewrite of ZimManageActivity, ZimFileSelectFragment, DownloadFragment and LibraryFragment was necessary. The new construction of these screens is that they all observe LiveData objects in the ZimManageViewModel which is a shared object that survives through configuration changes.
Internal subscriptions of the ViewModel keep the client data fresh and ready for binding, some updates can be triggered by pushing onto one of the ViewModel's inputs, named here as request* and implemented as reactive Processors.

@kelson42 kelson42 requested review from mhutti1, brijeshshah13 and rashiq and removed request for mhutti1 and brijeshshah13 May 10, 2019 08:03
Copy link
Contributor

@mhutti1 mhutti1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few issues:
The library top bar is too high on my s8 so it almost clips the top
The download only over wifi option doesn't seem to do anything. It doesn't work anymore.
You can't pause downloads anymore. (What if a user is downloading a very large file)
Downloads don't seem to be in parallel and you can't see total progress (I guess this is fine)
Am I right in concluding that you now can't download large files onto fat32 formatted systems. If so then how are we communicating this?

app/build.gradle Show resolved Hide resolved
@macgills macgills marked this pull request as ready for review May 20, 2019 14:12
Copy link
Contributor

@mhutti1 mhutti1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, but I haven't had a huge amount of time to debug on a device and check for all the edge cases.

@macgills macgills merged commit fcac33c into macgills/2.5-kotlin May 23, 2019
@macgills macgills deleted the macgills/2.5-downloader branch May 23, 2019 12:04
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 this pull request may close these issues.

4 participants