-
-
Notifications
You must be signed in to change notification settings - Fork 92
Reflect offline state in dowloads #1421
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
Conversation
Update log Update log
1117ae7 to
4650356
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1421 +/- ##
=======================================
Coverage 92.59% 92.59%
=======================================
Files 18 18
Lines 878 878
=======================================
Hits 813 813
Misses 65 65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kelson42
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is a good idea to introduced this new status "offline" because it is everything but clear what is offline.
The status has to be "paused" like requested in the issue.
One question left open would be if the user needs additionally (to the "paused" status) to be informed that the underlying reason is the device been offline. I don't think this is necessary.
If you believe that, to the contrary, this is important. We could introduce in the detailed download view, we could replace the "Pause" information item with "Pause (device is offline)".
|
I think the confusion will come from the fact that the download task itself can be paused and resumed, regardless if the user is online or offline. So let's say, I want to download 2 very large files, and press download on both of them. The other confusing part is that, if I don't have internet connection, I should not have the option to "resume" a download, but that's not going to be clear either. So in a state machine description, we have: Maybe this "offline" naming is not clear enough and needs to be changed. If I understand your idea correctly, that would mean that we try to squeeze our current 3 states: into 2 (visible) states: with the *, that if I am offline I won't be able to "resume", and once I get back online it will magically change back to "resumed". So far the user had the option to "pause" and "resume", now we would magically take away this option from them (if they are offline), without a clear explanation. I think that is confusing. |
4650356 to
007b513
Compare
|
Decision (when device is offline):
|
007b513 to
82be961
Compare
| import Combine | ||
| import CoreData | ||
| import SwiftUI | ||
| import Combine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imports should be unique
| import Combine |
|
Closing in favor of: #1424 |
Fixes: #1419