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

MediaUIView - Fix image download bug (#2131) #2215

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions IceCubesApp.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1545,6 +1545,7 @@
INFOPLIST_KEY_LSApplicationCategoryType = "public.app-category.social-networking";
INFOPLIST_KEY_NSCameraUsageDescription = "Upload photos & videos to attach to your Mastodon posts.";
INFOPLIST_KEY_NSHumanReadableCopyright = "© 2024 Thomas Ricouard";
INFOPLIST_KEY_NSPhotoLibraryAddUsageDescription = "Ice Cubes would like to save the selected photo in your photo library.";
INFOPLIST_KEY_NSPhotoLibraryUsageDescription = "Upload photos & videos to Mastodon";
INFOPLIST_KEY_UIApplicationSceneManifest_Generation = YES;
"INFOPLIST_KEY_UIApplicationSceneManifest_Generation[sdk=iphoneos*]" = YES;
Expand Down Expand Up @@ -1611,6 +1612,7 @@
INFOPLIST_KEY_LSApplicationCategoryType = "public.app-category.social-networking";
INFOPLIST_KEY_NSCameraUsageDescription = "Upload photos & videos to attach to your Mastodon posts.";
INFOPLIST_KEY_NSHumanReadableCopyright = "© 2024 Thomas Ricouard";
INFOPLIST_KEY_NSPhotoLibraryAddUsageDescription = "Ice Cubes would like to save the selected photo in your photo library.";
INFOPLIST_KEY_NSPhotoLibraryUsageDescription = "Upload photos & videos to Mastodon";
INFOPLIST_KEY_UIApplicationSceneManifest_Generation = YES;
"INFOPLIST_KEY_UIApplicationSceneManifest_Generation[sdk=iphoneos*]" = YES;
Expand Down
2 changes: 2 additions & 0 deletions IceCubesApp/App/IceCubesApp-release.entitlements
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
<true/>
<key>com.apple.security.network.client</key>
<true/>
<key>com.apple.security.personal-information.photos-library</key>
<true/>
<key>keychain-access-groups</key>
<array>
<string>$(AppIdentifierPrefix)$(BUNDLE_ID_PREFIX).IceCubesApp</string>
Expand Down
2 changes: 2 additions & 0 deletions IceCubesApp/App/IceCubesApp.entitlements
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
<true/>
<key>com.apple.security.network.client</key>
<true/>
<key>com.apple.security.personal-information.photos-library</key>
<true/>
<key>keychain-access-groups</key>
<array>
<string>$(AppIdentifierPrefix)$(BUNDLE_ID_PREFIX).IceCubesApp</string>
Expand Down
15 changes: 13 additions & 2 deletions Packages/MediaUI/Sources/MediaUI/MediaUIView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Models
import Nuke
import QuickLook
import SwiftUI
import Photos

public struct MediaUIView: View, @unchecked Sendable {
private let data: [DisplayData]
Expand Down Expand Up @@ -144,6 +145,8 @@ private struct SavePhotoToolbarItem: ToolbarContent, @unchecked Sendable {
state = .unsaved
}
}
} else {
state = .unsaved
}
}
} label: {
Expand Down Expand Up @@ -180,9 +183,17 @@ private struct SavePhotoToolbarItem: ToolbarContent, @unchecked Sendable {
}
return nil
}

private func saveImage(url: URL) async -> Bool {

Choose a reason for hiding this comment

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

Is it possible to change the var status into a let and use a switch case? Or is the if statement necessary after the requestAuthorization for addOnly? I had something in mind like this:

private func saveImage(url: URL) async -> Bool {
        guard let image = try? await uiimageFor(url: url) else { return false }

        let status: PHAuthorizationStatus
        switch PHPhotoLibrary.authorizationStatus(for: .addOnly) {
        case .authorized:
            UIImageWriteToSavedPhotosAlbum(image, nil, nil, nil)
        default:
            await PHPhotoLibrary.requestAuthorization(for: .addOnly)
            status = PHPhotoLibrary.authorizationStatus(for: .addOnly)
        }
        return true
    }

Keeps it more readable without the if statements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, D. Prodano, for your help. The guard statement is definitely a better solution than my if clause. Regarding the other if statements, my intention was to initiate the image download immediately after requesting authorization. Otherwise, the user would have to manually restart the download after granting authorization. What are your thoughts on this approach?

if let image = try? await uiimageFor(url: url) {
guard let image = try? await uiimageFor(url: url) else { return false }

var status = PHPhotoLibrary.authorizationStatus(for: .addOnly)

if status != .authorized {
await PHPhotoLibrary.requestAuthorization(for: .addOnly)
status = PHPhotoLibrary.authorizationStatus(for: .addOnly)
}
if status == .authorized {
UIImageWriteToSavedPhotosAlbum(image, nil, nil, nil)
return true
}
Expand Down
Loading