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

Add live activities for download progress #1058

Closed
wants to merge 1 commit into from

Conversation

plyght
Copy link

@plyght plyght commented Dec 21, 2024

Fixes #1037

Add Live Activities support for displaying download progress on the lock screen and in the Dynamic Island.

  • Import ActivityKit and create a new DownloadActivityAttributes struct in App/App_iOS.swift.
  • Update Model/DownloadService.swift to manage Live Activities during download progress changes.
  • Modify Views/BuildingBlocks/DownloadTaskCell.swift to include Live Activities updates.
  • Update Views/Library/ZimFileDetail.swift to handle Live Activities for download details.
  • Add Live Activities updates in Views/Library/ZimFilesDownloads.swift.
  • Add a new setting to enable or disable Live Activities in Views/Settings/Settings.swift.
  • Handle alerts for Live Activities in Views/ViewModifiers/AlertHandler.swift.
  • Add Model/DownloadActivityAttributes.swift to define the attributes for Live Activities.

For more details, open the Copilot Workspace session.

Fixes kiwix#1037

Add Live Activities support for displaying download progress on the lock screen and in the Dynamic Island.

* Import `ActivityKit` and create a new `DownloadActivityAttributes` struct in `App/App_iOS.swift`.
* Update `Model/DownloadService.swift` to manage Live Activities during download progress changes.
* Modify `Views/BuildingBlocks/DownloadTaskCell.swift` to include Live Activities updates.
* Update `Views/Library/ZimFileDetail.swift` to handle Live Activities for download details.
* Add Live Activities updates in `Views/Library/ZimFilesDownloads.swift`.
* Add a new setting to enable or disable Live Activities in `Views/Settings/Settings.swift`.
* Handle alerts for Live Activities in `Views/ViewModifiers/AlertHandler.swift`.
* Add `Model/DownloadActivityAttributes.swift` to define the attributes for Live Activities.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/kiwix/kiwix-apple/issues/1037?shareId=XXXX-XXXX-XXXX-XXXX).
@kelson42
Copy link
Contributor

@plyght Thank you for your PR, we will review it, but might take a while considering we are now mostly in the Christmas break.

@plyght
Copy link
Author

plyght commented Dec 22, 2024

we are now mostly in the Christmas break.

@kelson42 All good! Have a wonderful Christmas!

Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

@plyght thank you for your PR.

  • Please don't remove file headers.
  • Doesn't compile on my machine
  • Doesn't compile on CI
  • There are a couple of codefactor issues that can be addressed once the code actually runs.

@BPerlakiH
Copy link
Collaborator

BPerlakiH commented Jan 2, 2025

Thank you for your PR @plyght .

By the first glance, it needs more work.
I can come back to this later. Some first observations:

  • the imported ActivityKit is only available for iOS and only from 16.1. We do use the same code base for both macOS min. 13.0, and iOS min. 15.
  • the framework import should be added to project.yml, since we use that for generating our Xcode project. By bet is that, it should be:
      - sdk: Activity.framework
        destinationFilters:
          - iOS
  • for solving the iOS 16.1 issue, it needs to be wrapped in: @available(iOS 16.1, *)
  • since our views and most of our view models are re-used for both iOS and macOS, adding this to the views (cells) etc, is not the easiest task, and I would not recommend doing that.
  • I think hooking into the DownloadTasksPublisher should be enough to get all the relevant data.
  • Another thing to look out for, is that we can have multiple downloads running simultaneously, and that should be reflected on the Activity as well.

@@ -337,6 +346,11 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat
progress.updateFor(uuid: zimFileID,
downloaded: totalBytesWritten,
total: totalBytesExpectedToWrite)
// Update Live Activity
let progressPercentage = Double(totalBytesWritten) / Double(totalBytesExpectedToWrite)
let speed = Double(bytesWritten) / 1024.0 / 1024.0 // Convert to MB/s
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be incorrect.
It has no notion of time, therefore it cannot represent speed, which is per second.

@@ -265,6 +252,7 @@ struct Settings: View {
SelectedLanaguageLabel()
}.disabled(library.state != .complete)
Toggle("library_settings.toggle.cellular".localized, isOn: $downloadUsingCellular)
Toggle("library_settings.toggle.live_activities".localized, isOn: $enableLiveActivities)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be a nice addition, but the value of it isn't read anywhere, so it's not turning anything on / off atm.

@@ -139,4 +130,14 @@ private struct RootView: UIViewControllerRepresentable {
func updateUIViewController(_ controller: SplitViewController, context: Context) {
}
}

struct DownloadActivityAttributes: ActivityAttributes {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This struct is a duplicate.

@BPerlakiH
Copy link
Collaborator

Closing this PR for now, it requires more work, and a more throughout approach to handle cross platform, and iOS 16.1 availability.

@BPerlakiH BPerlakiH closed this Jan 3, 2025
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.

Add Live Activities for Download % and/or speed (MB/s)
4 participants