diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift index 658e2e2e2c7c..c52106d031d8 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift @@ -38,7 +38,7 @@ struct MediaGridItem: Identifiable, Equatable { } /// Designated initializer for data-bearing states. Initializes every - /// stored property exactly once. + /// stored property exactly once. Reached only via `init(item:)`. private init(media: MediaWithEditContext, id: Int64, state: State) { let payload = media.mediaDetails.parseAsMimeType(mimeType: media.mimeType) let kind = payload.flatMap(MediaKind.init(payload:)) ?? .document diff --git a/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift index 63ae41ce529b..381eaebcbac6 100644 --- a/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift +++ b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift @@ -14,10 +14,27 @@ public struct DownloadableMediaItem: Sendable { } } +/// Output of a successful `downloadForSharing(items:)`. `urls` are the local +/// file URLs to hand to `UIActivityViewController`. `cleanup`, when non-nil, +/// removes the service-owned temp scope (typically the per-batch directory +/// under `temporaryDirectory`); `MediaDetailViewModel.SharePayload` invokes +/// it on activity-sheet dismissal or selection-mode exit. The closure is the +/// only ownership signal โ€” callers do not infer ownership from URL paths. +public struct BulkShareDownloadResult: Sendable { + public let urls: [URL] + public let cleanup: (@Sendable () -> Void)? + + public init(urls: [URL], cleanup: (@Sendable () -> Void)? = nil) { + self.urls = urls + self.cleanup = cleanup + } +} + /// App-injected authenticated downloader. Returns local file URLs suitable -/// for `UIActivityViewController` activity items. Throws on any download or +/// for `UIActivityViewController` activity items plus an optional cleanup +/// closure for the service-owned temp scope. Throws on any download or /// auth failure; the detail VM surfaces the error in `shareErrorMessage`. @MainActor public protocol MediaDetailShareService: AnyObject { - func downloadForSharing(items: [DownloadableMediaItem]) async throws -> [URL] + func downloadForSharing(items: [DownloadableMediaItem]) async throws -> BulkShareDownloadResult } diff --git a/Modules/Sources/WordPressMediaLibrary/Services/MediaShareFilename.swift b/Modules/Sources/WordPressMediaLibrary/Services/MediaShareFilename.swift new file mode 100644 index 000000000000..32ddc43b46a8 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Services/MediaShareFilename.swift @@ -0,0 +1,45 @@ +import Foundation +import WordPressAPI + +/// Filename derivation for the Share action. The primitive `suggested(...)` +/// form is the canonical implementation; both the single-item detail VM +/// (which has a `MediaDetailDisplayModel` snapshot) and the bulk-share +/// path (which has a `MediaWithEditContext`) call through to it via +/// matching primitive fields. The `suggested(for media:)` overload is a +/// convenience for the bulk path. +enum MediaShareFilename { + /// Picks a human-meaningful filename in this priority order: trimmed + /// title, trimmed slug, URL last-path-component, then "media-". + /// Returns `nil` only if every fallback also fails; production callers + /// always have an `id`, so the "media-" tail is the effective floor. + /// + /// Each candidate is checked with `isUsable(_:)`, which rejects the + /// filesystem-special components `.` and `..` (and the empty string). + /// Title and slug are user-controlled site data; without the rejection, + /// a literal "." title with no MIME-derived extension would resolve to + /// the batch directory itself when appended as a path component, and + /// the share `moveItem` would fail silently. + static func suggested(title: String?, slug: String, sourceUrl: String, id: Int64) -> String? { + let trimmedTitle = (title ?? "").trimmingCharacters(in: .whitespacesAndNewlines) + if isUsable(trimmedTitle) { return trimmedTitle } + + let trimmedSlug = slug.trimmingCharacters(in: .whitespacesAndNewlines) + if isUsable(trimmedSlug) { return trimmedSlug } + + if let last = URL(string: sourceUrl)?.lastPathComponent, isUsable(last) { + return last + } + + return "media-\(id)" + } + + private static func isUsable(_ candidate: String) -> Bool { + !candidate.isEmpty && candidate != "." && candidate != ".." + } + + /// Convenience for callers that already hold a full `MediaWithEditContext` + /// (the bulk-share path in `MediaLibraryViewModel.startBulkShare()`). + static func suggested(for media: MediaWithEditContext) -> String? { + suggested(title: media.title.raw, slug: media.slug, sourceUrl: media.sourceUrl, id: media.id) + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift index acf7055284cb..7937f40cd36d 100644 --- a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift +++ b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift @@ -498,4 +498,93 @@ enum Strings { value: "Delete", comment: "Destructive button title in the delete-confirmation alert" ) + + // MARK: - Selection mode + + static let selectionSelect = NSLocalizedString( + "mediaLibrary.selection.select", + value: "Select", + comment: "Nav-bar button that enters selection mode in the V2 Media Library." + ) + + static let selectionTitleEmpty = NSLocalizedString( + "mediaLibrary.selection.title.empty", + value: "Select Items", + comment: "Bottom toolbar title shown in selection mode when no items are selected." + ) + + static let selectionTitleImageSingular = NSLocalizedString( + "mediaLibrary.selection.title.image.singular", + value: "%1$d Image Selected", + comment: "Bottom toolbar title when exactly one image is selected. %1$d is the count (always 1 here)." + ) + + static let selectionTitleImagePlural = NSLocalizedString( + "mediaLibrary.selection.title.image.plural", + value: "%1$d Images Selected", + comment: "Bottom toolbar title when 2+ images are selected. %1$d is the count." + ) + + static let selectionTitleItemSingular = NSLocalizedString( + "mediaLibrary.selection.title.item.singular", + value: "%1$d Item Selected", + comment: "Bottom toolbar title when exactly one item is selected (mixed or non-image)." + ) + + static let selectionTitleItemPlural = NSLocalizedString( + "mediaLibrary.selection.title.item.plural", + value: "%1$d Items Selected", + comment: "Bottom toolbar title when 2+ items are selected (mixed or non-image)." + ) + + // The single-item delete-confirmation title and the destructive "Delete" + // action label are shared with the detail screen (`detailDeleteConfirmation` + // / `detailDeleteAction`); only the multi-item confirmation is selection-only. + static let selectionDeleteConfirmationMany = NSLocalizedString( + "mediaLibrary.selection.deleteConfirmation.many", + value: "Are you sure you want to permanently delete these items?", + comment: "Confirmation dialog title when deleting multiple items from the V2 Media Library." + ) + + static let selectionDeleteFailedMessage = NSLocalizedString( + "mediaLibrary.selection.deleteFailed.message", + value: "Some items couldn't be deleted. Please try again.", + comment: "Alert message shown when a bulk delete fails for some or all of the selected items." + ) + + static let selectionShareNothingMessage = NSLocalizedString( + "mediaLibrary.selection.shareNothing.message", + value: "The selected items can't be shared.", + comment: "Alert message shown when none of the selected items has a shareable media URL." + ) + + static let selectionDeleteAccessibilityLabel = NSLocalizedString( + "mediaLibrary.selection.delete.accessibilityLabel", + value: "Delete selected items", + comment: "VoiceOver label for the icon-only Trash button in the selection-mode bottom toolbar." + ) + + static let accessibilitySelected = NSLocalizedString( + "mediaLibrary.selection.accessibility.selected", + value: "Selected", + comment: "VoiceOver value for a cell whose checkmark badge is on in selection mode." + ) + + static let accessibilityNotSelected = NSLocalizedString( + "mediaLibrary.selection.accessibility.notSelected", + value: "Not selected", + comment: "VoiceOver value for a cell whose checkmark badge is off in selection mode." + ) + + static let shareAccessibilityPreparing = NSLocalizedString( + "mediaLibrary.selection.share.accessibility.preparing", + value: "Preparing items to share", + comment: "VoiceOver label for the inline progress indicator while a bulk share is downloading." + ) + + static let cellDeletingAccessibilityValue = NSLocalizedString( + "mediaLibrary.selection.cell.accessibility.deleting", + value: "Deleting", + comment: "VoiceOver value appended to a cell that has an in-flight delete request." + ) } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/BannerView.swift b/Modules/Sources/WordPressMediaLibrary/Views/BannerView.swift index 94d745cbd5dd..fca07dbeeec3 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/BannerView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/BannerView.swift @@ -2,29 +2,42 @@ import SwiftUI struct BannerView: View { let summary: MediaLibraryViewModel.BannerSummary - let onTap: () -> Void + let onTap: (() -> Void)? var body: some View { - Button(action: onTap) { - HStack(spacing: 12) { - if summary.pendingCount > 0 { - ProgressView() - .progressViewStyle(.circular) - .controlSize(.small) - } - Text(label) - .font(.subheadline) - Spacer() + if let onTap { + Button(action: onTap) { + content + } + .buttonStyle(.plain) + .padding(.horizontal, 16) + .padding(.vertical, 8) + } else { + content + .padding(.horizontal, 16) + .padding(.vertical, 8) + .accessibilityElement(children: .combine) + } + } + + private var content: some View { + HStack(spacing: 12) { + if summary.pendingCount > 0 { + ProgressView() + .progressViewStyle(.circular) + .controlSize(.small) + } + Text(label) + .font(.subheadline) + Spacer() + if onTap != nil { Image(systemName: "chevron.right") .foregroundStyle(.tertiary) } - .padding(.horizontal, 16) - .padding(.vertical, 10) - .background(.thinMaterial, in: .rect(cornerRadius: 12)) } - .buttonStyle(.plain) .padding(.horizontal, 16) - .padding(.vertical, 8) + .padding(.vertical, 10) + .background(.thinMaterial, in: .rect(cornerRadius: 12)) } private var label: String { diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift index 2e6433ca1b85..b871d0b59328 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift @@ -57,8 +57,9 @@ struct MediaDetailView: View { } .sheet(item: $viewModel.sharePayload) { payload in ShareSheetRepresentable(urls: payload.urls) { completed in - viewModel.reportShareDismissed(completed: completed) + viewModel.reportShareDismissed(payload, completed: completed) } + .onAppear { viewModel.shareSheetDidPresent() } } .onChange(of: viewModel.shouldPop) { _, shouldPop in if shouldPop { dismiss() } @@ -66,8 +67,10 @@ struct MediaDetailView: View { .task { viewModel.onAppear() } // The in-flight guards keep anything from covering this screen while // a share prepares, so disappearance means a real pop (or a tab - // switch, which is an acceptable reason to cancel too). - .onDisappear { viewModel.cancelShare() } + // switch, which is an acceptable reason to cancel too). Besides + // cancelling, the VM releases a share payload whose sheet never + // presented, closing the pop-during-download temp-file leak. + .onDisappear { viewModel.viewDidDisappear() } } @ViewBuilder private var editableFieldsSection: some View { diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift index 5375c62b8e86..42ff3f7ab2e1 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift @@ -16,7 +16,24 @@ final class MediaDetailViewModel: ObservableObject { @Published var saveErrorMessage: String? @Published var deleteErrorMessage: String? @Published var shareErrorMessage: String? - @Published var sharePayload: SharePayload? + /// Cleanup chokepoint: whatever payload leaves this slot gets its temp + /// files released. Every dismissal path nils (or replaces) the property, + /// including the one that bypasses the activity controller entirely: an + /// interactive swipe-dismiss tears down the SwiftUI sheet without firing + /// `completionWithItemsHandler`, so no completion-side cleanup can run. + @Published var sharePayload: SharePayload? { + didSet { + if let oldValue, oldValue.id != sharePayload?.id { + oldValue.cleanupTemporaryFiles() + } + isSharePayloadPresented = false + } + } + /// True once the activity sheet for the current `sharePayload` has + /// actually appeared. Any reassignment of `sharePayload` resets it. + /// `viewDidDisappear()` uses it to tell an un-presented payload (safe + /// to release) from one an active activity sheet is still using. + private var isSharePayloadPresented = false @Published private(set) var shouldPop: Bool = false let capabilities: MediaLibraryCapabilities @@ -31,9 +48,25 @@ final class MediaDetailViewModel: ObservableObject { private var inFlightSaveTask: [MediaEditableField: Task] = [:] private var shareTask: Task? + /// Payload presented in `UIActivityViewController`. `cleanup` is the + /// service-supplied closure that removes the temp scope owning `urls`; + /// `cleanupTemporaryFiles()` invokes it iff non-nil. Ownership is + /// explicit โ€” never inferred from URL paths โ€” so a custom share service + /// that returns URLs from outside its own scope cannot accidentally + /// trigger deletion. struct SharePayload: Identifiable { let id = UUID() let urls: [URL] + private let cleanup: (@Sendable () -> Void)? + + init(urls: [URL], cleanup: (@Sendable () -> Void)? = nil) { + self.urls = urls + self.cleanup = cleanup + } + + func cleanupTemporaryFiles() { + cleanup?() + } } init( @@ -199,14 +232,41 @@ final class MediaDetailViewModel: ObservableObject { shareTask?.cancel() } + /// Marks the current `sharePayload` as presented. Wired to the activity + /// sheet content's appearance. + func shareSheetDidPresent() { + isSharePayloadPresented = true + } + + /// Screen-teardown hook. Cancels an in-flight download, and releases a + /// payload whose activity sheet never presented: when the download + /// finishes just as the screen pops, `performShare` assigns + /// `sharePayload` before `onDisappear` fires, and without this pass no + /// code path would ever invoke that payload's cleanup closure. A payload + /// whose sheet is up (e.g. the screen left the window because of a tab + /// switch) is left alone; the sheet's completion handler owns it. + func viewDidDisappear() { + cancelShare() + if sharePayload != nil, !isSharePayloadPresented { + sharePayload = nil + } + } + private func performShare(item: DownloadableMediaItem) async { do { - let urls = try await shareService.downloadForSharing(items: [item]) - try Task.checkCancellation() + let result = try await shareService.downloadForSharing(items: [item]) + if Task.isCancelled { + // Cancelled between the download finishing and this hop; the + // payload will never present, so release its files here. + result.cleanup?() + isSharing = false + return + } isSharing = false - sharePayload = SharePayload(urls: urls) + sharePayload = SharePayload(urls: result.urls, cleanup: result.cleanup) } catch is CancellationError { - // User-initiated cancellation is not an error. + // User-initiated cancellation is not an error. The adapter + // removes its batch directory when the download throws. isSharing = false } catch let error as URLError where error.code == .cancelled { // URLSession surfaces task cancellation as URLError(.cancelled). @@ -218,8 +278,15 @@ final class MediaDetailViewModel: ObservableObject { } } - func reportShareDismissed(completed: Bool) { - sharePayload = nil + /// Called from the activity controller's `completionWithItemsHandler`. + /// Takes the payload the sheet actually presented (captured by the sheet + /// content closure) so a swipe-dismiss that nils the published binding + /// first can't make the identity check match a different payload. The + /// actual temp-file release happens in `sharePayload`'s `didSet`. + func reportShareDismissed(_ payload: SharePayload, completed: Bool) { + if sharePayload?.id == payload.id { + sharePayload = nil + } if completed { tracker.track(.mediaLibrarySharedItemLink) } @@ -230,19 +297,15 @@ final class MediaDetailViewModel: ObservableObject { return DownloadableMediaItem( sourceUrl: url, mimeType: display.mimeType, - suggestedFilename: Self.suggestedFilename(for: display) + suggestedFilename: MediaShareFilename.suggested( + title: display.title, + slug: display.slug, + sourceUrl: display.sourceUrl, + id: display.id + ) ) } - private static func suggestedFilename(for display: MediaDetailDisplayModel) -> String? { - let title = (display.title ?? "").trimmingCharacters(in: .whitespacesAndNewlines) - if !title.isEmpty { return title } - let slug = display.slug.trimmingCharacters(in: .whitespacesAndNewlines) - if !slug.isEmpty { return slug } - if let last = URL(string: display.sourceUrl)?.lastPathComponent, !last.isEmpty { return last } - return "media-\(display.id)" - } - func delete() async { guard !isAnyOperationInFlight else { return } isDeleting = true diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift index 578c22e61f5c..a0a854b71acb 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift @@ -1,17 +1,13 @@ import SwiftUI -/// Pure grid rendering of media items. Takes the items and the aspect-ratio -/// mode directly (no view model) so it can back both the library grid and the -/// search-results grid. -struct MediaGridView: View { +/// Pure grid layout for media items. Takes the items and the aspect-ratio mode +/// directly (no view model) and a per-item cell builder, so it can back both +/// the library grid (rich cells: detail push, selection badges, pending-delete +/// overlays) and the search-results grid (detail-push cells). +struct MediaGridView: View { let items: [MediaGridItem] let isAspectRatioMode: Bool - /// Returns whether a cell should render as tappable. Defaults to never, - /// so callers that don't wire selection (e.g. the search grid) get a - /// plain, non-interactive grid. - var canSelect: (MediaGridItem) -> Bool = { _ in false } - /// Invoked when a selectable cell is tapped. - var onSelect: ((MediaGridItem) -> Void)? + @ViewBuilder let cell: (MediaGridItem) -> Cell @Environment(\.horizontalSizeClass) private var sizeClass @@ -28,27 +24,11 @@ struct MediaGridView: View { ScrollView { LazyVGrid(columns: columns, spacing: spacing) { ForEach(items) { item in - cell(for: item) + cell(item) } } .padding(.top, spacing) .animation(.default, value: isAspectRatioMode) } } - - /// Wraps the cell in a plain `Button` when the item is selectable and a - /// handler is wired. Placeholder cells (.fetching / .missing / .failed) - /// stay non-tappable so taps never push a half-baked detail screen. - @ViewBuilder private func cell(for item: MediaGridItem) -> some View { - if let onSelect, canSelect(item) { - Button { - onSelect(item) - } label: { - MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) - } - .buttonStyle(.plain) - } else { - MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) - } - } } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryHostingController.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryHostingController.swift index 45e85e72fd91..85dfdb10ca83 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryHostingController.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryHostingController.swift @@ -20,6 +20,7 @@ public enum MediaLibraryHostingController { capabilities: MediaLibraryCapabilities, externalPickerOptions: [ExternalMediaPickerOption] = [] ) -> UIViewController { + let hostContext = MediaLibraryHostContext() let view = MediaLibraryContainerView( client: client, tracker: tracker, @@ -28,14 +29,66 @@ public enum MediaLibraryHostingController { shareService: shareService, navigator: navigator, capabilities: capabilities, - externalPickerOptions: externalPickerOptions + externalPickerOptions: externalPickerOptions, + hostContext: hostContext ) - let host = UIHostingController(rootView: view) + let host = HostingController(rootView: view, context: hostContext) host.navigationItem.largeTitleDisplayMode = .never return host } } +/// Live, containment-derived facts about the screen's UIKit hosting that the +/// SwiftUI hierarchy can't observe on its own. Written by `HostingController` +/// from real containment at appearance time, read by `MediaLibraryView`. +final class MediaLibraryHostContext: ObservableObject { + /// When the screen sits above a bottom tab bar (e.g. Jetpack's tab bar, + /// or the iPad split view's compact column), the search field minimizes + /// into a toolbar button so it doesn't stack a second bar at the bottom + /// of the screen. Without one it stays a full-width search bar. + @Published var prefersMinimizedSearchBar = false + /// Invoked when the screen is popped from its navigation controller (a + /// real pop, not a tab switch). Tears down long-lived screen state such + /// as selection mode and its in-flight bulk share. + var handleDidPop: (() -> Void)? +} + +private final class HostingController: UIHostingController { + private let context: MediaLibraryHostContext + + init(rootView: MediaLibraryContainerView, context: MediaLibraryHostContext) { + self.context = context + super.init(rootView: rootView) + // Containment can change without a disappear/appear cycle when an + // iPad window is resized across the compact boundary; re-derive on + // size-class changes too. + registerForTraitChanges([UITraitHorizontalSizeClass.self]) { (self: HostingController, _) in + self.updateSearchBarPreference() + } + } + + @available(*, unavailable) + required dynamic init?(coder aDecoder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + override func viewWillAppear(_ animated: Bool) { + super.viewWillAppear(animated) + updateSearchBarPreference() + } + + override func viewDidDisappear(_ animated: Bool) { + super.viewDidDisappear(animated) + if isMovingFromParent { + context.handleDidPop?() + } + } + + private func updateSearchBarPreference() { + context.prefersMinimizedSearchBar = (tabBarController != nil) + } +} + /// Resolves `WpService` from the actor-isolated `WordPressClient` before /// constructing `MediaLibraryViewModel`, which needs the service synchronously. /// The resolution is a single actor hop (no network), so the loading state is @@ -51,6 +104,7 @@ private struct MediaLibraryContainerView: View { let navigator: any MediaDetailNavigator let capabilities: MediaLibraryCapabilities let externalPickerOptions: [ExternalMediaPickerOption] + let hostContext: MediaLibraryHostContext @State private var resolved: Resolved? @State private var error: Error? @@ -70,7 +124,8 @@ private struct MediaLibraryContainerView: View { service: resolved.service, client: client, tracker: tracker, - externalPickerOptions: externalPickerOptions + externalPickerOptions: externalPickerOptions, + hostContext: hostContext ) } else if let error { EmptyStateView.failure(error: error) { @@ -84,19 +139,23 @@ private struct MediaLibraryContainerView: View { guard resolved == nil else { return } do { let service = try await client.service - resolved = Resolved( - viewModel: MediaLibraryViewModel( - service: service, - client: client, - tracker: tracker, - uploader: uploader, - urlOpener: urlOpener, - shareService: shareService, - navigator: navigator, - capabilities: capabilities - ), - service: service + let viewModel = MediaLibraryViewModel( + service: service, + client: client, + tracker: tracker, + uploader: uploader, + urlOpener: urlOpener, + shareService: shareService, + navigator: navigator, + capabilities: capabilities ) + // Popping the screen ends selection mode, which also cancels + // an in-flight bulk share and releases its payload; nothing + // else references the leaving screen's selection state. + hostContext.handleDidPop = { [weak viewModel] in + viewModel?.exitSelectionMode() + } + resolved = Resolved(viewModel: viewModel, service: service) } catch { self.error = error } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift index 192c69eebd78..958b5c998c6d 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift @@ -94,12 +94,18 @@ private struct MediaSearchResultsView: View { } var body: some View { - MediaGridView( - items: viewModel.displayItems, - isAspectRatioMode: isAspectRatioMode, - canSelect: { viewModel.canOpenDetail(for: $0) }, - onSelect: { pushDetail(for: $0) } - ) + MediaGridView(items: viewModel.displayItems, isAspectRatioMode: isAspectRatioMode) { item in + if viewModel.canOpenDetail(for: item) { + Button { + pushDetail(for: item) + } label: { + MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + } + .buttonStyle(.plain) + } else { + MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + } + } .refreshable { await viewModel.refresh() } .task { tracker.track(.mediaLibrarySearched(queryLength: query.count)) diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift index 0883db1f5a52..d96c04bfe208 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift @@ -15,6 +15,9 @@ struct MediaLibraryView: View { let client: WordPressClient let tracker: any MediaTracker var externalPickerOptions: [ExternalMediaPickerOption] = [] + /// Containment facts (bottom tab bar presence, pop notification) derived + /// by the hosting controller; drives the minimized-search-bar behavior. + @ObservedObject var hostContext: MediaLibraryHostContext @State private var searchText = "" @State private var isAspectRatioMode = AspectRatioPreference.load() @@ -27,6 +30,7 @@ struct MediaLibraryView: View { /// cancel, which never invokes `onCompletion`. @State private var isImportingFile = false @State private var isPresentingUploads = false + @State private var isPresentingDeleteConfirm = false private enum ActivePicker: Hashable, Identifiable { case photoLibrary, takePhoto, takeVideo @@ -34,27 +38,43 @@ struct MediaLibraryView: View { var id: Self { self } } + private var deleteConfirmMessage: String { + let count = viewModel.selectedIDs.count + return count == 1 ? Strings.detailDeleteConfirmation : Strings.selectionDeleteConfirmationMany + } + + /// `.alert(isPresented:)` needs a Bool binding; these bridge the optional + /// error messages to one and clear the message when the alert dismisses. + private var bulkDeleteErrorBinding: Binding { + Binding( + get: { viewModel.bulkDeleteErrorMessage != nil }, + set: { if !$0 { viewModel.bulkDeleteErrorMessage = nil } } + ) + } + private var bulkShareErrorBinding: Binding { + Binding( + get: { viewModel.bulkShareErrorMessage != nil }, + set: { if !$0 { viewModel.bulkShareErrorMessage = nil } } + ) + } + var body: some View { ZStack { if searchText.isEmpty { VStack(spacing: 0) { if let summary = viewModel.bannerSummary { - BannerView(summary: summary) { - isPresentingUploads = true - } + BannerView( + summary: summary, + onTap: viewModel.isSelectionModeActive + ? nil + : { + isPresentingUploads = true + } + ) + } + MediaGridView(items: viewModel.displayItems, isAspectRatioMode: isAspectRatioMode) { item in + cellContent(for: item) } - // Tapping a cell pushes the detail screen through the - // app-injected UIKit navigator. The grid is hosted in a - // UIKit `UINavigationController` (no SwiftUI - // `NavigationStack` ancestor), so `pushDetail` wraps the - // SwiftUI screen in a `UIHostingController` and pushes it - // onto the outer nav controller at tap time. - MediaGridView( - items: viewModel.displayItems, - isAspectRatioMode: isAspectRatioMode, - canSelect: { viewModel.canOpenDetail(for: $0) }, - onSelect: { pushDetail(for: $0) } - ) .refreshable { await viewModel.refresh() } .overlay { libraryOverlay } } @@ -83,13 +103,89 @@ struct MediaLibraryView: View { await viewModel.refresh() } .navigationTitle(Strings.title) - .searchable(text: $searchText, prompt: Strings.searchPrompt) - .minimizedSearchToolbarBehavior() + // Search is suppressed entirely in selection mode: leaving it live let + // the user swap to the search results view while the selection toolbar + // (and its trash/share actions) kept operating on now-off-screen items, + // and on iOS 26 the minimized search capsule also collided with the + // bottom selection bar. Clearing searchText on entry guarantees the + // library grid (not stale search results) is what's selected against. + // The conditional `.searchable` hangs off a zero-size background leaf + // (its preference still reaches the hosting navigation item) so that + // toggling selection mode swaps only that leaf; wrapping the main + // content in a ConditionalContent branch instead would reset the + // subtree's identity, discarding grid scroll position and re-firing + // the load/observe/analytics tasks above. + .background { + if !viewModel.isSelectionModeActive { + Color.clear + .searchable(text: $searchText, prompt: Strings.searchPrompt) + .minimizedSearchToolbarBehavior(hostContext.prefersMinimizedSearchBar) + } + } .autocorrectionDisabled() .textInputAutocapitalization(.never) + .onChange(of: viewModel.isSelectionModeActive) { _, isActive in + if isActive { searchText = "" } + } .toolbar { - filterMenu - addMenu + if viewModel.isSelectionModeActive { + ToolbarItem(placement: .topBarTrailing) { + Button(Strings.commonDone) { viewModel.exitSelectionMode() } + } + } else { + ToolbarItem(placement: .topBarTrailing) { + Button(Strings.selectionSelect) { viewModel.enterSelectionMode() } + // Enablement tracks the visible (kind-filtered) grid, not + // the unfiltered item set, so Select can't enter a Done-only + // dead end over an empty filtered grid. + .disabled(!viewModel.canEnterSelectionMode) + } + filterMenu + addMenu + } + } + // The selection bar is a safe-area inset, not a + // `ToolbarItemGroup(placement: .bottomBar)`: this screen is a + // `UIHostingController` pushed on a UIKit navigation controller (no + // `NavigationStack` ancestor), and in that arrangement SwiftUI + // silently drops bottom-bar toolbar items. Top-bar items bridge + // through `navigationItem`, but nothing populates the UIKit toolbar + // (V1's `SiteMediaViewController` sets `toolbarItems` + + // `setToolbarHidden` by hand). The inset also keeps the last grid + // row reachable above the bar. + .safeAreaInset(edge: .bottom) { + if viewModel.isSelectionModeActive { + selectionToolbar + } + } + .navigationBarBackButtonHidden(viewModel.isSelectionModeActive) + .confirmationDialog( + deleteConfirmMessage, + isPresented: $isPresentingDeleteConfirm, + titleVisibility: .visible + ) { + Button(Strings.detailDeleteAction, role: .destructive) { + Task { await viewModel.confirmBulkDelete() } + } + Button(Strings.commonCancel, role: .cancel) {} + } + .alert( + Strings.detailUnableToDeleteTitle, + isPresented: bulkDeleteErrorBinding, + presenting: viewModel.bulkDeleteErrorMessage + ) { _ in + Button(Strings.commonOK, role: .cancel) { viewModel.bulkDeleteErrorMessage = nil } + } message: { + Text($0) + } + .alert( + Strings.detailUnableToShareTitle, + isPresented: bulkShareErrorBinding, + presenting: viewModel.bulkShareErrorMessage + ) { _ in + Button(Strings.commonOK, role: .cancel) { viewModel.bulkShareErrorMessage = nil } + } message: { + Text($0) } // Present the Uploads queue as a sheet rather than a push. It's a // self-contained management surface (its own toolbar + bulk menu), and @@ -117,6 +213,11 @@ struct MediaLibraryView: View { } } } + .sheet(item: $viewModel.sharePayload) { payload in + ShareSheetRepresentable(urls: payload.urls) { completed in + viewModel.reportShareDismissed(payload, completed: completed) + } + } .sheet(item: $activePicker) { picker in switch picker { case .photoLibrary: @@ -167,6 +268,53 @@ struct MediaLibraryView: View { ) } + /// Wraps openable grid cells in the interaction that matches the current mode. + /// Placeholder cells (.fetching / .missing / .failed) stay static so taps + /// don't push or select a half-baked detail screen. + @ViewBuilder private func cellContent(for item: MediaGridItem) -> some View { + let isPendingDelete = viewModel.pendingDeleteIDs.contains(item.id) + + if isPendingDelete { + MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + .overlay { ProgressView().tint(.white).shadow(radius: 2) } + .opacity(0.4) + .allowsHitTesting(false) + .accessibilityValue(Strings.cellDeletingAccessibilityValue) + .disabled(true) + } else if viewModel.isSelectionModeActive { + if viewModel.canOpenDetail(for: item) { + let isSelected = viewModel.isSelected(item) + Button { + viewModel.toggleSelection(for: item) + } label: { + MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + .overlay(alignment: .topTrailing) { + selectionBadge(isSelected: isSelected) + .opacity(viewModel.isPreparingBulkShare ? 0.5 : 1.0) + } + } + .buttonStyle(.plain) + .disabled(viewModel.isPreparingBulkShare) + .accessibilityAddTraits(isSelected ? .isSelected : []) + .accessibilityValue(isSelected ? Strings.accessibilitySelected : Strings.accessibilityNotSelected) + } else { + MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + .opacity(0.4) + } + } else { + if viewModel.canOpenDetail(for: item) { + Button { + pushDetail(for: item) + } label: { + MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + } + .buttonStyle(.plain) + } else { + MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + } + } + } + private func pushDetail(for item: MediaGridItem) { // Re-resolve the detail VM at push time so we don't capture a stale // snapshot if the underlying cache row was refreshed between the @@ -280,6 +428,68 @@ struct MediaLibraryView: View { } } + /// Bottom selection bar (trash / count title / share), presented as a + /// safe-area inset while selection mode is active. The hidden trash + /// placeholder keeps the title centered when deletion is unsupported. + private var selectionToolbar: some View { + HStack { + if viewModel.detailCapabilities?.supportsDeletion == true { + Button { + isPresentingDeleteConfirm = true + } label: { + Image(systemName: "trash") + } + .accessibilityLabel(Strings.selectionDeleteAccessibilityLabel) + .disabled(viewModel.selectedIDs.isEmpty || viewModel.isPreparingBulkShare) + } else { + Image(systemName: "trash") + .hidden() + .accessibilityHidden(true) + } + Spacer() + Text(viewModel.selectionToolbarTitle).font(.headline) + Spacer() + shareToolbarButton + } + .font(.title3) + .padding(.horizontal, 20) + .padding(.vertical, 12) + .background(.bar) + .overlay(alignment: .top) { Divider() } + } + + @ViewBuilder private var shareToolbarButton: some View { + if viewModel.isPreparingBulkShare { + ProgressView() + .accessibilityLabel(Strings.shareAccessibilityPreparing) + } else { + Button { + viewModel.startBulkShare() + } label: { + Image(systemName: "square.and.arrow.up") + } + .accessibilityLabel(Strings.commonShare) + .disabled(viewModel.selectedIDs.isEmpty) + } + } + + @ViewBuilder private func selectionBadge(isSelected: Bool) -> some View { + ZStack { + Circle() + .fill(isSelected ? Color.accentColor : Color.clear) + .frame(width: 24, height: 24) + Circle() + .stroke(Color.white.opacity(isSelected ? 1.0 : 0.85), lineWidth: 2) + .frame(width: 24, height: 24) + if isSelected { + Image(systemName: "checkmark") + .font(.caption.bold()) + .foregroundStyle(.white) + } + } + .padding(6) + } + private func errorView(_ error: Error) -> some View { VStack(spacing: 12) { Image(systemName: "exclamationmark.triangle") @@ -298,12 +508,13 @@ struct MediaLibraryView: View { } private extension View { - /// Collapses the `.searchable` field into a navigation-bar button that - /// expands on tap, matching the legacy Media screen. The `.minimize` + /// Collapses the `.searchable` field into a toolbar button that expands on + /// tap. Only applied when `isMinimized` is true (host app has a bottom tab + /// bar); otherwise the search field stays a full-width bar. The `.minimize` /// behavior is iOS 26+, so this is a no-op on earlier versions. @ViewBuilder - func minimizedSearchToolbarBehavior() -> some View { - if #available(iOS 26, *) { + func minimizedSearchToolbarBehavior(_ isMinimized: Bool) -> some View { + if #available(iOS 26, *), isMinimized { searchToolbarBehavior(.minimize) } else { self diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift index a89b81a3a1a4..37aae7aaa296 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift @@ -1,5 +1,6 @@ import Combine import Foundation +import OrderedCollections import SwiftUI import WordPressAPI import WordPressAPIInternal @@ -8,7 +9,7 @@ import WordPressCore /// App-target switches that gate which detail screen affordances are /// available. Public so app-side routing can populate it without going /// through the (internal) view model type. -public struct MediaLibraryCapabilities: Equatable { +public struct MediaLibraryCapabilities: Equatable, Sendable { public let supportsAltEditing: Bool public let supportsMetadataEditing: Bool public let supportsDeletion: Bool @@ -76,8 +77,97 @@ final class MediaLibraryViewModel: ObservableObject { @Published private(set) var error: Error? @Published private(set) var isLoadComplete = false - /// Guards re-entrant loads. Safe because each instance owns one collection, - /// so a skipped re-entrant call never loses a distinct load. + // MARK: - Selection state (M5) + + @Published private(set) var isSelectionModeActive: Bool = false + + /// Insertion-ordered set of selected media ids. Read by the view for + /// badge state, by `selectionToolbarTitle`, and by the bulk-action + /// methods. Iteration order is consumed by bulk share to preserve the + /// user's tap order when assembling the activity items. + @Published private(set) var selectedIDs = OrderedSet() + + /// In-flight delete markers, dims + spinners + disables hit testing + /// on the corresponding cells. Cleared inside `performDelete`'s `defer` + /// (both success and failure paths) for pagination safety. + @Published private(set) var pendingDeleteIDs: Set = [] + + /// Identity for the currently-active bulk-share download, which runs in + /// the VM-owned `bulkShareTask` so it survives the view leaving the + /// window (e.g. a tab switch); `exitSelectionMode()` cancels it + /// explicitly. The preparing-vs-idle UI state is derived from this (see + /// `isPreparingBulkShare`), so the two can never drift out of lockstep. + @Published private(set) var bulkShareRequest: BulkShareRequest? + + /// Owns the bulk-share download so its lifetime is the view model's, + /// not the view's; a view-lifetime `.task` would be cancelled by any + /// disappearance and silently drop the preparation. + private var bulkShareTask: Task? + + /// True while a bulk-share download is in flight. Derived from + /// `bulkShareRequest` so there is a single source of truth. + var isPreparingBulkShare: Bool { bulkShareRequest != nil } + + /// Activity-sheet payload. Set by `performBulkShare` on success; + /// presented via `.sheet(item:)`. Nilled in `reportShareDismissed` or + /// `exitSelectionMode`. Cleanup chokepoint: whatever payload leaves this + /// slot gets its temp files released. Every dismissal path nils (or + /// replaces) the property, including the one that bypasses the activity + /// controller entirely: an interactive swipe-dismiss tears down the + /// SwiftUI sheet without firing `completionWithItemsHandler`, so no + /// completion-side cleanup can run. + @Published var sharePayload: MediaDetailViewModel.SharePayload? { + didSet { + if let oldValue, oldValue.id != sharePayload?.id { + oldValue.cleanupTemporaryFiles() + } + } + } + + /// Bulk-delete failure message, presented as an alert. Set when the + /// confirmed delete fails wholesale or partially; cleared by the view. + @Published var bulkDeleteErrorMessage: String? + + /// Bulk-share failure message, presented as an alert. Set when the share + /// download fails or no selected item can be prepared; cleared by the view. + @Published var bulkShareErrorMessage: String? + + /// Toggle-time payload snapshot, keyed by media id. Survives + /// `loadItems` rebuilds of `resolvedMediaByID`, so a selection that + /// spans pages remains shareable after a page-1 refresh. Not + /// `@Published`; the view never reads it directly. Bulk-share-item + /// construction reads it. + private var selectedMediaSnapshots: [Int64: MediaWithEditContext] = [:] + + /// V1 parity title: five variants for empty / image-singular / image-plural + /// / item-singular / item-plural. Reads `selectedMediaSnapshots` for the + /// image-vs-mixed decision so the lookup is cheap and survives refresh. + var selectionToolbarTitle: String { + let count = selectedIDs.count + if count == 0 { return Strings.selectionTitleEmpty } + if allSelectedAreImages { + let template = count == 1 ? Strings.selectionTitleImageSingular : Strings.selectionTitleImagePlural + return String.localizedStringWithFormat(template, count) + } + let template = count == 1 ? Strings.selectionTitleItemSingular : Strings.selectionTitleItemPlural + return String.localizedStringWithFormat(template, count) + } + + private var allSelectedAreImages: Bool { + guard !selectedIDs.isEmpty else { return false } + return selectedIDs.allSatisfy { id in + selectedMediaSnapshots[id]?.mimeType.hasPrefix("image/") == true + } + } + + struct BulkShareRequest: Identifiable { + let id = UUID() + let items: [DownloadableMediaItem] + } + + /// Serializes loads: `load()` waits for an in-flight (possibly cancelled + /// and still-unwinding) load to finish before starting, because each + /// instance owns one collection. private var isLoading = false /// Pure type-filter, extracted so it can be unit-tested directly with @@ -246,6 +336,9 @@ final class MediaLibraryViewModel: ObservableObject { func setKind(_ newKind: MediaKind?) { guard kind != newKind else { return } + if isSelectionModeActive { + exitSelectionMode() + } withAnimation { kind = newKind displayItems = Self.applyingKindFilter(items, kind: newKind) @@ -253,10 +346,240 @@ final class MediaLibraryViewModel: ObservableObject { tracker.track(.mediaLibraryFilterChanged(kind: newKind)) } + // MARK: - Selection mode (M5) + + func enterSelectionMode() { + isSelectionModeActive = true + } + + func exitSelectionMode() { + // Cancel the in-flight bulk-share download and nil bulkShareRequest + // (which also flips isPreparingBulkShare back to false). Nils + // sharePayload too, closing the race where downloads finish and the + // activity sheet is about to present when the user taps Done. + bulkShareTask?.cancel() + bulkShareTask = nil + bulkShareRequest = nil + sharePayload = nil + isSelectionModeActive = false + selectedIDs.removeAll() + selectedMediaSnapshots.removeAll() + } + + /// Toggles the item's id in `selectedIDs` and captures/clears its + /// payload snapshot in `selectedMediaSnapshots`. No-op for items where + /// `canOpenDetail` returns false (placeholders, error rows without + /// cached payload). Reads `resolvedMediaByID[item.id]` for the snapshot + /// payload at toggle time; that payload then survives subsequent + /// refreshes / pagination changes that mutate `resolvedMediaByID`. + func toggleSelection(for item: MediaGridItem) { + guard canOpenDetail(for: item) else { return } + guard let media = resolvedMediaByID[item.id] else { return } + if selectedIDs.contains(item.id) { + selectedIDs.remove(item.id) + selectedMediaSnapshots[item.id] = nil + } else { + selectedIDs.append(item.id) + selectedMediaSnapshots[item.id] = media + } + } + + // MARK: - Bulk delete (M5) + + /// Fire-and-forget bulk delete. Selection mode exits immediately on + /// confirm; per-cell dim+spinner indicates in-flight deletes. Both + /// success and failure clear the pending marker in `performDelete`'s + /// `defer` (both success and failure paths) for pagination safety. A + /// wholesale or partial failure surfaces `bulkDeleteErrorMessage` (the + /// detail screen has an equivalent delete-failure alert). + func confirmBulkDelete() async { + let ids = Array(selectedIDs) + guard !ids.isEmpty else { return } + + pendingDeleteIDs.formUnion(ids) + exitSelectionMode() + + let service: WpService + do { + service = try await client.service + } catch { + Loggers.mediaLibrary.error("Bulk delete: client resolve failed: \(error)") + pendingDeleteIDs.subtract(ids) + bulkDeleteErrorMessage = Strings.selectionDeleteFailedMessage + return + } + + let successCount = await withTaskGroup(of: Bool.self) { group in + let maxConcurrent = 3 + var iterator = ids.makeIterator() + + func submit(_ id: Int64) { + group.addTask { [weak self] in + await self?.performDelete(id: id, service: service) ?? false + } + } + for _ in 0.. 0 { + tracker.track(.mediaLibraryDeletedItems(count: successCount)) + } + if successCount < ids.count { + bulkDeleteErrorMessage = Strings.selectionDeleteFailedMessage + } + } + + /// One delete attempt. Returns `true` on success. `defer` clears the + /// pending marker on both paths synchronously. `MediaLibraryViewModel` + /// is `@MainActor`, so direct mutation is safe; no nested Task is needed. + private func performDelete(id: Int64, service: WpService) async -> Bool { + defer { pendingDeleteIDs.remove(id) } + do { + _ = try await service.media().deleteMediaPermanently(mediaId: MediaId(id)) + return true + } catch { + Loggers.mediaLibrary.error("Bulk delete failed for id \(id): \(error)") + return false + } + } + + // MARK: - Bulk share (M5) + + /// Starts a bulk-share preparation. Builds `DownloadableMediaItem`s + /// from `selectedMediaSnapshots` (NOT `resolvedMediaByID`, the snapshot + /// map survives refresh and pagination). Preflight keeps only items with + /// an absolute http/https URL with a host. Fires + /// `.siteMediaShareTapped(count:)` before the download starts, using the + /// actually-prepared count. + func startBulkShare() { + guard !selectedIDs.isEmpty, !isPreparingBulkShare else { return } + let ids = Array(selectedIDs) + let items: [DownloadableMediaItem] = ids.compactMap { id in + guard let media = selectedMediaSnapshots[id], + let url = URL(string: media.sourceUrl), + let scheme = url.scheme?.lowercased(), + scheme == "http" || scheme == "https", + url.host?.isEmpty == false + else { return nil } + return DownloadableMediaItem( + sourceUrl: url, + mimeType: media.mimeType, + suggestedFilename: MediaShareFilename.suggested(for: media) + ) + } + + // When no selected item yields a shareable URL the tap would otherwise + // do nothing (no spinner, no sheet); surface an alert instead. A partial + // drop still proceeds: the activity sheet shows the prepared subset, which + // is its own feedback. + guard !items.isEmpty else { + Loggers.mediaLibrary.warning( + "Bulk share: tap produced 0 preparable items from \(ids.count) selected ids" + ) + bulkShareErrorMessage = Strings.selectionShareNothingMessage + return + } + + if items.count != ids.count { + Loggers.mediaLibrary.warning( + "Bulk share: \(ids.count - items.count) selected items dropped (missing snapshot or unparseable sourceUrl); proceeding with \(items.count)" + ) + } + tracker.track(.siteMediaShareTapped(count: items.count)) + let request = BulkShareRequest(items: items) + bulkShareRequest = request + // The handle is deliberately not cleared on completion: a stale + // task's trailing write could clobber a newer task's handle and + // leave it uncancellable, while a finished task kept around is + // inert (cancelling it is a no-op). The next share or exit + // overwrites it. + bulkShareTask = Task { [weak self] in + await self?.performBulkShare(request) + } + } + + /// Runs the bulk-share download inside the VM-owned `bulkShareTask`; + /// `exitSelectionMode()` cancels it. Cleanup and `sharePayload` + /// publication are request-id-scoped: a stale cancelled task that + /// unwinds after a newer request started will NOT clobber the newer + /// request's state. + private func performBulkShare(_ request: BulkShareRequest) async { + defer { + if bulkShareRequest?.id == request.id { + bulkShareRequest = nil + } + } + guard let shareService else { return } + do { + let result = try await shareService.downloadForSharing(items: request.items) + do { + try Task.checkCancellation() + } catch { + result.cleanup?() + throw error + } + guard bulkShareRequest?.id == request.id else { + result.cleanup?() + return + } + sharePayload = .init(urls: result.urls, cleanup: result.cleanup) + } catch is CancellationError { + // expected, Done cancelled mid-download + } catch let urlError as URLError where urlError.code == .cancelled { + // URLSession surfaces cancellation as URLError(.cancelled), not CancellationError + } catch { + // The download is an atomic batch (one failure discards the whole + // batch dir), so surface the failure instead of silently reverting + // the spinner to the share icon, mirroring the detail screen's + // share-failure alert. + Loggers.mediaLibrary.error("Bulk share download failed: \(error)") + if bulkShareRequest?.id == request.id { + bulkShareErrorMessage = error.localizedDescription + } + } + } + + /// Called from the activity controller's `completionWithItemsHandler`. + /// Takes the payload the sheet actually presented (captured by the sheet + /// content closure) so a swipe-dismiss that nils the published binding + /// first can't make the identity check match a different payload. The + /// actual temp-file release happens in `sharePayload`'s `didSet`. + /// V1 bulk parity: completed share exits selection mode; cancelled + /// activity sheet keeps selection intact for retry. Neither path fires + /// `.mediaLibrarySharedItemLink`; that event is V1-single-item only. + func reportShareDismissed(_ payload: MediaDetailViewModel.SharePayload, completed: Bool) { + if sharePayload?.id == payload.id { + sharePayload = nil + } + if completed { + exitSelectionMode() + } + } + + func isSelected(_ item: MediaGridItem) -> Bool { + selectedIDs.contains(item.id) + } + // MARK: Load (eager) func load() async { - guard !isLoading else { return } + // A cancelled predecessor may still be unwinding (its defer hasn't + // reset `isLoading` yet) when a replacement load starts; wait for it + // instead of dropping this call, so a task restart can't strand the + // library half-loaded with `isLoadComplete` stuck false. + while isLoading { + if Task.isCancelled { return } + try? await Task.sleep(for: .milliseconds(20)) + } isLoading = true defer { isLoading = false } @@ -276,7 +599,14 @@ final class MediaLibraryViewModel: ObservableObject { break } } - if !Task.isCancelled { isLoadComplete = true } + if !Task.isCancelled { + isLoadComplete = true + // Now that every page is loaded, drop any selection that points + // at an item the server no longer returns (e.g. deleted from + // another device). reload()'s own reconcile is a no-op until this + // flag flips, so the final pass has to happen here. + reconcileSelection() + } } catch { if !(error is CancellationError), !Task.isCancelled { Loggers.mediaLibrary.error("Media library load failed: \(error)") @@ -324,6 +654,7 @@ final class MediaLibraryViewModel: ObservableObject { items = metadataItems.map(MediaGridItem.init(item:)) displayItems = Self.applyingKindFilter(items, kind: kind) } + reconcileSelection() } catch { if !(error is CancellationError) { Loggers.mediaLibrary.error("Failed to load items: \(error)") @@ -331,15 +662,42 @@ final class MediaLibraryViewModel: ObservableObject { } } + /// Drops any selected id (and its share snapshot) that the loaded item set + /// no longer contains, so the toolbar count can't strand a ghost the user + /// can't deselect and bulk share can't 404 on a deleted item's stale URL. + /// Guarded on `isLoadComplete`: during initial load / pagination `items` + /// holds only a partial set, and pruning there would drop a legitimate + /// selection on a not-yet-loaded page (the share snapshot deliberately + /// survives pagination). Once every page is loaded, an absent id is a real + /// deletion. + private func reconcileSelection() { + guard isLoadComplete, !selectedIDs.isEmpty else { return } + let liveIDs = Set(items.map(\.id)) + let staleIDs = selectedIDs.filter { !liveIDs.contains($0) } + guard !staleIDs.isEmpty else { return } + for id in staleIDs { + selectedIDs.remove(id) + selectedMediaSnapshots[id] = nil + } + } + // MARK: Detail navigation - /// Cheap check for whether the cell should render as tappable. - /// Mirrors the early-out conditions in `makeDetailVM(for:)` without + /// Cheap check for whether the cell should render as tappable. Mirrors + /// the early-out conditions in `makeDetailVM(for:)` without /// constructing the throwaway detail VM on every cell render. func canOpenDetail(for item: MediaGridItem) -> Bool { detailNavigator != nil && resolvedMediaByID[item.id] != nil } + /// Whether Select should be enabled. Evaluated over `displayItems` (the + /// kind-filtered grid the user actually sees), not the unfiltered `items`, + /// so Select can't enter selection mode over an empty filtered grid. The + /// `contains` short-circuits on the first openable item. + var canEnterSelectionMode: Bool { + displayItems.contains { canOpenDetail(for: $0) } + } + /// Builds a `MediaDetailViewModel` for the tapped cell. Returns nil when /// the cell carries no resolvable payload (placeholder states), or when the /// instance has no detail wiring (e.g. a search-results grid). diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaShareFilenameTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaShareFilenameTests.swift new file mode 100644 index 000000000000..702aae425633 --- /dev/null +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaShareFilenameTests.swift @@ -0,0 +1,123 @@ +import Testing +@testable import WordPressMediaLibrary + +@Suite("MediaShareFilename.suggested") +struct MediaShareFilenameTests { + // Primitive form, both call sites (grid VM + detail VM) flow through this. + @Test func primitives_returnsTrimmedTitleWhenPresent() { + #expect( + MediaShareFilename.suggested( + title: " My Photo ", + slug: "ignored", + sourceUrl: "https://example.com/image.jpg", + id: 1 + ) == "My Photo" + ) + } + + @Test func primitives_fallsBackToSlugWhenTitleBlank() { + #expect( + MediaShareFilename.suggested( + title: " ", + slug: " my-slug ", + sourceUrl: "https://example.com/image.jpg", + id: 1 + ) == "my-slug" + ) + } + + @Test func primitives_fallsBackToLastPathComponentWhenTitleAndSlugBlank() { + #expect( + MediaShareFilename.suggested( + title: nil, + slug: "", + sourceUrl: "https://example.com/2024/05/IMG_1234.jpg", + id: 1 + ) == "IMG_1234.jpg" + ) + } + + @Test func primitives_fallsBackToMediaIdWhenNothingElseAvailable() { + #expect(MediaShareFilename.suggested(title: nil, slug: "", sourceUrl: "", id: 42) == "media-42") + } + + // Convenience overload, forwards to the primitive helper from a `MediaWithEditContext`. + @Test func forMedia_forwardsToTitle() { + let media = makeMediaFixture(titleRaw: "Vacation", slug: "ignored", sourceUrl: "https://example.com/image.jpg") + #expect(MediaShareFilename.suggested(for: media) == "Vacation") + } + + @Test func forMedia_forwardsToSlugFallback() { + let media = makeMediaFixture( + titleRaw: " ", + slug: " vacation-photo ", + sourceUrl: "https://example.com/image.jpg" + ) + #expect(MediaShareFilename.suggested(for: media) == "vacation-photo") + } + + @Test func forMedia_forwardsToSourceUrlFallback() { + let media = makeMediaFixture(titleRaw: nil, slug: "", sourceUrl: "https://example.com/2024/05/IMG_1234.jpg") + #expect(MediaShareFilename.suggested(for: media) == "IMG_1234.jpg") + } + + @Test func forMedia_forwardsToMediaIdFallback() { + let media = makeMediaFixture(id: 42, titleRaw: nil, slug: "", sourceUrl: "") + #expect(MediaShareFilename.suggested(for: media) == "media-42") + } + + // Filesystem-special candidate rejection. Title and slug are + // user-controlled site data and could literally be "." or "..". Without + // rejection, the share path would append them as path components and + // moveItem would fail (NSCocoaErrorDomain 516). + + @Test func primitives_rejectsDotTitle_fallsBackToSlug() { + #expect( + MediaShareFilename.suggested(title: ".", slug: "my-slug", sourceUrl: "https://example.com/IMG_1.jpg", id: 1) + == "my-slug" + ) + } + + @Test func primitives_rejectsDotDotTitle_fallsBackToSlug() { + #expect( + MediaShareFilename.suggested( + title: "..", + slug: "my-slug", + sourceUrl: "https://example.com/IMG_1.jpg", + id: 1 + ) == "my-slug" + ) + } + + @Test func primitives_rejectsDotSlug_fallsBackToUrlLastComponent() { + #expect( + MediaShareFilename.suggested( + title: nil, + slug: ".", + sourceUrl: "https://example.com/2024/05/IMG_1.jpg", + id: 1 + ) == "IMG_1.jpg" + ) + } + + @Test func primitives_rejectsDotDotSlug_fallsBackToUrlLastComponent() { + #expect( + MediaShareFilename.suggested( + title: nil, + slug: "..", + sourceUrl: "https://example.com/2024/05/IMG_1.jpg", + id: 1 + ) == "IMG_1.jpg" + ) + } + + @Test func primitives_rejectsAllSpecialComponents_fallsBackToMediaId() { + #expect(MediaShareFilename.suggested(title: ".", slug: "..", sourceUrl: "", id: 42) == "media-42") + } + + @Test func primitives_rejectsWhitespaceWrappedDot_fallsBackToSlug() { + // Title is trimmed before the special-component check, so " . " is + // first reduced to "." and then rejected. + #expect(MediaShareFilename.suggested(title: " . ", slug: "my-slug", sourceUrl: "", id: 1) == "my-slug") + } +} diff --git a/Modules/Tests/WordPressMediaLibraryTests/TestSupport.swift b/Modules/Tests/WordPressMediaLibraryTests/TestSupport.swift index fee6b19433d9..211d7bda4da5 100644 --- a/Modules/Tests/WordPressMediaLibraryTests/TestSupport.swift +++ b/Modules/Tests/WordPressMediaLibraryTests/TestSupport.swift @@ -114,8 +114,20 @@ func waitUntil( // MARK: - MediaWithEditContext fixture +private final class TestMediaDetails: MediaDetails { + override func parseAsMimeType(mimeType: String) -> MediaDetailsPayload? { + nil + } +} + extension MediaWithEditContext { - static func fixture(id: Int64 = 9999) -> MediaWithEditContext { + static func fixture( + id: Int64 = 9999, + titleRaw: String? = nil, + slug: String = "", + sourceUrl: String = "", + mimeType: String = "" + ) -> MediaWithEditContext { MediaWithEditContext( id: id, date: "", @@ -124,13 +136,13 @@ extension MediaWithEditContext { link: "", modified: "", modifiedGmt: Date(timeIntervalSince1970: 0), - slug: "", + slug: slug, status: .inherit, postType: "", password: nil, permalinkTemplate: "", generatedSlug: "", - title: PostTitleWithEditContext(raw: nil, rendered: ""), + title: PostTitleWithEditContext(raw: titleRaw, rendered: titleRaw ?? ""), author: 0, commentStatus: .closed, pingStatus: .closed, @@ -139,15 +151,26 @@ extension MediaWithEditContext { caption: MediaCaptionWithEditContext(raw: "", rendered: ""), description: MediaDescriptionWithEditContext(raw: "", rendered: ""), mediaType: .file, - mimeType: "", - mediaDetails: MediaDetails(noHandle: .init()), + mimeType: mimeType, + mediaDetails: TestMediaDetails(noHandle: .init()), postId: nil, - sourceUrl: "", + sourceUrl: sourceUrl, missingImageSizes: [] ) } } +/// Top-level alias matching the call shape used by selection / share tests. +func makeMediaFixture( + id: Int64 = 9999, + titleRaw: String? = nil, + slug: String = "", + sourceUrl: String = "", + mimeType: String = "" +) -> MediaWithEditContext { + .fixture(id: id, titleRaw: titleRaw, slug: slug, sourceUrl: sourceUrl, mimeType: mimeType) +} + // MARK: - MediaUploadPolicy helper func makeAllowEverythingPolicy() -> MediaUploadPolicy { diff --git a/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift index d9e1ecaa4d00..446fc343987e 100644 --- a/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift +++ b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift @@ -16,38 +16,80 @@ final class MediaDetailShareServiceAdapter: MediaDetailShareService { self.authenticator = authenticator } - func downloadForSharing(items: [DownloadableMediaItem]) async throws -> [URL] { + func downloadForSharing(items: [DownloadableMediaItem]) async throws -> BulkShareDownloadResult { + guard !items.isEmpty else { + return BulkShareDownloadResult(urls: [], cleanup: nil) + } + + let batchDir = FileManager.default.temporaryDirectory + .appendingPathComponent("media-share-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: batchDir, withIntermediateDirectories: true) + + var usedNames: Set = [] var result: [URL] = [] - for item in items { - let request = try await authenticator.authenticatedRequest(for: item.sourceUrl, host: MediaHost(blog)) - let (downloadedURL, response) = try await URLSession.shared.download(for: request) - guard let http = response as? HTTPURLResponse, (200...299).contains(http.statusCode) else { - // URLSession.download wrote a temp file before we knew the - // response code. Clean it up so we don't leak. - try? FileManager.default.removeItem(at: downloadedURL) - throw URLError(.badServerResponse) + do { + for item in items { + try Task.checkCancellation() + let request = try await authenticator.authenticatedRequest(for: item.sourceUrl, host: MediaHost(blog)) + try Task.checkCancellation() + let (downloadedURL, response) = try await URLSession.shared.download(for: request) + guard let http = response as? HTTPURLResponse, (200...299).contains(http.statusCode) else { + // URLSession.download wrote a temp file before we knew the + // response code. Clean it up so we don't leak. + try? FileManager.default.removeItem(at: downloadedURL) + throw URLError(.badServerResponse) + } + + let filename = Self.uniqueFilename(for: item, against: usedNames) + usedNames.insert(filename) + let destination = batchDir.appendingPathComponent(filename) + do { + try FileManager.default.moveItem(at: downloadedURL, to: destination) + } catch { + try? FileManager.default.removeItem(at: downloadedURL) + throw error + } + result.append(destination) } - let url = try moveDownloadedFile(at: downloadedURL, item: item) - result.append(url) + } catch { + try? FileManager.default.removeItem(at: batchDir) + throw error } - return result + // Cleanup ownership is explicit: the closure captures the batch + // directory the adapter just created. SharePayload invokes it on + // activity-sheet dismissal or selection-mode exit; no caller infers + // ownership from URL paths. + return BulkShareDownloadResult( + urls: result, + cleanup: { try? FileManager.default.removeItem(at: batchDir) } + ) } - private func moveDownloadedFile(at source: URL, item: DownloadableMediaItem) throws -> URL { - let dir = FileManager.default.temporaryDirectory - let filename = Self.resolveFilename(for: item) - let destination = dir.appendingPathComponent(filename) - // Replace any prior temp file at the destination so the share path - // is idempotent within a session. - try? FileManager.default.removeItem(at: destination) - try FileManager.default.moveItem(at: source, to: destination) - return destination + static func uniqueFilename(for item: DownloadableMediaItem, against used: Set) -> String { + let base = resolveFilename(for: item) + let usedNames = Set(used.map { $0.lowercased() }) + if !usedNames.contains(base.lowercased()) { + return base + } + + let stem = (base as NSString).deletingPathExtension + let ext = (base as NSString).pathExtension + var counter = 2 + while true { + let candidate = ext.isEmpty ? "\(stem)-\(counter)" : "\(stem)-\(counter).\(ext)" + if !usedNames.contains(candidate.lowercased()) { + return candidate + } + counter += 1 + } } /// Filename derivation (design ยง Filename derivation): /// 1. Start with `suggestedFilename ?? sourceUrl.lastPathComponent`, /// trimmed, with `/` replaced by `-`; empty or dot-only names fall - /// back to "media". + /// back to "media". The module-level helper already screens + /// user-controlled title/slug, but this is the final filesystem + /// boundary so it screens the URL-last-component fallback path too. /// 2. Validate any apparent extension against `mimeType`. A dot segment /// in a human title ("Logo v2.0") is not an extension; only a known /// UTType that agrees with the MIME type counts.