From fc8a22978a62486f4ea4ba90f3f249d2bed6ab92 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 19 May 2026 22:40:20 +1200 Subject: [PATCH 1/4] Add V2 Media Library detail/edit screen Implements the SwiftUI detail screen reached from the V2 Media Library grid with per-field push edits, single-item delete and share, and analytics parity with the existing UIKit detail screen. Saves use a per-field serial queue so server-side ordering is last-write-wins, and adopt the cache-aware MediaService.updateMedia path so the grid's existing cache observer fans out updates without a manual nudge. Field edits also commit when the editor disappears, matching the V1 editor's viewWillDisappear save that users rely on, and row taps bail while a delete or share is in flight so a pop cannot strand a pushed screen. Share routes through an app-injected service that authenticates source URLs via MediaRequestAuthenticator and streams the download into a temp file. URL row opens via an injected opener that wraps WebViewControllerFactory. Cell tap and field-row push bridge through an app-injected MediaDetailNavigator that wraps SwiftUI screens in UIHostingController and pushes onto the outer UINavigationController, avoiding nested-NavigationStack double nav bars. --- .../Analytics/MediaTracker.swift | 7 + .../Models/MediaDetailDisplayModel.swift | 48 ++++ .../Models/MediaEditableField.swift | 47 ++++ .../Models/MediaKind.swift | 12 + ...MediaMetadataCollectionItem+Resolved.swift | 18 ++ .../Services/MediaDetailNavigator.swift | 17 ++ .../Services/MediaDetailShareService.swift | 23 ++ .../Services/MediaDetailURLOpener.swift | 9 + .../Strings/Strings.swift | 179 ++++++++++++ .../Views/Detail/MediaDetailView.swift | 227 +++++++++++++++ .../Views/Detail/MediaDetailViewModel.swift | 260 ++++++++++++++++++ .../Views/Detail/MediaFieldEditorView.swift | 43 +++ .../Views/Detail/MediaPreviewHeader.swift | 60 ++++ .../Detail/ShareSheetRepresentable.swift | 33 +++ .../Views/MediaGridView.swift | 24 +- .../Views/MediaLibraryHostingController.swift | 18 +- .../Views/MediaLibraryView.swift | 38 ++- .../Views/MediaLibraryViewModel.swift | 81 +++++- .../MediaKindTests.swift | 11 + .../Analytics/MediaTrackerAdapter.swift | 20 ++ .../Media/MediaLibraryRouting.swift | 18 +- .../V2/MediaDetailNavigatorAdapter.swift | 23 ++ .../V2/MediaDetailShareServiceAdapter.swift | 63 +++++ .../V2/MediaDetailURLOpenerAdapter.swift | 28 ++ 24 files changed, 1292 insertions(+), 15 deletions(-) create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaDetailDisplayModel.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaEditableField.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaMetadataCollectionItem+Resolved.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Services/MediaDetailNavigator.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Services/MediaDetailURLOpener.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaFieldEditorView.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaPreviewHeader.swift create mode 100644 Modules/Sources/WordPressMediaLibrary/Views/Detail/ShareSheetRepresentable.swift create mode 100644 WordPress/Classes/ViewRelated/Media/V2/MediaDetailNavigatorAdapter.swift create mode 100644 WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift create mode 100644 WordPress/Classes/ViewRelated/Media/V2/MediaDetailURLOpenerAdapter.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift b/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift index 1df5c8388a08..6494903cb808 100644 --- a/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift +++ b/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift @@ -20,6 +20,13 @@ public enum MediaTrackerEvent: Sendable { // Upload events: case mediaLibraryAdded(source: MediaUploadSource, kind: MediaKind) case mediaLibraryUploadRetried + + // Detail / Edit events: + case mediaLibraryPreviewedItem + case mediaLibraryEditedItemMetadata + case mediaLibraryDeletedItems(count: Int) + case siteMediaShareTapped(count: Int) + case mediaLibrarySharedItemLink } public enum MediaUploadSource: Sendable { diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaDetailDisplayModel.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaDetailDisplayModel.swift new file mode 100644 index 000000000000..82e3520fc4f4 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaDetailDisplayModel.swift @@ -0,0 +1,48 @@ +import Foundation +import WordPressAPI +import WordPressAPIInternal + +/// Snapshot model the V2 detail screen reads and binds to. Built once from +/// a `MediaWithEditContext` at detail-VM init, then mutated in place when a +/// per-field save returns its server response. Keeping the merge logic +/// against a Swift-native value type avoids rebuilding a 27-arg UniFFI +/// struct on every save. +struct MediaDetailDisplayModel: Equatable, Sendable { + let id: Int64 + var title: String? + var caption: String + var description: String + var altText: String + let mimeType: String + let sourceUrl: String + let mediaDetails: MediaDetails + let dateGmt: Date + let slug: String + let kind: MediaKind + + init(media: MediaWithEditContext) { + self.id = media.id + self.title = media.title.raw + self.caption = media.caption.raw + self.description = media.description.raw + self.altText = media.altText + self.mimeType = media.mimeType + self.sourceUrl = media.sourceUrl + self.mediaDetails = media.mediaDetails + self.dateGmt = media.dateGmt + self.slug = media.slug + self.kind = .from(mimeType: media.mimeType) + } + + /// Adopts the server's value for one field after a successful save. The + /// other fields stay at their local values so a concurrent different- + /// field save can't clobber siblings by returning a stale snapshot. + mutating func apply(_ field: MediaEditableField, fromServer server: MediaWithEditContext) { + switch field { + case .title: self.title = server.title.raw + case .caption: self.caption = server.caption.raw + case .description: self.description = server.description.raw + case .altText: self.altText = server.altText + } + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaEditableField.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaEditableField.swift new file mode 100644 index 000000000000..d7b0fd50c07b --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaEditableField.swift @@ -0,0 +1,47 @@ +import Foundation + +/// Editable metadata fields on the V2 detail screen. Each case maps to one +/// `MediaUpdateParams` slot. Alt-text visibility is gated at the +/// `MediaDetailViewModel.visibleEditableFields` layer, not on this enum. +enum MediaEditableField: Hashable { + case title + case caption + case description + case altText + + var localizedTitle: String { + switch self { + case .title: return Strings.detailFieldTitle + case .caption: return Strings.detailFieldCaption + case .description: return Strings.detailFieldDescription + case .altText: return Strings.detailFieldAltText + } + } + + var placeholder: String { + switch self { + case .title: return Strings.detailFieldTitlePlaceholder + case .caption: return Strings.detailFieldCaptionPlaceholder + case .description: return Strings.detailFieldDescriptionPlaceholder + case .altText: return Strings.detailFieldAltTextPlaceholder + } + } + + var hint: String { + switch self { + case .title: return Strings.detailFieldTitleHint + case .caption: return Strings.detailFieldCaptionHint + case .description: return Strings.detailFieldDescriptionHint + case .altText: return Strings.detailFieldAltTextHint + } + } + + func value(in display: MediaDetailDisplayModel) -> String { + switch self { + case .title: return display.title ?? "" + case .caption: return display.caption + case .description: return display.description + case .altText: return display.altText + } + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift index 9af193b58d95..63fb44c03cea 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift @@ -33,6 +33,18 @@ public enum MediaKind: String, CaseIterable, Hashable, Sendable { self = .document } } + + /// Derives the kind directly from a MIME-type string. Mirrors the + /// prefix logic in wordpress-rs' `MediaDetails::parse_as_mime_type` + /// (`image/*`, `video/*`, `audio/*`, else document) but avoids the + /// FFI call and full-payload JSON deserialization those callers pay + /// for when only the kind tag is needed. + static func from(mimeType: String) -> MediaKind { + if mimeType.hasPrefix("image/") { return .image } + if mimeType.hasPrefix("video/") { return .video } + if mimeType.hasPrefix("audio/") { return .audio } + return .document + } } // MARK: - UI helpers diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaMetadataCollectionItem+Resolved.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaMetadataCollectionItem+Resolved.swift new file mode 100644 index 000000000000..2eb2371bdbc7 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaMetadataCollectionItem+Resolved.swift @@ -0,0 +1,18 @@ +import Foundation +import WordPressAPI +import WordPressAPIInternal + +extension MediaMetadataCollectionItem { + /// Extracts the `MediaWithEditContext` from data-bearing states. + /// Returns nil for placeholder states (.fetching / .missing / .failed) + /// where no media payload is carried. + var resolvedMedia: MediaWithEditContext? { + switch state { + case .fresh(let entity): return entity.data + case .stale(let entity): return entity.data + case .fetchingWithData(let entity): return entity.data + case .failedWithData(_, let entity): return entity.data + case .fetching, .missing, .failed: return nil + } + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailNavigator.swift b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailNavigator.swift new file mode 100644 index 000000000000..15a296eec9c3 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailNavigator.swift @@ -0,0 +1,17 @@ +import Foundation +import UIKit + +/// App-injected UIKit navigation seam for the V2 Media Library detail flow. +/// The module wraps SwiftUI screens (`MediaDetailView`, `MediaFieldEditorView`) +/// in `UIHostingController` and asks the navigator to push them onto the +/// hosting controller's outer `UINavigationController`. The app-target +/// adapter resolves the current nav controller at push time. +/// +/// Why: hosting `MediaLibraryView` inside an outer `UINavigationController` +/// AND wrapping its body in a SwiftUI `NavigationStack` produces a stacked +/// double nav bar. Bridging pushes through UIKit avoids the nested-stack +/// problem entirely. +@MainActor +public protocol MediaDetailNavigator: AnyObject { + func push(_ viewController: UIViewController) +} diff --git a/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift new file mode 100644 index 000000000000..63ae41ce529b --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailShareService.swift @@ -0,0 +1,23 @@ +import Foundation + +/// Information the share path needs to authenticate, download, and name a +/// single item. The URL and MIME type drive the request and filename. +public struct DownloadableMediaItem: Sendable { + public let sourceUrl: URL + public let mimeType: String? + public let suggestedFilename: String? + + public init(sourceUrl: URL, mimeType: String?, suggestedFilename: String?) { + self.sourceUrl = sourceUrl + self.mimeType = mimeType + self.suggestedFilename = suggestedFilename + } +} + +/// App-injected authenticated downloader. Returns local file URLs suitable +/// for `UIActivityViewController` activity items. 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] +} diff --git a/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailURLOpener.swift b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailURLOpener.swift new file mode 100644 index 000000000000..6187d29594cc --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Services/MediaDetailURLOpener.swift @@ -0,0 +1,9 @@ +import Foundation + +/// App-injected opener for the URL row on the V2 detail screen. App-target +/// implementation wraps `WebViewControllerFactory.controller(url:blog:source:)` +/// and pushes onto the resolved nav controller. +@MainActor +public protocol MediaDetailURLOpener: AnyObject { + func open(_ url: URL, mediaTitle: String?) +} diff --git a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift index 7fa0af6b827a..2b86c0826fda 100644 --- a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift +++ b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift @@ -314,4 +314,183 @@ enum Strings { value: "Choose File", comment: "Add-menu item that opens the system file picker." ) + + // MARK: - Detail — fields + + static let detailFieldTitle = NSLocalizedString( + "mediaLibrary.detail.field.title.label", + value: "Title", + comment: "Label for the title field on the media detail screen" + ) + static let detailFieldCaption = NSLocalizedString( + "mediaLibrary.detail.field.caption.label", + value: "Caption", + comment: "Label for the caption field on the media detail screen" + ) + static let detailFieldDescription = NSLocalizedString( + "mediaLibrary.detail.field.description.label", + value: "Description", + comment: "Label for the description field on the media detail screen" + ) + static let detailFieldAltText = NSLocalizedString( + "mediaLibrary.detail.field.altText.label", + value: "Alt Text", + comment: "Label for the alt text field on the media detail screen" + ) + + static let detailFieldTitlePlaceholder = NSLocalizedString( + "mediaLibrary.detail.field.title.placeholder", + value: "Title", + comment: "Placeholder for the title editor" + ) + static let detailFieldCaptionPlaceholder = NSLocalizedString( + "mediaLibrary.detail.field.caption.placeholder", + value: "Caption", + comment: "Placeholder for the caption editor" + ) + static let detailFieldDescriptionPlaceholder = NSLocalizedString( + "mediaLibrary.detail.field.description.placeholder", + value: "Description", + comment: "Placeholder for the description editor" + ) + static let detailFieldAltTextPlaceholder = NSLocalizedString( + "mediaLibrary.detail.field.altText.placeholder", + value: "Alt text", + comment: "Placeholder for the alt text editor" + ) + + static let detailFieldTitleHint = NSLocalizedString( + "mediaLibrary.detail.field.title.hint", + value: "Image title", + comment: "Hint shown under the title editor" + ) + static let detailFieldCaptionHint = NSLocalizedString( + "mediaLibrary.detail.field.caption.hint", + value: "Image caption", + comment: "Hint shown under the caption editor" + ) + static let detailFieldDescriptionHint = NSLocalizedString( + "mediaLibrary.detail.field.description.hint", + value: "Image description", + comment: "Hint shown under the description editor" + ) + static let detailFieldAltTextHint = NSLocalizedString( + "mediaLibrary.detail.field.altText.hint", + value: "Alt text", + comment: "Hint shown under the alt text editor" + ) + + static let detailShareErrorInvalidURL = NSLocalizedString( + "mediaLibrary.detail.share.error.invalidURL", + value: "Source URL is invalid.", + comment: "Error shown when share fails due to an invalid source URL." + ) + + static let detailPreviewImageAccessibility = NSLocalizedString( + "mediaLibrary.detail.preview.imageAccessibility", + value: "Image preview", + comment: "Accessibility label for the image preview header." + ) + static let detailPreviewVideoAccessibility = NSLocalizedString( + "mediaLibrary.detail.preview.videoAccessibility", + value: "Video preview", + comment: "Accessibility label for the video preview header." + ) + static let detailPreviewAudioAccessibility = NSLocalizedString( + "mediaLibrary.detail.preview.audioAccessibility", + value: "Audio", + comment: "Accessibility label for the audio icon header." + ) + static let detailPreviewDocumentAccessibility = NSLocalizedString( + "mediaLibrary.detail.preview.documentAccessibility", + value: "Document", + comment: "Accessibility label for the document icon header." + ) + + static let commonDone = NSLocalizedString( + "mediaLibrary.common.done", + value: "Done", + comment: "Confirmation action — used in editor toolbars" + ) + static let commonOK = NSLocalizedString( + "mediaLibrary.common.ok", + value: "OK", + comment: "Acknowledgement action — used in alert dismissals" + ) + static let commonCancel = NSLocalizedString( + "mediaLibrary.common.cancel", + value: "Cancel", + comment: "Cancel action — used in alert dismissals" + ) + static let commonShare = NSLocalizedString( + "mediaLibrary.common.share", + value: "Share", + comment: "Accessibility label for the share button" + ) + + static let detailMetadataURL = NSLocalizedString( + "mediaLibrary.detail.metadata.url", + value: "URL", + comment: "Label for the URL row on the media detail screen" + ) + static let detailMetadataFileName = NSLocalizedString( + "mediaLibrary.detail.metadata.fileName", + value: "File Name", + comment: "Label for the file name row on the media detail screen" + ) + static let detailMetadataFileType = NSLocalizedString( + "mediaLibrary.detail.metadata.fileType", + value: "File Type", + comment: "Label for the file type row on the media detail screen" + ) + static let detailMetadataFileSize = NSLocalizedString( + "mediaLibrary.detail.metadata.fileSize", + value: "File Size", + comment: "Label for the file size row on the media detail screen" + ) + static let detailMetadataDimensions = NSLocalizedString( + "mediaLibrary.detail.metadata.dimensions", + value: "Dimensions", + comment: "Label for the dimensions row on the media detail screen" + ) + static let detailMetadataUploaded = NSLocalizedString( + "mediaLibrary.detail.metadata.uploaded", + value: "Uploaded", + comment: "Label for the uploaded-date row on the media detail screen" + ) + static let detailMetadataMimeType = NSLocalizedString( + "mediaLibrary.detail.metadata.mimeType", + value: "MIME Type", + comment: "Label for the MIME type row on the media detail screen" + ) + static let detailIdFooter = NSLocalizedString( + "mediaLibrary.detail.idFooter", + value: "ID %1$lld", + comment: "Footer caption showing the entity ID; %1$lld is the media ID" + ) + static let detailUnableToSaveTitle = NSLocalizedString( + "mediaLibrary.detail.unableToSaveTitle", + value: "Unable to save changes", + comment: "Title for the save-failure alert on the detail screen" + ) + static let detailUnableToDeleteTitle = NSLocalizedString( + "mediaLibrary.detail.unableToDeleteTitle", + value: "Unable to delete media", + comment: "Title for the delete-failure alert on the detail screen" + ) + static let detailUnableToShareTitle = NSLocalizedString( + "mediaLibrary.detail.unableToShareTitle", + value: "Unable to share media", + comment: "Title for the share-failure alert on the detail screen" + ) + static let detailDeleteConfirmation = NSLocalizedString( + "mediaLibrary.detail.deleteConfirmation", + value: "Are you sure you want to permanently delete this item?", + comment: "Confirmation message in the delete alert" + ) + static let detailDeleteAction = NSLocalizedString( + "mediaLibrary.detail.deleteAction", + value: "Delete", + comment: "Destructive button title in the delete-confirmation alert" + ) } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift new file mode 100644 index 000000000000..ad8a32bae55e --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift @@ -0,0 +1,227 @@ +import SwiftUI +import UniformTypeIdentifiers +import WordPressAPI + +struct MediaDetailView: View { + @StateObject var viewModel: MediaDetailViewModel + @State private var isConfirmingDelete = false + @Environment(\.dismiss) private var dismiss + + var body: some View { + Form { + Section { + EmptyView() + } header: { + MediaPreviewHeader(display: viewModel.display) + .frame(maxWidth: .infinity) + .listRowInsets(EdgeInsets()) + .textCase(nil) + } + + editableFieldsSection + + metadataSection + } + .navigationTitle(viewModel.displayValue(for: .title)) + .navigationBarTitleDisplayMode(.inline) + .toolbar { detailToolbar } + .alert(Strings.detailUnableToSaveTitle, isPresented: saveErrorBinding, presenting: viewModel.saveErrorMessage) { + _ in + Button(Strings.commonOK, role: .cancel) { viewModel.saveErrorMessage = nil } + } message: { + Text($0) + } + .alert( + Strings.detailUnableToDeleteTitle, + isPresented: deleteErrorBinding, + presenting: viewModel.deleteErrorMessage + ) { _ in + Button(Strings.commonOK, role: .cancel) { viewModel.deleteErrorMessage = nil } + } message: { + Text($0) + } + .alert( + Strings.detailUnableToShareTitle, + isPresented: shareErrorBinding, + presenting: viewModel.shareErrorMessage + ) { _ in + Button(Strings.commonOK, role: .cancel) { viewModel.shareErrorMessage = nil } + } message: { + Text($0) + } + .alert(Strings.detailDeleteConfirmation, isPresented: $isConfirmingDelete) { + Button(Strings.detailDeleteAction, role: .destructive) { + Task { await viewModel.delete() } + } + Button(Strings.commonCancel, role: .cancel) {} + } + .sheet(item: $viewModel.sharePayload) { payload in + ShareSheetRepresentable(urls: payload.urls) { completed in + viewModel.reportShareDismissed(completed: completed) + } + } + .onChange(of: viewModel.shouldPop) { _, shouldPop in + if shouldPop { dismiss() } + } + .task { viewModel.onAppear() } + } + + @ViewBuilder private var editableFieldsSection: some View { + Section { + ForEach(viewModel.visibleEditableFields, id: \.self) { field in + editableRow(for: field) + } + } + } + + @ViewBuilder private func editableRow(for field: MediaEditableField) -> some View { + if viewModel.isEditable(field) { + // UIKit-bridge push instead of SwiftUI NavigationLink. The + // detail screen is hosted on the outer UINavigationController + // (no NavigationStack ancestor), so NavigationLink wouldn't + // resolve a destination. The VM constructs the editor's + // UIHostingController and asks the injected navigator to push. + // We render the disclosure chevron manually since the + // NavigationLink affordance is gone. + Button { + viewModel.pushFieldEditor(for: field) + } label: { + HStack { + LabeledContent(field.localizedTitle, value: viewModel.displayValue(for: field)) + Image(systemName: "chevron.forward") + .font(.footnote.weight(.semibold)) + .foregroundStyle(.tertiary) + } + } + .buttonStyle(.plain) + } else { + LabeledContent(field.localizedTitle, value: viewModel.displayValue(for: field)) + .foregroundStyle(.secondary) + } + } + + @ViewBuilder private var metadataSection: some View { + Section { + if !viewModel.display.sourceUrl.isEmpty { + // Custom HStack — `LabeledContent` collapses to a vertical + // stack when its trailing content has tap behaviour, so the + // label-on-top / value-below layout we got with Button or + // Link in the trailing slot doesn't match V1. Manual HStack + // gives label-on-left + truncated-link-on-right reliably. + HStack(spacing: 8) { + Text(Strings.detailMetadataURL) + Text(viewModel.display.sourceUrl) + .lineLimit(1) + .truncationMode(.middle) + // Dimmed while an operation is in flight; the VM + // guard in `openSourceURL` is the functional gate. + .foregroundStyle( + viewModel.isAnyOperationInFlight ? AnyShapeStyle(.secondary) : AnyShapeStyle(.tint) + ) + .frame(maxWidth: .infinity, alignment: .trailing) + } + .contentShape(Rectangle()) + .onTapGesture { viewModel.openSourceURL() } + .accessibilityElement(children: .combine) + .accessibilityAddTraits(.isLink) + .accessibilityAction { viewModel.openSourceURL() } + } + LabeledContent(Strings.detailMetadataFileName, value: fileName) + LabeledContent(Strings.detailMetadataFileType, value: fileType) + if let size = fileSizeString { + LabeledContent(Strings.detailMetadataFileSize, value: size) + } + if let dims = dimensionsString { + LabeledContent(Strings.detailMetadataDimensions, value: dims) + } + LabeledContent(Strings.detailMetadataUploaded, value: uploadedString) + LabeledContent(Strings.detailMetadataMimeType, value: viewModel.display.mimeType.nonEmptyOr("—")) + } footer: { + Text(String.localizedStringWithFormat(Strings.detailIdFooter, viewModel.display.id)) + .frame(maxWidth: .infinity, alignment: .center) + .padding(.top, 20) + } + } + + @ToolbarContentBuilder private var detailToolbar: some ToolbarContent { + if viewModel.capabilities.supportsDeletion { + ToolbarItem(placement: .topBarTrailing) { + Button { + isConfirmingDelete = true + } label: { + Image(systemName: "trash").accessibilityLabel(Strings.detailDeleteAction) + } + .disabled(!viewModel.isTrashEnabled) + } + } + ToolbarItem(placement: .topBarTrailing) { + Button { + Task { await viewModel.share() } + } label: { + Image(systemName: "square.and.arrow.up").accessibilityLabel(Strings.commonShare) + } + .disabled(!viewModel.isShareEnabled) + } + } + + private var fileName: String { + URL(string: viewModel.display.sourceUrl)?.lastPathComponent.nonEmptyOr("—") ?? "—" + } + + private var fileType: String { + let urlExt = URL(string: viewModel.display.sourceUrl)?.pathExtension ?? "" + if !urlExt.isEmpty { return urlExt.uppercased() } + if let mimeExt = UTType(mimeType: viewModel.display.mimeType)?.preferredFilenameExtension { + return mimeExt.uppercased() + } + return "—" + } + + private var fileSizeString: String? { + guard let payload = viewModel.display.mediaDetails.parseAsMimeType(mimeType: viewModel.display.mimeType) else { + return nil + } + let bytes: UInt64 = { + switch payload { + case .image(let d): return d.fileSize + case .video(let d): return d.fileSize + case .audio(let d): return d.fileSize + case .document(let d): return d.fileSize + } + }() + let formatter = ByteCountFormatter() + return formatter.string(fromByteCount: Int64(bytes)) + } + + private var dimensionsString: String? { + guard let payload = viewModel.display.mediaDetails.parseAsMimeType(mimeType: viewModel.display.mimeType) else { + return nil + } + switch payload { + case .image(let d): return "\(d.width) × \(d.height)" + case .video(let d): return "\(d.width) × \(d.height)" + case .audio, .document: return nil + } + } + + private var uploadedString: String { + let formatter = DateFormatter() + formatter.dateStyle = .medium + formatter.timeStyle = .none + return formatter.string(from: viewModel.display.dateGmt) + } + + private var saveErrorBinding: Binding { + Binding(get: { viewModel.saveErrorMessage != nil }, set: { if !$0 { viewModel.saveErrorMessage = nil } }) + } + private var deleteErrorBinding: Binding { + Binding(get: { viewModel.deleteErrorMessage != nil }, set: { if !$0 { viewModel.deleteErrorMessage = nil } }) + } + private var shareErrorBinding: Binding { + Binding(get: { viewModel.shareErrorMessage != nil }, set: { if !$0 { viewModel.shareErrorMessage = nil } }) + } +} + +private extension String { + func nonEmptyOr(_ fallback: String) -> String { isEmpty ? fallback : self } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift new file mode 100644 index 000000000000..9d1e3dae2c71 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift @@ -0,0 +1,260 @@ +import Foundation +import SwiftUI +import UIKit +import UniformTypeIdentifiers +import WordPressAPI +import WordPressAPIInternal +import WordPressCore + +@MainActor +final class MediaDetailViewModel: ObservableObject { + @Published private(set) var display: MediaDetailDisplayModel + @Published private(set) var pendingValues: [MediaEditableField: String] = [:] + @Published private(set) var inFlightSaveFields: Set = [] + @Published private(set) var isDeleting: Bool = false + @Published private(set) var isSharing: Bool = false + @Published var saveErrorMessage: String? + @Published var deleteErrorMessage: String? + @Published var shareErrorMessage: String? + @Published var sharePayload: SharePayload? + @Published private(set) var shouldPop: Bool = false + + let capabilities: MediaLibraryCapabilities + + private let client: WordPressClient + private let shareService: any MediaDetailShareService + private let tracker: any MediaTracker + private let urlOpener: any MediaDetailURLOpener + private let navigator: any MediaDetailNavigator + + private var didFireOpen = false + private var inFlightSaveTask: [MediaEditableField: Task] = [:] + + struct SharePayload: Identifiable { + let id = UUID() + let urls: [URL] + } + + init( + media: MediaWithEditContext, + client: WordPressClient, + tracker: any MediaTracker, + urlOpener: any MediaDetailURLOpener, + shareService: any MediaDetailShareService, + navigator: any MediaDetailNavigator, + capabilities: MediaLibraryCapabilities + ) { + self.display = MediaDetailDisplayModel(media: media) + self.client = client + self.shareService = shareService + self.tracker = tracker + self.urlOpener = urlOpener + self.navigator = navigator + self.capabilities = capabilities + } + + /// Bridges the per-field push into UIKit navigation. `MediaDetailView` + /// is hosted on the outer `UINavigationController` via + /// `UIHostingController`; SwiftUI `NavigationLink` requires a + /// `NavigationStack` ancestor, which we deliberately don't have (it + /// double-stacks with the outer UIKit nav bar). Pushing a fresh + /// `UIHostingController(rootView: MediaFieldEditorView)` integrates + /// with the outer nav controller cleanly. + func pushFieldEditor(for field: MediaEditableField) { + // The row renders non-tappable when the field isn't editable; bail + // defensively if a stale tap races past the UI gate (e.g. a delete + // started between the render and the tap). + guard isEditable(field) else { return } + let editor = MediaFieldEditorView( + field: field, + value: displayValue(for: field), + onCommit: { [weak self] newValue in + self?.commitField(field, value: newValue) + } + ) + let host = UIHostingController(rootView: editor) + host.navigationItem.largeTitleDisplayMode = .never + navigator.push(host) + } + + func onAppear() { + guard !didFireOpen else { return } + didFireOpen = true + tracker.track(.mediaLibraryPreviewedItem) + } + + func displayValue(for field: MediaEditableField) -> String { + pendingValues[field] ?? field.value(in: display) + } + + var visibleEditableFields: [MediaEditableField] { + var fields: [MediaEditableField] = [.title, .caption, .description] + if showsAltText { fields.append(.altText) } + return fields + } + + private var showsAltText: Bool { + capabilities.supportsAltEditing && display.mimeType.hasPrefix("image/") + } + + func commitField(_ field: MediaEditableField, value: String) { + // Skip work when the value didn't change. + guard value != displayValue(for: field) else { return } + // Row is disabled while a save is in flight; bail defensively if + // a stale tap races past the UI gate. + guard inFlightSaveTask[field] == nil else { return } + + pendingValues[field] = value + startSave(field: field, value: value) + } + + private func startSave(field: MediaEditableField, value: String) { + inFlightSaveFields.insert(field) + let task: Task = Task { [weak self] in + await self?.performSave(field: field, value: value) + } + inFlightSaveTask[field] = task + } + + private func performSave(field: MediaEditableField, value: String) async { + let params = Self.makeUpdateParams(field: field, value: value) + let outcome: Result + do { + let service = try await client.service + let server = try await service.media().updateMedia(mediaId: MediaId(display.id), params: params) + outcome = .success(server) + } catch { + outcome = .failure(error) + } + + await MainActor.run { + self.applyOutcome(field: field, outcome: outcome) + } + } + + private func applyOutcome(field: MediaEditableField, outcome: Result) { + var lastError: String? + switch outcome { + case .success(let server): + self.display.apply(field, fromServer: server) + self.tracker.track(.mediaLibraryEditedItemMetadata) + case .failure(let error): + Loggers.mediaLibrary.error("Metadata save failed for field \(field): \(error)") + lastError = error.localizedDescription + } + + // The cache-aware `updateMedia` call upserts the server response into + // the wordpress-rs cache and fires `notify_collections`, so any live + // `MediaMetadataCollection` (e.g. the grid VM's) refreshes its + // membership without us nudging it. + inFlightSaveTask[field] = nil + inFlightSaveFields.remove(field) + pendingValues[field] = nil + + if let lastError { + saveErrorMessage = lastError + } + } + + var isMetadataEditingEnabled: Bool { + capabilities.supportsMetadataEditing && !isDeleting && !isSharing && !shouldPop + } + + func isEditable(_ field: MediaEditableField) -> Bool { + isMetadataEditingEnabled && !inFlightSaveFields.contains(field) + } + + var isTrashEnabled: Bool { + capabilities.supportsDeletion && !isAnyOperationInFlight + } + + var isAnyOperationInFlight: Bool { + !inFlightSaveFields.isEmpty || isDeleting || isSharing || shouldPop + } + + var isShareEnabled: Bool { + !isAnyOperationInFlight && !display.sourceUrl.isEmpty + } + + func share() async { + guard isShareEnabled else { return } + isSharing = true + tracker.track(.siteMediaShareTapped(count: 1)) + guard let item = makeShareItem() else { + isSharing = false + shareErrorMessage = Strings.detailShareErrorInvalidURL + return + } + do { + let urls = try await shareService.downloadForSharing(items: [item]) + isSharing = false + sharePayload = SharePayload(urls: urls) + } catch { + Loggers.mediaLibrary.error("Media share failed for id \(display.id): \(error)") + isSharing = false + shareErrorMessage = error.localizedDescription + } + } + + func reportShareDismissed(completed: Bool) { + sharePayload = nil + if completed { + tracker.track(.mediaLibrarySharedItemLink) + } + } + + private func makeShareItem() -> DownloadableMediaItem? { + guard let url = URL(string: display.sourceUrl) else { return nil } + return DownloadableMediaItem( + sourceUrl: url, + mimeType: display.mimeType, + suggestedFilename: Self.suggestedFilename(for: display) + ) + } + + 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 + do { + let service = try await client.service + _ = try await service.media().deleteMediaPermanently(mediaId: MediaId(display.id)) + tracker.track(.mediaLibraryDeletedItems(count: 1)) + // `shouldPop` joins the in-flight gates, so flipping it before + // resetting `isDeleting` keeps every affordance disabled through + // the pop animation. + shouldPop = true + isDeleting = false + } catch { + Loggers.mediaLibrary.error("Media delete failed for id \(display.id): \(error)") + deleteErrorMessage = error.localizedDescription + isDeleting = false + } + } + + func openSourceURL() { + // Pushing the web view during an in-flight delete would let the + // delete's pop remove the web view instead of this screen, stranding + // a detail view for an item that no longer exists. + guard !isAnyOperationInFlight else { return } + guard let url = URL(string: display.sourceUrl), !display.sourceUrl.isEmpty else { return } + urlOpener.open(url, mediaTitle: display.title) + } + + private static func makeUpdateParams(field: MediaEditableField, value: String) -> MediaUpdateParams { + switch field { + case .title: return MediaUpdateParams(title: value) + case .caption: return MediaUpdateParams(caption: value) + case .description: return MediaUpdateParams(description: value) + case .altText: return MediaUpdateParams(altText: value) + } + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaFieldEditorView.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaFieldEditorView.swift new file mode 100644 index 000000000000..8b4b88e7b668 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaFieldEditorView.swift @@ -0,0 +1,43 @@ +import SwiftUI + +struct MediaFieldEditorView: View { + let field: MediaEditableField + @State var value: String + let onCommit: (String) -> Void + @Environment(\.dismiss) private var dismiss + + var body: some View { + Form { + Section { + switch field { + case .title, .altText: + TextField(field.placeholder, text: $value, axis: .vertical) + .lineLimit(1...4) + case .caption, .description: + TextEditor(text: $value) + .frame(minHeight: 200) + } + } footer: { + Text(field.hint) + } + } + .navigationTitle(field.localizedTitle) + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .topBarTrailing) { + Button(Strings.commonDone) { + onCommit(value) + dismiss() + } + } + } + // Commit on back button and swipe-back too, matching the V1 editor's + // save-on-exit behavior. A cancelled interactive swipe never reaches + // the disappearance callback, so it neither commits nor loses the + // typed value. After Done this is a no-op (commitField skips + // unchanged values). + .onDisappear { + onCommit(value) + } + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaPreviewHeader.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaPreviewHeader.swift new file mode 100644 index 000000000000..1c77a0f48d0d --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaPreviewHeader.swift @@ -0,0 +1,60 @@ +import SwiftUI +import AsyncImageKit + +struct MediaPreviewHeader: View { + let display: MediaDetailDisplayModel + + var body: some View { + ZStack { + switch display.kind { + case .image: + CachedAsyncImage(url: thumbnailURL) { phase in + switch phase { + case .success(let image): image.resizable().scaledToFit() + case .empty: ProgressView() + case .failure: typeIcon + @unknown default: typeIcon + } + } + case .video: + if let url = URL(string: display.sourceUrl) { + CachedAsyncImage(videoUrl: url) { image in + image.resizable().scaledToFit() + } placeholder: { + typeIcon + } + } else { + typeIcon + } + case .audio, .document: + typeIcon + } + } + .frame(maxWidth: .infinity, minHeight: 200, maxHeight: 320) + .background(Color(.secondarySystemBackground)) + .accessibilityLabel(accessibilityLabel) + } + + private var thumbnailURL: URL? { + guard + let payload = display.mediaDetails.parseAsMimeType(mimeType: display.mimeType), + case .image(let imageDetails) = payload + else { return URL(string: display.sourceUrl) } + return MediaThumbnailURL.pick(from: imageDetails, sourceUrl: display.sourceUrl) + } + + private var typeIcon: some View { + Image(systemName: display.kind.systemImageName) + .font(.system(size: 64)) + .foregroundStyle(.secondary) + } + + private var accessibilityLabel: String { + switch display.kind { + case .image: return Strings.detailPreviewImageAccessibility + case .video: return Strings.detailPreviewVideoAccessibility + case .audio: return Strings.detailPreviewAudioAccessibility + case .document: return Strings.detailPreviewDocumentAccessibility + } + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/ShareSheetRepresentable.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/ShareSheetRepresentable.swift new file mode 100644 index 000000000000..b1d1aa46b9db --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/ShareSheetRepresentable.swift @@ -0,0 +1,33 @@ +import SwiftUI +import UIKit + +struct ShareSheetRepresentable: UIViewControllerRepresentable { + let urls: [URL] + let onDismiss: (_ completed: Bool) -> Void + + func makeUIViewController(context: Context) -> UIActivityViewController { + let controller = UIActivityViewController(activityItems: urls, applicationActivities: nil) + controller.completionWithItemsHandler = { _, completed, _, _ in + onDismiss(completed) + } + // On iPad, `UIActivityViewController` defaults to popover + // presentation and crashes if no anchor is set. SwiftUI's + // `.sheet(item:)` doesn't expose the originating Share toolbar + // button as a popover source, so anchor on the activity + // controller's own view (centered, no arrow) — a centered modal + // popover. iPhone presentations ignore the popover settings. + if let popover = controller.popoverPresentationController { + popover.sourceView = controller.view + popover.sourceRect = CGRect( + x: controller.view.bounds.midX, + y: controller.view.bounds.midY, + width: 0, + height: 0 + ) + popover.permittedArrowDirections = [] + } + return controller + } + + func updateUIViewController(_ uiViewController: UIActivityViewController, context: Context) {} +} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift index 135eb7872b11..578c22e61f5c 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridView.swift @@ -6,6 +6,12 @@ import SwiftUI 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)? @Environment(\.horizontalSizeClass) private var sizeClass @@ -22,11 +28,27 @@ struct MediaGridView: View { ScrollView { LazyVGrid(columns: columns, spacing: spacing) { ForEach(items) { item in - MediaGridCell(item: item, isAspectRatioMode: isAspectRatioMode) + cell(for: 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 540cbbd496fe..45e85e72fd91 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryHostingController.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryHostingController.swift @@ -14,12 +14,20 @@ public enum MediaLibraryHostingController { client: WordPressClient, tracker: any MediaTracker, uploader: MediaUploader, + urlOpener: any MediaDetailURLOpener, + shareService: any MediaDetailShareService, + navigator: any MediaDetailNavigator, + capabilities: MediaLibraryCapabilities, externalPickerOptions: [ExternalMediaPickerOption] = [] ) -> UIViewController { let view = MediaLibraryContainerView( client: client, tracker: tracker, uploader: uploader, + urlOpener: urlOpener, + shareService: shareService, + navigator: navigator, + capabilities: capabilities, externalPickerOptions: externalPickerOptions ) let host = UIHostingController(rootView: view) @@ -38,6 +46,10 @@ private struct MediaLibraryContainerView: View { let client: WordPressClient let tracker: any MediaTracker let uploader: MediaUploader + let urlOpener: any MediaDetailURLOpener + let shareService: any MediaDetailShareService + let navigator: any MediaDetailNavigator + let capabilities: MediaLibraryCapabilities let externalPickerOptions: [ExternalMediaPickerOption] @State private var resolved: Resolved? @@ -77,7 +89,11 @@ private struct MediaLibraryContainerView: View { service: service, client: client, tracker: tracker, - uploader: uploader + uploader: uploader, + urlOpener: urlOpener, + shareService: shareService, + navigator: navigator, + capabilities: capabilities ), service: service ) diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift index dce03bb7bd71..3ddd7a429cd3 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift @@ -43,9 +43,20 @@ struct MediaLibraryView: View { isPresentingUploads = true } } - MediaGridView(items: viewModel.displayItems, isAspectRatioMode: isAspectRatioMode) - .refreshable { await viewModel.refresh() } - .overlay { libraryOverlay } + // 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 } } } else { MediaLibrarySearchView( @@ -76,12 +87,11 @@ struct MediaLibraryView: View { filterMenu addMenu } - // `MediaLibraryView` is hosted in a UIKit `UINavigationController` - // via `UIHostingController`, so there's no SwiftUI `NavigationStack` - // ancestor for `.navigationDestination` to push into. Present the - // Uploads queue as a sheet instead — it's a self-contained - // management surface (its own toolbar + bulk menu) and survives - // the SwiftUI/UIKit boundary cleanly. + // Present the Uploads queue as a sheet rather than a push. It's a + // self-contained management surface (its own toolbar + bulk menu), and + // modal presentation keeps it reachable from any point in the detail + // navigation stack without the cell-tap push and the Uploads push + // fighting over the same back stack. .sheet(isPresented: $isPresentingUploads) { NavigationStack { UploadsView(viewModel: viewModel) @@ -153,6 +163,16 @@ struct MediaLibraryView: View { ) } + 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 + // cell rendering and the user's tap. + guard let detailVM = viewModel.makeDetailVM(for: item) else { return } + let host = UIHostingController(rootView: MediaDetailView(viewModel: detailVM)) + host.navigationItem.largeTitleDisplayMode = .never + viewModel.detailNavigator?.push(host) + } + @ToolbarContentBuilder private var filterMenu: some ToolbarContent { if searchText.isEmpty { ToolbarItem(placement: .topBarTrailing) { diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift index 67d0e30aa2dc..050023b6bfbf 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift @@ -5,6 +5,25 @@ import WordPressAPI import WordPressAPIInternal 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 let supportsAltEditing: Bool + public let supportsMetadataEditing: Bool + public let supportsDeletion: Bool + + public init( + supportsAltEditing: Bool, + supportsMetadataEditing: Bool, + supportsDeletion: Bool + ) { + self.supportsAltEditing = supportsAltEditing + self.supportsMetadataEditing = supportsMetadataEditing + self.supportsDeletion = supportsDeletion + } +} + /// Backs a single media grid: the library (no query) or one search query. /// Owns exactly one collection. The library instance also drives the /// client-side `kind` filter; the search instance leaves `kind` nil, so its @@ -17,6 +36,15 @@ final class MediaLibraryViewModel: ObservableObject { private let client: WordPressClient private let collection: Collection let uploader: MediaUploader? + let urlOpener: (any MediaDetailURLOpener)? + let shareService: (any MediaDetailShareService)? + let detailNavigator: (any MediaDetailNavigator)? + let detailCapabilities: MediaLibraryCapabilities? + + /// Caches the most-recent `MediaWithEditContext` per item id so + /// `makeDetailVM(for:)` can hand the detail screen a fully-resolved + /// payload without re-fetching. Rebuilt on every `loadItems` snapshot. + private var resolvedMediaByID: [Int64: MediaWithEditContext] = [:] @Published private(set) var bannerSummary: BannerSummary? @Published private(set) var uploadsScreenItems: [UploadRowItem] = [] @@ -81,14 +109,19 @@ final class MediaLibraryViewModel: ObservableObject { /// Builds the collection from the wordpress-rs service: the library when /// `search` is nil, a search collection otherwise. `client` is retained so /// `observe()` can subscribe to the local cache's update stream. The - /// `uploader` is wired only for the library instance; search instances - /// leave it nil and never surface the upload banner or queue. + /// `uploader` and detail wiring are passed only for the library instance; + /// search instances leave them nil and never surface the upload banner, + /// the queue, or the cell-tap detail push. init( service: WpService, client: WordPressClient, tracker: any MediaTracker, search: String? = nil, - uploader: MediaUploader? = nil + uploader: MediaUploader? = nil, + urlOpener: (any MediaDetailURLOpener)? = nil, + shareService: (any MediaDetailShareService)? = nil, + navigator: (any MediaDetailNavigator)? = nil, + capabilities: MediaLibraryCapabilities? = nil ) { self.tracker = tracker self.client = client @@ -98,6 +131,10 @@ final class MediaLibraryViewModel: ObservableObject { perPage: 100 ) self.uploader = uploader + self.urlOpener = urlOpener + self.shareService = shareService + self.detailNavigator = navigator + self.detailCapabilities = capabilities startUploaderObserver() } @@ -275,6 +312,14 @@ final class MediaLibraryViewModel: ObservableObject { do { let metadataItems = try await collection.loadItems() guard !Task.isCancelled else { return } + // Rebuild the resolved-media side store from this batch so + // `makeDetailVM(for:)` can hand the detail screen a fully- + // hydrated payload without re-fetching. + var resolved: [Int64: MediaWithEditContext] = [:] + for item in metadataItems { + if let media = item.resolvedMedia { resolved[item.id] = media } + } + self.resolvedMediaByID = resolved withAnimation { items = metadataItems.map(MediaGridItem.init(item:)) displayItems = Self.applyingKindFilter(items, kind: kind) @@ -285,6 +330,36 @@ final class MediaLibraryViewModel: ObservableObject { } } } + + // MARK: Detail navigation + + /// 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 + } + + /// 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). + func makeDetailVM(for item: MediaGridItem) -> MediaDetailViewModel? { + guard let urlOpener, + let shareService, + let detailNavigator, + let detailCapabilities, + let media = resolvedMediaByID[item.id] + else { return nil } + return MediaDetailViewModel( + media: media, + client: client, + tracker: tracker, + urlOpener: urlOpener, + shareService: shareService, + navigator: detailNavigator, + capabilities: detailCapabilities + ) + } } extension MediaLibraryViewModel: ExternalMediaPickerDelegate { diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift index d76b771b7591..3e161605be3c 100644 --- a/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift @@ -49,4 +49,15 @@ struct MediaKindTests { let details = DocumentMediaDetails(fileSize: 0) #expect(MediaKind(payload: .document(details)) == .document) } + + @Test func fromMimeTypeClassifiesByPrefix() { + #expect(MediaKind.from(mimeType: "image/jpeg") == .image) + #expect(MediaKind.from(mimeType: "image/png") == .image) + #expect(MediaKind.from(mimeType: "video/mp4") == .video) + #expect(MediaKind.from(mimeType: "video/videopress") == .video) + #expect(MediaKind.from(mimeType: "audio/mpeg") == .audio) + #expect(MediaKind.from(mimeType: "application/pdf") == .document) + #expect(MediaKind.from(mimeType: "text/plain") == .document) + #expect(MediaKind.from(mimeType: "") == .document) + } } diff --git a/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift b/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift index fbc7eef4d382..f2cba600995d 100644 --- a/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift +++ b/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift @@ -37,6 +37,26 @@ struct MediaTrackerAdapter: MediaTracker { case .mediaLibraryUploadRetried: stat = .mediaLibraryUploadMediaRetried + + case .mediaLibraryPreviewedItem: + stat = .mediaLibraryPreviewedItem + + case .mediaLibraryEditedItemMetadata: + stat = .mediaLibraryEditedItemMetadata + + case .mediaLibraryDeletedItems(let count): + stat = .mediaLibraryDeletedItems + properties["number_of_items_deleted"] = count + + case .mediaLibrarySharedItemLink: + stat = .mediaLibrarySharedItemLink + + case .siteMediaShareTapped(let count): + // V1 surface — WPAnalyticsEvent, not WPAnalyticsStat. + var shareProperties = properties + shareProperties["number_of_items"] = count + WPAnalytics.track(.siteMediaShareTapped, properties: shareProperties) + return } WPAppAnalytics.track(stat, properties: properties, blog: blog) diff --git a/WordPress/Classes/ViewRelated/Media/MediaLibraryRouting.swift b/WordPress/Classes/ViewRelated/Media/MediaLibraryRouting.swift index fb1b31e49cb7..eb2832d1289e 100644 --- a/WordPress/Classes/ViewRelated/Media/MediaLibraryRouting.swift +++ b/WordPress/Classes/ViewRelated/Media/MediaLibraryRouting.swift @@ -38,12 +38,28 @@ enum MediaLibraryRouting { return nil } - return MediaLibraryHostingController.make( + let urlOpener = MediaDetailURLOpenerAdapter(blog: blog) + let shareService = MediaDetailShareServiceAdapter(blog: blog) + let navigator = MediaDetailNavigatorAdapter() + let capabilities = MediaLibraryCapabilities( + supportsAltEditing: blog.supports(.mediaAltEditing), + supportsMetadataEditing: blog.supports(.mediaMetadataEditing), + supportsDeletion: blog.supports(.mediaDeletion) + ) + + let hostingController = MediaLibraryHostingController.make( client: client, tracker: tracker, uploader: uploader, + urlOpener: urlOpener, + shareService: shareService, + navigator: navigator, + capabilities: capabilities, externalPickerOptions: externalPickerOptions(for: blog) ) + urlOpener.attach(host: hostingController) + navigator.attach(host: hostingController) + return hostingController } /// Internal helper so tests can assert the option array without UI introspection. diff --git a/WordPress/Classes/ViewRelated/Media/V2/MediaDetailNavigatorAdapter.swift b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailNavigatorAdapter.swift new file mode 100644 index 000000000000..490facd5b736 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailNavigatorAdapter.swift @@ -0,0 +1,23 @@ +import UIKit +import WordPressMediaLibrary + +/// App-target conformer for `MediaDetailNavigator`. Pushes view controllers +/// the module hands it (already wrapped in `UIHostingController`) onto the +/// host's outer `UINavigationController`. Holds the host weakly; production +/// routing constructs the adapter, builds the hosting controller, then +/// calls `attach(host:)` to close the loop — same pattern as +/// `MediaDetailURLOpenerAdapter`. +@MainActor +final class MediaDetailNavigatorAdapter: MediaDetailNavigator { + private weak var host: UIViewController? + + init() {} + + func attach(host: UIViewController) { + self.host = host + } + + func push(_ viewController: UIViewController) { + host?.navigationController?.pushViewController(viewController, animated: true) + } +} diff --git a/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift new file mode 100644 index 000000000000..f2904431388d --- /dev/null +++ b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift @@ -0,0 +1,63 @@ +import Foundation +import UniformTypeIdentifiers +import WordPressData +import WordPressMediaLibrary + +@MainActor +final class MediaDetailShareServiceAdapter: MediaDetailShareService { + private let blog: Blog + private let authenticator: MediaRequestAuthenticator + + init( + blog: Blog, + authenticator: MediaRequestAuthenticator = MediaRequestAuthenticator() + ) { + self.blog = blog + self.authenticator = authenticator + } + + func downloadForSharing(items: [DownloadableMediaItem]) async throws -> [URL] { + 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) + } + let url = try moveDownloadedFile(at: downloadedURL, item: item) + result.append(url) + } + return result + } + + 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 + } + + /// Filename derivation (design § Filename derivation): + /// 1. Start with `suggestedFilename ?? sourceUrl.lastPathComponent` + /// 2. Sanitize: trim whitespace, replace `/` with `-`, truncate to 200 chars. + /// 3. If no extension, derive from `mimeType` via + /// `UTType(mimeType:).preferredFilenameExtension`. + static func resolveFilename(for item: DownloadableMediaItem) -> String { + var name = item.suggestedFilename ?? item.sourceUrl.lastPathComponent + name = name.trimmingCharacters(in: .whitespacesAndNewlines) + name = name.replacingOccurrences(of: "/", with: "-") + if name.count > 200 { name = String(name.prefix(200)) } + let ext = (name as NSString).pathExtension + if ext.isEmpty, let mime = item.mimeType, let resolved = UTType(mimeType: mime)?.preferredFilenameExtension { + name = "\(name).\(resolved)" + } + return name + } +} diff --git a/WordPress/Classes/ViewRelated/Media/V2/MediaDetailURLOpenerAdapter.swift b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailURLOpenerAdapter.swift new file mode 100644 index 000000000000..b757330fcc17 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailURLOpenerAdapter.swift @@ -0,0 +1,28 @@ +import UIKit +import WordPressData +import WordPressMediaLibrary + +@MainActor +final class MediaDetailURLOpenerAdapter: MediaDetailURLOpener { + private let blog: Blog + private weak var host: UIViewController? + + init(blog: Blog) { + self.blog = blog + } + + /// Attach the hosting controller AFTER it's been constructed. The host + /// is held weakly so reference cycles are avoided. Production routing + /// constructs the adapter, builds the hosting controller (which captures + /// the adapter), then calls this to close the loop. + func attach(host: UIViewController) { + self.host = host + } + + func open(_ url: URL, mediaTitle: String?) { + let controller = WebViewControllerFactory.controller(url: url, blog: blog, source: "media_item") + controller.loadViewIfNeeded() + controller.title = mediaTitle ?? "" + host?.navigationController?.pushViewController(controller, animated: true) + } +} From 9060f23915dffc37a1134a01d5e978cd9bbe49fc Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sat, 4 Jul 2026 19:21:52 +1200 Subject: [PATCH 2/4] Wire detail navigation into media search results Search result cells were inert because the search view model was built without the detail dependencies, while V1 opened the detail screen from search. Thread the existing optional dependencies through the search views; the pushed detail screen owns its view model and lives on the outer UIKit nav stack, so per-query view model teardown cannot strand it. --- .../Views/MediaLibrarySearchView.swift | 55 +++++++++++++++---- .../Views/MediaLibraryView.swift | 6 +- .../Views/MediaLibraryViewModel.swift | 6 +- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift index e504790b7e1f..192c69eebd78 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibrarySearchView.swift @@ -1,4 +1,5 @@ import SwiftUI +import UIKit import WordPressAPI import WordPressAPIInternal import WordPressCore @@ -13,6 +14,10 @@ struct MediaLibrarySearchView: View { let tracker: any MediaTracker @Binding var searchText: String let isAspectRatioMode: Bool + let urlOpener: (any MediaDetailURLOpener)? + let shareService: (any MediaDetailShareService)? + let navigator: (any MediaDetailNavigator)? + let capabilities: MediaLibraryCapabilities? /// The debounced, committed query. Empty until the first debounce fires. @State private var query = "" @@ -27,7 +32,11 @@ struct MediaLibrarySearchView: View { service: service, client: client, tracker: tracker, - isAspectRatioMode: isAspectRatioMode + isAspectRatioMode: isAspectRatioMode, + urlOpener: urlOpener, + shareService: shareService, + navigator: navigator, + capabilities: capabilities ) .id(query) } @@ -61,7 +70,11 @@ private struct MediaSearchResultsView: View { service: WpService, client: WordPressClient, tracker: any MediaTracker, - isAspectRatioMode: Bool + isAspectRatioMode: Bool, + urlOpener: (any MediaDetailURLOpener)?, + shareService: (any MediaDetailShareService)?, + navigator: (any MediaDetailNavigator)?, + capabilities: MediaLibraryCapabilities? ) { self.query = query self.tracker = tracker @@ -71,20 +84,40 @@ private struct MediaSearchResultsView: View { service: service, client: client, tracker: tracker, - search: query + search: query, + urlOpener: urlOpener, + shareService: shareService, + navigator: navigator, + capabilities: capabilities ) ) } var body: some View { - MediaGridView(items: viewModel.displayItems, isAspectRatioMode: isAspectRatioMode) - .refreshable { await viewModel.refresh() } - .task { - tracker.track(.mediaLibrarySearched(queryLength: query.count)) - await viewModel.load() - } - .task { await viewModel.observe() } - .overlay { overlay } + MediaGridView( + items: viewModel.displayItems, + isAspectRatioMode: isAspectRatioMode, + canSelect: { viewModel.canOpenDetail(for: $0) }, + onSelect: { pushDetail(for: $0) } + ) + .refreshable { await viewModel.refresh() } + .task { + tracker.track(.mediaLibrarySearched(queryLength: query.count)) + await viewModel.load() + } + .task { await viewModel.observe() } + .overlay { overlay } + } + + /// Same UIKit-bridge push as `MediaLibraryView.pushDetail`. The pushed + /// detail screen owns its view model and lives on the outer UIKit nav + /// stack, so a query change tearing down this view's `@StateObject` + /// cannot strand it. + private func pushDetail(for item: MediaGridItem) { + guard let detailVM = viewModel.makeDetailVM(for: item) else { return } + let host = UIHostingController(rootView: MediaDetailView(viewModel: detailVM)) + host.navigationItem.largeTitleDisplayMode = .never + viewModel.detailNavigator?.push(host) } @ViewBuilder private var overlay: some View { diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift index 3ddd7a429cd3..0883db1f5a52 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift @@ -64,7 +64,11 @@ struct MediaLibraryView: View { client: client, tracker: tracker, searchText: $searchText, - isAspectRatioMode: isAspectRatioMode + isAspectRatioMode: isAspectRatioMode, + urlOpener: viewModel.urlOpener, + shareService: viewModel.shareService, + navigator: viewModel.detailNavigator, + capabilities: viewModel.detailCapabilities ) } } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift index 050023b6bfbf..a89b81a3a1a4 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift @@ -109,9 +109,9 @@ final class MediaLibraryViewModel: ObservableObject { /// Builds the collection from the wordpress-rs service: the library when /// `search` is nil, a search collection otherwise. `client` is retained so /// `observe()` can subscribe to the local cache's update stream. The - /// `uploader` and detail wiring are passed only for the library instance; - /// search instances leave them nil and never surface the upload banner, - /// the queue, or the cell-tap detail push. + /// `uploader` is wired only for the library instance; search instances + /// leave it nil and never surface the upload banner or queue. The detail + /// wiring is passed for both, so search results can push detail too. init( service: WpService, client: WordPressClient, From 00b1e35fbbfc3bef4e215978301ad3cc49c98d55 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sat, 4 Jul 2026 19:23:53 +1200 Subject: [PATCH 3/4] Validate share filename extensions against the MIME type A dot segment in a media title ("Logo v2.0") was treated as a file extension, so the MIME-derived extension was never appended and share targets misidentified the file. Extensions now count only when UTType recognizes them and they agree with the MIME type; truncation applies to the stem so a real extension survives long names. --- ...tailShareServiceAdapterFilenameTests.swift | 67 +++++++++++++++++++ .../V2/MediaDetailShareServiceAdapter.swift | 54 ++++++++++++--- 2 files changed, 112 insertions(+), 9 deletions(-) create mode 100644 Tests/KeystoneTests/Tests/Features/Media/V2/MediaDetailShareServiceAdapterFilenameTests.swift diff --git a/Tests/KeystoneTests/Tests/Features/Media/V2/MediaDetailShareServiceAdapterFilenameTests.swift b/Tests/KeystoneTests/Tests/Features/Media/V2/MediaDetailShareServiceAdapterFilenameTests.swift new file mode 100644 index 000000000000..f80393778023 --- /dev/null +++ b/Tests/KeystoneTests/Tests/Features/Media/V2/MediaDetailShareServiceAdapterFilenameTests.swift @@ -0,0 +1,67 @@ +import Testing +import UniformTypeIdentifiers +import WordPressMediaLibrary +@testable import WordPress + +@MainActor +struct MediaDetailShareServiceAdapterFilenameTests { + + private func resolve( + suggested: String?, + mimeType: String?, + sourceUrl: String = "https://example.com/wp-content/uploads/photo.png" + ) -> String { + let item = DownloadableMediaItem( + sourceUrl: URL(string: sourceUrl)!, + mimeType: mimeType, + suggestedFilename: suggested + ) + return MediaDetailShareServiceAdapter.resolveFilename(for: item) + } + + @Test func dotSegmentTitleGetsMimeExtension() { + #expect(resolve(suggested: "Logo v2.0", mimeType: "image/png") == "Logo v2.0.png") + } + + @Test func matchingExtensionIsKept() { + #expect(resolve(suggested: "photo.jpg", mimeType: "image/jpeg") == "photo.jpg") + #expect(resolve(suggested: "photo.jpeg", mimeType: "image/jpeg") == "photo.jpeg") + #expect(resolve(suggested: "PHOTO.JPG", mimeType: "image/jpeg") == "PHOTO.JPG") + } + + @Test func missingExtensionDerivedFromMimeType() { + #expect(resolve(suggested: "vacation", mimeType: "image/png") == "vacation.png") + } + + @Test func missingExtensionFallsBackToSourceURL() { + #expect(resolve(suggested: "vacation", mimeType: nil) == "vacation.png") + } + + @Test func dotSegmentTitleWithoutMimeTypeUsesURLExtension() { + #expect(resolve(suggested: "Logo v2.0", mimeType: nil) == "Logo v2.0.png") + } + + @Test func mismatchedRealExtensionIsRetyped() { + #expect(resolve(suggested: "notes.txt", mimeType: "image/png") == "notes.txt.png") + } + + @Test func genericMimeTypeKeepsRealExtension() { + #expect(resolve(suggested: "report.pdf", mimeType: "application/octet-stream") == "report.pdf") + } + + @Test func longNameTruncatesStemAndKeepsExtension() { + let longStem = String(repeating: "a", count: 250) + let resolved = resolve(suggested: "\(longStem).jpg", mimeType: "image/jpeg") + #expect(resolved == "\(String(repeating: "a", count: 200)).jpg") + } + + @Test func unusableNamesFallBackToMedia() { + #expect(resolve(suggested: "", mimeType: "image/png") == "media.png") + #expect(resolve(suggested: ".", mimeType: "image/png") == "media.png") + #expect(resolve(suggested: "..", mimeType: "image/png") == "media.png") + } + + @Test func nilSuggestionUsesURLFilename() { + #expect(resolve(suggested: nil, mimeType: "image/png") == "photo.png") + } +} diff --git a/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift index f2904431388d..d9e1ecaa4d00 100644 --- a/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift +++ b/WordPress/Classes/ViewRelated/Media/V2/MediaDetailShareServiceAdapter.swift @@ -45,19 +45,55 @@ final class MediaDetailShareServiceAdapter: MediaDetailShareService { } /// Filename derivation (design § Filename derivation): - /// 1. Start with `suggestedFilename ?? sourceUrl.lastPathComponent` - /// 2. Sanitize: trim whitespace, replace `/` with `-`, truncate to 200 chars. - /// 3. If no extension, derive from `mimeType` via - /// `UTType(mimeType:).preferredFilenameExtension`. + /// 1. Start with `suggestedFilename ?? sourceUrl.lastPathComponent`, + /// trimmed, with `/` replaced by `-`; empty or dot-only names fall + /// back to "media". + /// 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. + /// 3. Without a valid extension, derive one from `mimeType` via + /// `UTType(mimeType:).preferredFilenameExtension`, falling back to + /// the source URL's extension. + /// 4. Truncate the stem to 200 characters, keeping the extension. static func resolveFilename(for item: DownloadableMediaItem) -> String { var name = item.suggestedFilename ?? item.sourceUrl.lastPathComponent name = name.trimmingCharacters(in: .whitespacesAndNewlines) name = name.replacingOccurrences(of: "/", with: "-") - if name.count > 200 { name = String(name.prefix(200)) } - let ext = (name as NSString).pathExtension - if ext.isEmpty, let mime = item.mimeType, let resolved = UTType(mimeType: mime)?.preferredFilenameExtension { - name = "\(name).\(resolved)" + if name.isEmpty || name == "." || name == ".." { + name = "media" } - return name + + var stem = (name as NSString).deletingPathExtension + var ext = (name as NSString).pathExtension + if !isValidFilenameExtension(ext, forMimeType: item.mimeType) { + stem = name + ext = "" + } + if ext.isEmpty { + if let mime = item.mimeType, let resolved = UTType(mimeType: mime)?.preferredFilenameExtension { + ext = resolved + } else { + ext = item.sourceUrl.pathExtension + } + } + if stem.count > 200 { + stem = String(stem.prefix(200)) + } + return ext.isEmpty ? stem : "\(stem).\(ext)" + } + + /// An extension is usable only when UTType recognizes it (unknown + /// extensions produce dynamic `dyn.*` types, e.g. the "0" in + /// "Logo v2.0") and it doesn't contradict a known MIME type. The + /// reverse conformance check keeps real extensions under generic + /// server MIME types like `application/octet-stream`. + private static func isValidFilenameExtension(_ ext: String, forMimeType mimeType: String?) -> Bool { + guard !ext.isEmpty, let extType = UTType(filenameExtension: ext.lowercased()), !extType.isDynamic else { + return false + } + guard let mimeType, let mimeUTType = UTType(mimeType: mimeType), !mimeUTType.isDynamic else { + return true + } + return extType.conforms(to: mimeUTType) || mimeUTType.conforms(to: extType) } } From 3714c1302272dff488d33d5a116d1836f48b3532 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sat, 4 Jul 2026 19:25:19 +1200 Subject: [PATCH 4/4] Show progress and allow cancelling the detail share Preparing a share previously disabled the whole screen with no feedback, and a slow download could not be stopped. The share toolbar slot now shows a spinner that cancels the download when tapped, leaving the screen cancels it too, and cancellation resolves silently instead of surfacing an error alert. --- .../Strings/Strings.swift | 5 ++++ .../Views/Detail/MediaDetailView.swift | 25 ++++++++++++++---- .../Views/Detail/MediaDetailViewModel.swift | 26 +++++++++++++++++-- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift index 2b86c0826fda..acf7055284cb 100644 --- a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift +++ b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift @@ -385,6 +385,11 @@ enum Strings { value: "Source URL is invalid.", comment: "Error shown when share fails due to an invalid source URL." ) + static let detailShareCancelAccessibility = NSLocalizedString( + "mediaLibrary.detail.share.cancelAccessibility", + value: "Cancel share", + comment: "Accessibility label for the in-progress share spinner; tapping it cancels the share download." + ) static let detailPreviewImageAccessibility = NSLocalizedString( "mediaLibrary.detail.preview.imageAccessibility", diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift index ad8a32bae55e..2e6433ca1b85 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailView.swift @@ -64,6 +64,10 @@ struct MediaDetailView: View { if shouldPop { dismiss() } } .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() } } @ViewBuilder private var editableFieldsSection: some View { @@ -155,12 +159,23 @@ struct MediaDetailView: View { } } ToolbarItem(placement: .topBarTrailing) { - Button { - Task { await viewModel.share() } - } label: { - Image(systemName: "square.and.arrow.up").accessibilityLabel(Strings.commonShare) + if viewModel.isSharing { + // The spinner replaces the share button while the download + // prepares; tapping it cancels the share. + Button { + viewModel.cancelShare() + } label: { + ProgressView() + } + .accessibilityLabel(Strings.detailShareCancelAccessibility) + } else { + Button { + viewModel.share() + } label: { + Image(systemName: "square.and.arrow.up").accessibilityLabel(Strings.commonShare) + } + .disabled(!viewModel.isShareEnabled) } - .disabled(!viewModel.isShareEnabled) } } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift index 9d1e3dae2c71..5375c62b8e86 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/Detail/MediaDetailViewModel.swift @@ -29,6 +29,7 @@ final class MediaDetailViewModel: ObservableObject { private var didFireOpen = false private var inFlightSaveTask: [MediaEditableField: Task] = [:] + private var shareTask: Task? struct SharePayload: Identifiable { let id = UUID() @@ -176,8 +177,8 @@ final class MediaDetailViewModel: ObservableObject { !isAnyOperationInFlight && !display.sourceUrl.isEmpty } - func share() async { - guard isShareEnabled else { return } + func share() { + guard isShareEnabled, shareTask == nil else { return } isSharing = true tracker.track(.siteMediaShareTapped(count: 1)) guard let item = makeShareItem() else { @@ -185,10 +186,31 @@ final class MediaDetailViewModel: ObservableObject { shareErrorMessage = Strings.detailShareErrorInvalidURL return } + shareTask = Task { [weak self] in + guard let self else { return } + await self.performShare(item: item) + self.shareTask = nil + } + } + + /// Cancels the in-flight share download, if any. Wired to the progress + /// spinner in the toolbar and to the screen's disappearance. + func cancelShare() { + shareTask?.cancel() + } + + private func performShare(item: DownloadableMediaItem) async { do { let urls = try await shareService.downloadForSharing(items: [item]) + try Task.checkCancellation() isSharing = false sharePayload = SharePayload(urls: urls) + } catch is CancellationError { + // User-initiated cancellation is not an error. + isSharing = false + } catch let error as URLError where error.code == .cancelled { + // URLSession surfaces task cancellation as URLError(.cancelled). + isSharing = false } catch { Loggers.mediaLibrary.error("Media share failed for id \(display.id): \(error)") isSharing = false