From cc999b471d5b640ce3794408aca3f05b8540f26c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:34:13 +1200 Subject: [PATCH 01/21] Register WordPressMediaLibraryTests target --- Modules/Package.swift | 7 +++++++ Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift | 3 +++ 2 files changed, 10 insertions(+) create mode 100644 Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift diff --git a/Modules/Package.swift b/Modules/Package.swift index 1fe2da97bfde..ce5bc379a259 100644 --- a/Modules/Package.swift +++ b/Modules/Package.swift @@ -145,6 +145,13 @@ let package = Package( .product(name: "Logging", package: "swift-log") ] ), + .testTarget( + name: "WordPressMediaLibraryTests", + dependencies: [ + .target(name: "WordPressMediaLibrary"), + .product(name: "WordPressAPI", package: "wordpress-rs") + ] + ), .target( name: "ShareExtensionCore", dependencies: [ diff --git a/Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift b/Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift new file mode 100644 index 000000000000..c7252505171e --- /dev/null +++ b/Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift @@ -0,0 +1,3 @@ +// Placeholder so SwiftPM accepts the test target before the first real +// test file lands. Deleted by Task 6 when MediaKindTests.swift is added. +import Foundation From ed4dcaa73ba2b28319320ef8ae6496c1bd18c152 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:36:12 +1200 Subject: [PATCH 02/21] Add WPAnalyticsStat cases for Media Library V2 filter/search/grid-toggle --- Modules/Sources/WordPressSharedObjC/include/WPAnalytics.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Modules/Sources/WordPressSharedObjC/include/WPAnalytics.h b/Modules/Sources/WordPressSharedObjC/include/WPAnalytics.h index 4d81781bc57e..081b7a1bde48 100644 --- a/Modules/Sources/WordPressSharedObjC/include/WPAnalytics.h +++ b/Modules/Sources/WordPressSharedObjC/include/WPAnalytics.h @@ -227,6 +227,9 @@ typedef NS_ENUM(NSUInteger, WPAnalyticsStat) { WPAnalyticsStatMediaLibraryAddedVideoViaCamera, WPAnalyticsStatMediaLibraryAddedVideoViaOtherApps, WPAnalyticsStatMediaLibraryUploadMediaRetried, + WPAnalyticsStatSiteMediaFilterChanged, + WPAnalyticsStatSiteMediaGridModeToggled, + WPAnalyticsStatSiteMediaSearched, WPAnalyticsStatMediaServiceUploadStarted, WPAnalyticsStatMediaServiceUploadFailed, WPAnalyticsStatMediaServiceUploadSuccessful, From bd9c6c4f5bacc2b47e8bd67b13cff9aafdd0b11c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:37:49 +1200 Subject: [PATCH 03/21] Format TracksMappedEvent.swift before M2 mappings --- .../Utility/Analytics/TracksMappedEvent.swift | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift b/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift index 7faecfc99a1e..b2bb333ce025 100644 --- a/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift +++ b/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift @@ -1237,16 +1237,16 @@ extension TracksMappedEvent { case .welcomeNoSitesInterstitialDismissed: name = "welcome_no_sites_interstitial_dismissed" - // The following are yet to be implemented. - // - // If you get test failures in AnalyticsTrackerAutomatticTracks, it's most likely - // because there are new . enum values. This can mean that somebody is - // currently working on it. In cases like this, add the enum values here, returning - // as `nil`. The tests should pass. + // The following are yet to be implemented. + // + // If you get test failures in AnalyticsTrackerAutomatticTracks, it's most likely + // because there are new . enum values. This can mean that somebody is + // currently working on it. In cases like this, add the enum values here, returning + // as `nil`. The tests should pass. case .defaultAccountChanged, - .noStat, - .performedCoreDataMigrationFixFor45, - .maxValue: + .noStat, + .performedCoreDataMigrationFixFor45, + .maxValue: return nil @unknown default: return nil From 59210bb92ff67e691bdedc4a972de6ae6260bedf Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:38:50 +1200 Subject: [PATCH 04/21] Map M2 Media Library stats to Tracks event names --- WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift b/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift index b2bb333ce025..c5d8f6763da5 100644 --- a/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift +++ b/WordPress/Classes/Utility/Analytics/TracksMappedEvent.swift @@ -960,6 +960,12 @@ extension TracksMappedEvent { name = "signup_magic_link_requested" case .signupTermsButtonTapped: name = "signup_terms_of_service_tapped" + case .siteMediaFilterChanged: + name = "site_media_filter_changed" + case .siteMediaGridModeToggled: + name = "site_media_grid_mode_toggled" + case .siteMediaSearched: + name = "site_media_searched" case .siteSettingsSiteIconTapped: name = "my_site_icon_tapped" case .siteSettingsSiteIconRemoved: From fe8e882552829c87af5f9b136bfd062f6626e831 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:50:22 +1200 Subject: [PATCH 05/21] Add WordPressMediaLibraryTests to the WordPress test plan --- Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift | 3 --- Tests/KeystoneTests/WordPressUnitTests.xctestplan | 7 +++++++ 2 files changed, 7 insertions(+), 3 deletions(-) delete mode 100644 Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift diff --git a/Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift b/Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift deleted file mode 100644 index c7252505171e..000000000000 --- a/Modules/Tests/WordPressMediaLibraryTests/Placeholder.swift +++ /dev/null @@ -1,3 +0,0 @@ -// Placeholder so SwiftPM accepts the test target before the first real -// test file lands. Deleted by Task 6 when MediaKindTests.swift is added. -import Foundation diff --git a/Tests/KeystoneTests/WordPressUnitTests.xctestplan b/Tests/KeystoneTests/WordPressUnitTests.xctestplan index 897d693fb166..301b4c9017a8 100644 --- a/Tests/KeystoneTests/WordPressUnitTests.xctestplan +++ b/Tests/KeystoneTests/WordPressUnitTests.xctestplan @@ -115,6 +115,13 @@ "name" : "WordPressFluxTests" } }, + { + "target" : { + "containerPath" : "container:..\/Modules", + "identifier" : "WordPressMediaLibraryTests", + "name" : "WordPressMediaLibraryTests" + } + }, { "target" : { "containerPath" : "container:WordPress.xcodeproj", From cad76df343e661ed92146f746e3cd55fda9a5123 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:50:22 +1200 Subject: [PATCH 06/21] Add MediaKind enum mapping MediaDetailsPayload to image/video/audio/document --- .../Models/MediaKind.swift | 33 +++++++++++ .../MediaKindTests.swift | 59 +++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift create mode 100644 Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift new file mode 100644 index 000000000000..6ef8f33619b0 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift @@ -0,0 +1,33 @@ +import Foundation +import WordPressAPI +import WordPressAPIInternal + +/// The enum itself is public so `MediaTrackerEvent.mediaLibraryFilterChanged(kind:)` +/// can carry it across the module boundary; the app-target analytics +/// adapter reads `rawValue` for its property dict. The `MediaDetailsPayload` +/// initializer, the `MediaTypeParam` mapping, and the UI helpers +/// (`title` / `systemImageName`, added in Task 11) all stay module-internal +/// — they're used only inside the module and in `@testable` tests, so +/// there's no reason to leak `WordPressAPIInternal` types through the +/// public surface. +public enum MediaKind: String, CaseIterable, Hashable, Sendable { + case image, video, audio, document + + init?(payload: MediaDetailsPayload) { + switch payload { + case .image: self = .image + case .video: self = .video + case .audio: self = .audio + case .document: self = .document + } + } + + var asMediaTypeParam: MediaTypeParam { + switch self { + case .image: .image + case .video: .video + case .audio: .audio + case .document: .application + } + } +} diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift new file mode 100644 index 000000000000..c918dc86e8cb --- /dev/null +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift @@ -0,0 +1,59 @@ +import Testing +import WordPressAPI +import WordPressAPIInternal + +@testable import WordPressMediaLibrary + +struct MediaKindTests { + @Test func payloadImageMapsToImage() { + let details = ImageMediaDetails(fileSize: 0, width: 100, height: 100, file: "x.jpg", sizes: nil) + #expect(MediaKind(payload: .image(details)) == .image) + } + + @Test func payloadVideoMapsToVideo() { + let details = VideoMediaDetails( + fileSize: 0, + length: 0, + width: 0, + height: 0, + fileFormat: nil, + dataFormat: nil, + createdTimestamp: nil + ) + #expect(MediaKind(payload: .video(details)) == .video) + } + + @Test func payloadAudioMapsToAudio() { + // AudioMediaDetails has 13 init parameters per wp_api.swift:32603 — + // fileSize, length (UInt64), lengthFormatted, plus ten optional + // metadata fields. Pass minimal valid values. + let details = AudioMediaDetails( + fileSize: 0, + length: 0, + lengthFormatted: "", + dataFormat: nil, + codec: nil, + sampleRate: nil, + channels: nil, + bitsPerSample: nil, + lossless: nil, + channelMode: nil, + bitrate: nil, + compressionRatio: nil, + fileFormat: nil + ) + #expect(MediaKind(payload: .audio(details)) == .audio) + } + + @Test func payloadDocumentMapsToDocument() { + let details = DocumentMediaDetails(fileSize: 0) + #expect(MediaKind(payload: .document(details)) == .document) + } + + @Test func mediaTypeParamMappingCoversAllKinds() { + #expect(MediaKind.image.asMediaTypeParam == .image) + #expect(MediaKind.video.asMediaTypeParam == .video) + #expect(MediaKind.audio.asMediaTypeParam == .audio) + #expect(MediaKind.document.asMediaTypeParam == .application) + } +} From de0c4e90955e980eb9e963ffcc8ef8ee16f4e555 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:54:47 +1200 Subject: [PATCH 07/21] Add MediaThumbnailURL picker preferring medium size with sourceUrl fallback --- .../Models/MediaThumbnailURL.swift | 21 ++++++ .../MediaThumbnailURLTests.swift | 75 +++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaThumbnailURL.swift create mode 100644 Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaThumbnailURL.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaThumbnailURL.swift new file mode 100644 index 000000000000..d66a7f23f1e2 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaThumbnailURL.swift @@ -0,0 +1,21 @@ +import Foundation +import WordPressAPI +import WordPressAPIInternal + +/// Picks a thumbnail URL from `ImageMediaDetails.sizes`, falling back through +/// a preference list and finally to `sourceUrl`. The 4-per-row phone grid +/// renders ~270px cells at @3x — `medium` (default 300px) is the closest +/// well-known size; `thumbnail` (150px) covers the case where only the small +/// image has been generated server-side. +enum MediaThumbnailURL { + private static let preferenceOrder = ["medium", "medium_large", "large", "thumbnail"] + + static func pick(from imageDetails: ImageMediaDetails, sourceUrl: String) -> URL? { + for key in preferenceOrder { + if let scaled = imageDetails.sizes?[key], let url = URL(string: scaled.sourceUrl) { + return url + } + } + return URL(string: sourceUrl) + } +} diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift new file mode 100644 index 000000000000..0df7075b39cc --- /dev/null +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift @@ -0,0 +1,75 @@ +import Testing +import WordPressAPI +import WordPressAPIInternal +@testable import WordPressMediaLibrary + +struct MediaThumbnailURLTests { + private func detailsWithSizes(_ sizes: [String: ScaledImageDetails]?) -> ImageMediaDetails { + ImageMediaDetails(fileSize: 0, width: 0, height: 0, file: "x.jpg", sizes: sizes) + } + + private func scaled(_ urlString: String) -> ScaledImageDetails { + // ScaledImageDetails init signature per wp_api.swift:65820 — + // file / width / height / sourceUrl (no mimeType). + ScaledImageDetails(file: "x", width: 0, height: 0, sourceUrl: urlString) + } + + @Test func picksMediumWhenPresent() { + let details = detailsWithSizes([ + "thumbnail": scaled("https://example.com/thumb.jpg"), + "medium": scaled("https://example.com/medium.jpg"), + "large": scaled("https://example.com/large.jpg") + ]) + let url = MediaThumbnailURL.pick(from: details, sourceUrl: "https://example.com/original.jpg") + #expect(url?.absoluteString == "https://example.com/medium.jpg") + } + + @Test func picksMediumLargeWhenMediumIsAbsent() { + let details = detailsWithSizes([ + "thumbnail": scaled("https://example.com/thumb.jpg"), + "medium_large": scaled("https://example.com/medium-large.jpg"), + "large": scaled("https://example.com/large.jpg") + ]) + let url = MediaThumbnailURL.pick(from: details, sourceUrl: "https://example.com/original.jpg") + #expect(url?.absoluteString == "https://example.com/medium-large.jpg") + } + + @Test func picksLargeWhenMediumAndMediumLargeAreAbsent() { + let details = detailsWithSizes([ + "thumbnail": scaled("https://example.com/thumb.jpg"), + "large": scaled("https://example.com/large.jpg") + ]) + let url = MediaThumbnailURL.pick(from: details, sourceUrl: "https://example.com/original.jpg") + #expect(url?.absoluteString == "https://example.com/large.jpg") + } + + @Test func picksThumbnailWhenItIsTheOnlyPreferredSize() { + let details = detailsWithSizes([ + "thumbnail": scaled("https://example.com/thumb.jpg") + ]) + let url = MediaThumbnailURL.pick(from: details, sourceUrl: "https://example.com/original.jpg") + #expect(url?.absoluteString == "https://example.com/thumb.jpg") + } + + @Test func fallsBackToSourceUrlWhenSizesIsNil() { + let details = detailsWithSizes(nil) + let url = MediaThumbnailURL.pick(from: details, sourceUrl: "https://example.com/original.jpg") + #expect(url?.absoluteString == "https://example.com/original.jpg") + } + + @Test func fallsBackToSourceUrlWhenNoPreferredKeyMatches() { + let details = detailsWithSizes([ + "custom-size": scaled("https://example.com/custom.jpg") + ]) + let url = MediaThumbnailURL.pick(from: details, sourceUrl: "https://example.com/original.jpg") + #expect(url?.absoluteString == "https://example.com/original.jpg") + } + + @Test func returnsNilWhenAllUrlsAreMalformed() { + let details = detailsWithSizes([ + "medium": scaled("") + ]) + let url = MediaThumbnailURL.pick(from: details, sourceUrl: "") + #expect(url == nil) + } +} From d0dbf6195854f232a97859bb3192c6e3b49ec6d8 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:56:03 +1200 Subject: [PATCH 08/21] Add MediaGridDuration formatter for m:ss and h:mm:ss video durations --- .../Models/MediaGridDuration.swift | 17 +++++++++++ .../MediaGridDurationTests.swift | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift create mode 100644 Modules/Tests/WordPressMediaLibraryTests/MediaGridDurationTests.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift new file mode 100644 index 000000000000..e1df99acb30b --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift @@ -0,0 +1,17 @@ +import Foundation + +/// Formats a video duration (seconds) as `m:ss` for durations under one hour +/// and `h:mm:ss` from one hour up. Matches the V1 reference at +/// `SiteMediaCollectionCellViewModel.swift`'s `makeString(forDuration:)`. +enum MediaGridDuration { + static func string(forSeconds seconds: UInt32) -> String { + let total = Int(seconds) + let hours = total / 3600 + let minutes = (total % 3600) / 60 + let secs = total % 60 + if hours > 0 { + return String(format: "%d:%02d:%02d", hours, minutes, secs) + } + return String(format: "%d:%02d", minutes, secs) + } +} diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaGridDurationTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaGridDurationTests.swift new file mode 100644 index 000000000000..f814f27bc8a7 --- /dev/null +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaGridDurationTests.swift @@ -0,0 +1,28 @@ +import Testing +@testable import WordPressMediaLibrary + +struct MediaGridDurationTests { + @Test func zeroSeconds() { + #expect(MediaGridDuration.string(forSeconds: 0) == "0:00") + } + + @Test func underOneMinute() { + #expect(MediaGridDuration.string(forSeconds: 7) == "0:07") + } + + @Test func ninetySeconds() { + #expect(MediaGridDuration.string(forSeconds: 90) == "1:30") + } + + @Test func justUnderOneHour() { + #expect(MediaGridDuration.string(forSeconds: 3599) == "59:59") + } + + @Test func exactlyOneHour() { + #expect(MediaGridDuration.string(forSeconds: 3600) == "1:00:00") + } + + @Test func overOneHour() { + #expect(MediaGridDuration.string(forSeconds: 3661) == "1:01:01") + } +} From ba969d78d2e76b2e4e395eab6609c88115dcfc8b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:57:18 +1200 Subject: [PATCH 09/21] Add MediaGridFilter as the Hashable identity driving .task(id:) collection rebuilds --- .../Models/MediaGridFilter.swift | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaGridFilter.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridFilter.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridFilter.swift new file mode 100644 index 000000000000..7ab08210be62 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridFilter.swift @@ -0,0 +1,33 @@ +import Foundation +import WordPressAPI +import WordPressAPIInternal + +/// Filter state for the Media Library grid. Hashable so it can drive +/// `.task(id: viewModel.filter)` — when any field changes, SwiftUI +/// cancels the outstanding refresh/observer tasks and re-runs them +/// against the freshly-rebuilt collection. +struct MediaGridFilter: Hashable { + var kind: MediaKind? // nil = all kinds + var search: String // empty = no constraint + + static let initial = MediaGridFilter(kind: nil, search: "") + + func with(kind: MediaKind?) -> Self { + var copy = self + copy.kind = kind + return copy + } + + func with(search: String) -> Self { + var copy = self + copy.search = search + return copy + } + + func asMediaListFilter() -> MediaListFilter { + MediaListFilter( + search: search.isEmpty ? nil : search, + mediaType: kind?.asMediaTypeParam + ) + } +} From fe32c0382cfcf15d42bb4813568b0735272321aa Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 13:58:29 +1200 Subject: [PATCH 10/21] Add AspectRatioPreference helper reusing the V1 UserDefaults key --- .../Preferences/AspectRatioPreference.swift | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Preferences/AspectRatioPreference.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Preferences/AspectRatioPreference.swift b/Modules/Sources/WordPressMediaLibrary/Preferences/AspectRatioPreference.swift new file mode 100644 index 000000000000..cc5eb57f77c8 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Preferences/AspectRatioPreference.swift @@ -0,0 +1,23 @@ +import Foundation +import UIKit +import WordPressShared + +/// Read/write the V1 `mediaAspectRatioModeEnabled` UserDefaults key from +/// inside the module. The constant key lives in `WordPressShared` +/// (`UPRUConstants.mediaAspectRatioModeEnabledKey`); the convenience getter +/// the V1 host uses is on the app target's `UserPersistentRepositoryUtility` +/// extension, which the module can't reach — so we re-implement it locally. +/// Default matches V1: `.pad` users default to aspect-ratio mode on, +/// `.phone` to off. +enum AspectRatioPreference { + private static let key = UPRUConstants.mediaAspectRatioModeEnabledKey + + static func load(defaults: UserDefaults = .standard) -> Bool { + if let value = defaults.object(forKey: key) as? Bool { return value } + return UIDevice.current.userInterfaceIdiom == .pad + } + + static func save(_ value: Bool, defaults: UserDefaults = .standard) { + defaults.set(value, forKey: key) + } +} From 0fc8fff3ee3aa3cea9ea80e046a5e64a5d718542 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:00:05 +1200 Subject: [PATCH 11/21] Add M2 grid strings and MediaKind UI helpers --- .../Models/MediaKind.swift | 26 ++++++ .../Strings/Strings.swift | 91 +++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift index 6ef8f33619b0..54a4b8bcbde6 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift @@ -31,3 +31,29 @@ public enum MediaKind: String, CaseIterable, Hashable, Sendable { } } } + +// MARK: - UI helpers +// +// These properties live in the same file as the enum but in their own +// extension so they're easy to spot and so the base enum (used by the +// public analytics surface) doesn't pull in localized strings unnecessarily. + +extension MediaKind { + var title: String { + switch self { + case .image: Strings.filterImages + case .video: Strings.filterVideos + case .audio: Strings.filterAudio + case .document: Strings.filterDocuments + } + } + + var systemImageName: String { + switch self { + case .image: "photo" + case .video: "video" + case .audio: "waveform" + case .document: "folder" + } + } +} diff --git a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift index 8cff5f5862fa..1422d5ea0d2b 100644 --- a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift +++ b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift @@ -1,6 +1,8 @@ import Foundation enum Strings { + // MARK: - M1 + static let title = NSLocalizedString( "mediaLibrary.screen.title", value: "Media", @@ -24,4 +26,93 @@ enum Strings { value: "(no title)", comment: "Placeholder shown for media items with no title" ) + + // MARK: - M2 + + static let searchPrompt = NSLocalizedString( + "mediaLibrary.search.prompt", + value: "Search media", + comment: "Prompt for the Media Library search field" + ) + + static let filterAll = NSLocalizedString( + "mediaLibrary.filter.all", + value: "All", + comment: "Title of the no-filter option in the Media Library filter menu" + ) + + static let filterImages = NSLocalizedString( + "mediaLibrary.filter.images", + value: "Images", + comment: "Title of the images filter option in the Media Library filter menu" + ) + + static let filterVideos = NSLocalizedString( + "mediaLibrary.filter.videos", + value: "Videos", + comment: "Title of the videos filter option in the Media Library filter menu" + ) + + static let filterDocuments = NSLocalizedString( + "mediaLibrary.filter.documents", + value: "Documents", + comment: "Title of the documents filter option in the Media Library filter menu" + ) + + static let filterAudio = NSLocalizedString( + "mediaLibrary.filter.audio", + value: "Audio", + comment: "Title of the audio filter option in the Media Library filter menu" + ) + + static let aspectRatioGrid = NSLocalizedString( + "mediaLibrary.gridMode.aspectRatio", + value: "Aspect Ratio Grid", + comment: "Menu option to switch the grid into aspect-ratio mode" + ) + + static let squareGrid = NSLocalizedString( + "mediaLibrary.gridMode.square", + value: "Square Grid", + comment: "Menu option to switch the grid into square (default) mode" + ) + + static let emptyFiltered = NSLocalizedString( + "mediaLibrary.empty.filtered", + value: "No media for this filter", + comment: "Message shown when the Media Library has items but none match the active filter" + ) + + // MARK: - Accessibility labels (V1 parity) + + static let accessibilityLabelImage = NSLocalizedString( + "mediaLibrary.accessibility.image", + value: "Image, %1$@", + comment: "Accessibility label for an image cell. %1$@ is the creation date." + ) + + static let accessibilityLabelVideo = NSLocalizedString( + "mediaLibrary.accessibility.video", + value: "Video, %1$@", + comment: "Accessibility label for a video cell. %1$@ is the creation date." + ) + + static let accessibilityLabelAudio = NSLocalizedString( + "mediaLibrary.accessibility.audio", + value: "Audio, %1$@", + comment: "Accessibility label for an audio cell. %1$@ is the creation date." + ) + + static let accessibilityLabelDocument = NSLocalizedString( + "mediaLibrary.accessibility.document", + value: "Document, %1$@", + comment: + "Accessibility label for a document cell. %1$@ is the filename, or the creation date if filename can't be derived." + ) + + static let accessibilityLoadingMedia = NSLocalizedString( + "mediaLibrary.accessibility.loading", + value: "Loading media", + comment: "Accessibility label for a cell that is still loading its data" + ) } From 15b8f889c2ab30f9a6d306a47e00cd3fefc9ee0a Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:01:07 +1200 Subject: [PATCH 12/21] Add MediaGridLayoutMath value type mirroring V1 cell-size math --- .../Models/MediaGridLayoutMath.swift | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift new file mode 100644 index 000000000000..76f16d800321 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift @@ -0,0 +1,28 @@ +import Foundation +import SwiftUI + +/// Pure layout math for the Media Library grid. Mirrors V1's +/// `SiteMediaCollectionViewController.updateFlowLayoutItemSize` at lines +/// 148-158: 4 cells per row below 500pt width, 5 otherwise; spacing 2 in +/// default mode, 8 in aspect-ratio mode; cell size rounded down to avoid +/// fractional pixels. +struct MediaGridLayoutMath { + let availableWidth: CGFloat + let isAspectRatioMode: Bool + + var spacing: CGFloat { isAspectRatioMode ? 8 : 2 } + + var itemsPerRow: Int { availableWidth < 500 ? 4 : 5 } + + var cellSize: CGFloat { + let raw = (availableWidth - spacing * CGFloat(itemsPerRow - 1)) / CGFloat(itemsPerRow) + return raw.rounded(.down) + } + + var columns: [GridItem] { + Array( + repeating: GridItem(.fixed(cellSize), spacing: spacing), + count: itemsPerRow + ) + } +} From 5b258df7537581d51c477393de2d0592aeaafd9c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:04:52 +1200 Subject: [PATCH 13/21] Add MediaGridItem display model replacing MediaListItem --- .../Models/MediaGridItem.swift | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift new file mode 100644 index 000000000000..bddfcb7338f8 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift @@ -0,0 +1,133 @@ +import Foundation +import CoreGraphics +import WordPressAPI +import WordPressAPIInternal + +/// Display model for a single grid cell. Extracted from +/// `MediaMetadataCollectionItem` at construction time so the cell view is +/// purely render — no FFI calls during `body`. +struct MediaGridItem: Identifiable, Equatable { + let id: Int64 + let kind: MediaKind + let displayTitle: String + let thumbnailURL: URL? // image kind only + let aspectRatio: CGFloat? // image kind only; width / height + let durationString: String? // video kind only + let state: State + let accessibilityLabel: String + + enum State: Equatable { + case loaded(isUpToDate: Bool) + case loading + case error(message: String) + } + + init(item: MediaMetadataCollectionItem) { + switch item.state { + case .fresh(let entity): + self.init(media: entity.data, id: item.id, state: .loaded(isUpToDate: true)) + case .stale(let entity): + self.init(media: entity.data, id: item.id, state: .loaded(isUpToDate: false)) + case .fetchingWithData(let entity): + self.init(media: entity.data, id: item.id, state: .loading) + case .failedWithData(let message, let entity): + self.init(media: entity.data, id: item.id, state: .error(message: message)) + case .fetching, .missing: + self.init(placeholderID: item.id, state: .loading) + case .failed(let message): + self.init(placeholderID: item.id, state: .error(message: message)) + } + } + + /// Designated initializer for data-bearing states. Initializes every + /// stored property exactly once. + private init(media: MediaWithEditContext, id: Int64, state: State) { + let payload = media.mediaDetails.parseAsMimeType(mimeType: media.mimeType) + let kind = payload.flatMap(MediaKind.init(payload:)) ?? .document + + self.id = id + self.kind = kind + self.displayTitle = MediaGridItem.makeTitle(media: media) + self.state = state + self.accessibilityLabel = MediaGridItem.makeAccessibilityLabel(media: media, kind: kind) + + switch payload { + case .image(let imageDetails): + self.thumbnailURL = MediaThumbnailURL.pick(from: imageDetails, sourceUrl: media.sourceUrl) + if imageDetails.width > 0, imageDetails.height > 0 { + self.aspectRatio = CGFloat(imageDetails.width) / CGFloat(imageDetails.height) + } else { + self.aspectRatio = nil + } + self.durationString = nil + case .video(let videoDetails): + self.thumbnailURL = nil + self.aspectRatio = nil + self.durationString = MediaGridDuration.string(forSeconds: videoDetails.length) + case .audio, .document, .none: + self.thumbnailURL = nil + self.aspectRatio = nil + self.durationString = nil + } + } + + /// Designated initializer for payload-less states. Initializes every + /// stored property exactly once. + private init(placeholderID id: Int64, state: State) { + self.id = id + self.kind = .image // best-effort placeholder; cell renders a uniformly-grey square under the loading / error overlay + self.displayTitle = "" + self.thumbnailURL = nil + self.aspectRatio = nil + self.durationString = nil + self.state = state + self.accessibilityLabel = Strings.accessibilityLoadingMedia + } + + private static func makeTitle(media: MediaWithEditContext) -> String { + let raw = (media.title.raw ?? "").trimmingCharacters(in: .whitespacesAndNewlines) + if !raw.isEmpty { return raw } + let slug = media.slug.trimmingCharacters(in: .whitespacesAndNewlines) + if !slug.isEmpty { return slug } + if let filename = filename(from: media.sourceUrl), !filename.isEmpty { + return filename + } + return Strings.untitled + } + + private static func makeAccessibilityLabel(media: MediaWithEditContext, kind: MediaKind) -> String { + // `WpGmtDateTime` is a typealias for `Date` in the wordpress-rs Swift + // binding (wp_api.swift:167229), so `media.dateGmt` is already a + // proper Date — no string parsing needed. The DateFormatter applies + // the user's locale + time zone, so a UTC dateGmt renders as local + // time, matching the V1 cell view-model's behavior. + let date = MediaGridItem.accessibilityDateFormatter.string(from: media.dateGmt) + switch kind { + case .image: + return String.localizedStringWithFormat(Strings.accessibilityLabelImage, date) + case .video: + return String.localizedStringWithFormat(Strings.accessibilityLabelVideo, date) + case .audio: + return String.localizedStringWithFormat(Strings.accessibilityLabelAudio, date) + case .document: + // V1 falls back to filename for documents; if filename can't be + // derived, use the date so the row is still describable. + let filenameOrDate = filename(from: media.sourceUrl) ?? date + return String.localizedStringWithFormat(Strings.accessibilityLabelDocument, filenameOrDate) + } + } + + private static func filename(from sourceUrl: String) -> String? { + guard let url = URL(string: sourceUrl) else { return nil } + let last = url.lastPathComponent + return last.isEmpty ? nil : last + } + + private static let accessibilityDateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.doesRelativeDateFormatting = true + formatter.dateStyle = .full + formatter.timeStyle = .short + return formatter + }() +} From c8e6937c7737759788bd0824531bcdab57eb00d5 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:07:15 +1200 Subject: [PATCH 14/21] Add MediaGridCell with per-kind rendering and V1-parity aspect-ratio behavior --- .../Views/MediaGridCell.swift | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift new file mode 100644 index 000000000000..ad943f7cd548 --- /dev/null +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift @@ -0,0 +1,93 @@ +import SwiftUI +import AsyncImageKit + +struct MediaGridCell: View { + let item: MediaGridItem + let isAspectRatioMode: Bool + + var body: some View { + ZStack(alignment: .bottomTrailing) { + Color(uiColor: .secondarySystemBackground) + content + .frame(maxWidth: .infinity, maxHeight: .infinity) + durationOverlay + stateOverlay + } + .aspectRatio(1, contentMode: .fit) + .clipShape(RoundedRectangle(cornerRadius: isAspectRatioMode ? 4 : 0)) + .accessibilityElement(children: .ignore) + .accessibilityLabel(item.accessibilityLabel) + } + + @ViewBuilder private var content: some View { + switch item.kind { + case .image: + imageContent + case .video: + Image(systemName: "play.rectangle.fill") + .font(.title) + .foregroundStyle(.secondary) + case .audio: + kindIcon(systemImage: "waveform", title: item.displayTitle) + case .document: + kindIcon(systemImage: "doc", title: item.displayTitle) + } + } + + /// V1 parity for aspect-ratio mode: the inner image container takes the + /// media's natural aspect ratio centered inside the square cell. In + /// default mode the container fills the cell and the image crops. See + /// `SiteMediaCollectionCell.swift:113-125` for the V1 reference. + @ViewBuilder private var imageContent: some View { + let cached = CachedAsyncImage(url: item.thumbnailURL) { image in + image.resizable().aspectRatio(contentMode: .fill) + } placeholder: { + Color(uiColor: .secondarySystemBackground) + } + if isAspectRatioMode, let ratio = item.aspectRatio { + cached + .aspectRatio(ratio, contentMode: .fit) + .clipped() + } else { + cached.clipped() + } + } + + @ViewBuilder private var durationOverlay: some View { + if let duration = item.durationString { + Text(duration) + .font(.system(size: 13, weight: .semibold, design: .monospaced)) + .foregroundStyle(.white) + .shadow(color: .black.opacity(0.6), radius: 4, x: 0, y: 0) + .padding(.trailing, 4) + .padding(.bottom, 4) + } + } + + @ViewBuilder private var stateOverlay: some View { + switch item.state { + case .loading, .loaded(isUpToDate: false): + Color.black.opacity(0.05) + case .error: + Image(systemName: "exclamationmark.triangle") + .foregroundStyle(.secondary) + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(Color(uiColor: .secondarySystemBackground)) + case .loaded(isUpToDate: true): + EmptyView() + } + } + + private func kindIcon(systemImage: String, title: String) -> some View { + VStack(spacing: 4) { + Image(systemName: systemImage).font(.title2) + Text(title) + .font(.caption2) + .lineLimit(2) + .multilineTextAlignment(.center) + .truncationMode(.middle) + .padding(.horizontal, 4) + } + .foregroundStyle(.secondary) + } +} From 9b38ea8dca91aee4aac580ba11053c421861ba76 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:09:25 +1200 Subject: [PATCH 15/21] Extend MediaTrackerEvent with M2 cases and route them through MediaTrackerAdapter --- .../Analytics/MediaTracker.swift | 4 +++- .../Analytics/MediaTrackerAdapter.swift | 21 ++++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift b/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift index 0f1e53a4cbe8..a0fd069fbeaf 100644 --- a/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift +++ b/Modules/Sources/WordPressMediaLibrary/Analytics/MediaTracker.swift @@ -13,7 +13,9 @@ public protocol MediaTracker { public enum MediaTrackerEvent: Sendable { case mediaLibraryOpened - // M2-M7 add more cases here. + case mediaLibraryFilterChanged(kind: MediaKind?) // nil = "All" + case mediaLibrarySearched(queryLength: Int) // fires AFTER 300ms debounce trailing edge; non-empty only + case mediaLibraryGridModeToggled(isAspectRatio: Bool) } /// No-op tracker for previews and module-internal default-construction. diff --git a/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift b/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift index ce702230a467..f20496925cac 100644 --- a/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift +++ b/WordPress/Classes/Utility/Analytics/MediaTrackerAdapter.swift @@ -4,8 +4,8 @@ import WordPressMediaLibrary import WordPressShared /// App-target adapter that bridges the module's `MediaTracker` to -/// `WPAppAnalytics` while preserving the V1 analytics property fidelity -/// (tap_source, tab_source) and adding an `is_v2: "1"` discriminator. +/// `WPAppAnalytics` while preserving V1 analytics property fidelity +/// (`tap_source`, `tab_source`, `is_v2`). @MainActor struct MediaTrackerAdapter: MediaTracker { let blog: Blog @@ -13,10 +13,25 @@ struct MediaTrackerAdapter: MediaTracker { func track(_ event: MediaTrackerEvent) { let stat: WPAnalyticsStat + var properties = baseProperties + switch event { case .mediaLibraryOpened: stat = .openedMediaLibrary + + case .mediaLibraryFilterChanged(let kind): + stat = .siteMediaFilterChanged + properties["filter_kind"] = kind?.rawValue ?? "all" + + case .mediaLibrarySearched(let queryLength): + stat = .siteMediaSearched + properties["query_length"] = queryLength + + case .mediaLibraryGridModeToggled(let isAspectRatio): + stat = .siteMediaGridModeToggled + properties["mode"] = isAspectRatio ? "aspect_ratio" : "square" } - WPAppAnalytics.track(stat, properties: baseProperties, blog: blog) + + WPAppAnalytics.track(stat, properties: properties, blog: blog) } } From ae467e02801348ee794a13f45119321772bb0531 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:14:03 +1200 Subject: [PATCH 16/21] Rewrite Media Library V2 view model and view for M2 grid --- .../Models/MediaListItem.swift | 67 ----- .../Views/MediaLibraryRow.swift | 56 ---- .../Views/MediaLibraryView.swift | 143 +++++++--- .../Views/MediaLibraryViewModel.swift | 270 +++++++++++------- 4 files changed, 273 insertions(+), 263 deletions(-) delete mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaListItem.swift delete mode 100644 Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryRow.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaListItem.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaListItem.swift deleted file mode 100644 index 187b8a330b8f..000000000000 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaListItem.swift +++ /dev/null @@ -1,67 +0,0 @@ -import Foundation -import WordPressAPI -import WordPressAPIInternal - -struct MediaListItem: Identifiable, Equatable { - let id: Int64 - let title: String? - let thumbnailURL: URL? - let state: State - - enum State: Equatable { - case loaded(isUpToDate: Bool) - case loading - case error(message: String) - } - - init(item: MediaMetadataCollectionItem) { - self.id = item.id - - switch item.state { - case .fresh(let entity): - self.title = MediaListItem.makeTitle(from: entity.data) - self.thumbnailURL = MediaListItem.makeThumbnailURL(from: entity.data) - self.state = .loaded(isUpToDate: true) - - case .stale(let entity): - self.title = MediaListItem.makeTitle(from: entity.data) - self.thumbnailURL = MediaListItem.makeThumbnailURL(from: entity.data) - self.state = .loaded(isUpToDate: false) - - case .fetchingWithData(let entity): - self.title = MediaListItem.makeTitle(from: entity.data) - self.thumbnailURL = MediaListItem.makeThumbnailURL(from: entity.data) - self.state = .loading - - case .fetching, .missing: - self.title = nil - self.thumbnailURL = nil - self.state = .loading - - case .failed(let error): - self.title = nil - self.thumbnailURL = nil - self.state = .error(message: error) - - case .failedWithData(let error, let entity): - self.title = MediaListItem.makeTitle(from: entity.data) - self.thumbnailURL = MediaListItem.makeThumbnailURL(from: entity.data) - self.state = .error(message: error) - } - } - - /// Prefer `title.raw`, fall back to `slug`, fall back to nil. The view - /// renders `Strings.untitled` when this is nil. - private static func makeTitle(from media: MediaWithEditContext) -> String? { - let raw = (media.title.raw ?? "").trimmingCharacters(in: .whitespacesAndNewlines) - if !raw.isEmpty { return raw } - let slug = media.slug.trimmingCharacters(in: .whitespacesAndNewlines) - return slug.isEmpty ? nil : slug - } - - /// M1 uses `sourceUrl` as the thumbnail. M2 picks a smaller size from - /// `media.mediaDetails.sizes` for grid rendering. - private static func makeThumbnailURL(from media: MediaWithEditContext) -> URL? { - URL(string: media.sourceUrl) - } -} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryRow.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryRow.swift deleted file mode 100644 index deafbf604c4e..000000000000 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryRow.swift +++ /dev/null @@ -1,56 +0,0 @@ -import SwiftUI -import AsyncImageKit - -struct MediaLibraryRow: View { - let item: MediaListItem - - var body: some View { - HStack(spacing: 12) { - thumbnail - .frame(width: 44, height: 44) - .clipShape(RoundedRectangle(cornerRadius: 6)) - Text(displayTitle) - .font(.body) - .lineLimit(1) - Spacer() - } - .opacity(opacityForState) - .accessibilityLabel(displayTitle) - } - - @ViewBuilder - private var thumbnail: some View { - switch item.state { - case .error: - Image(systemName: "exclamationmark.triangle") - .foregroundStyle(.secondary) - .frame(maxWidth: .infinity, maxHeight: .infinity) - .background(Color(uiColor: .secondarySystemBackground)) - case .loading, .loaded: - // Use the closure-form initializer so we can call - // `.resizable()` on the inner image — the default - // `CachedAsyncImage(url:)` returns a non-resizable Image (or a - // Color), which would render at the asset's natural size and - // ignore the .frame(width: 44, height: 44) we apply outside. - // Matches the existing pattern in JetpackStats/Views/AvatarView.swift. - CachedAsyncImage(url: item.thumbnailURL) { image in - image - .resizable() - .aspectRatio(contentMode: .fill) - } placeholder: { - Color(uiColor: .secondarySystemBackground) - } - } - } - - private var displayTitle: String { - item.title ?? Strings.untitled - } - - private var opacityForState: Double { - if case .loaded(let isUpToDate) = item.state, !isUpToDate { - return 0.7 - } - return 1.0 - } -} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift index e66f974472e9..e6b59984789e 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift @@ -4,46 +4,125 @@ struct MediaLibraryView: View { @ObservedObject var viewModel: MediaLibraryViewModel let tracker: any MediaTracker + @State private var searchText: String = "" + /// Incremented by the Retry button. A `.task(id: retryToken)` modifier + /// listens for changes; the initial value 0 is ignored so we don't + /// re-fire refresh on appearance (the filter-driven `.task(id:)` already + /// covers initial load). Switching to a SwiftUI-owned token keeps Retry + /// inside the same cancellation lifecycle as filter / search / observer + /// tasks — an unstructured `Task { … }` from the button action would + /// outlive view dismantle. + @State private var retryToken: Int = 0 + var body: some View { - List(viewModel.items) { item in - MediaLibraryRow(item: item) - .onAppear { - Task { await viewModel.loadNextPageIfNeeded(after: item) } + GeometryReader { proxy in + let layout = MediaGridLayoutMath( + availableWidth: proxy.size.width, + isAspectRatioMode: viewModel.isAspectRatioModeEnabled + ) + ScrollView { + LazyVGrid(columns: layout.columns, spacing: layout.spacing) { + ForEach(viewModel.items) { item in + MediaGridCell( + item: item, + isAspectRatioMode: viewModel.isAspectRatioModeEnabled + ) + // Cell-scoped pagination trigger. SwiftUI cancels + // this task on cell disappear AND on view dismantle. + .task(id: item.id) { + await viewModel.loadNextPageIfNeeded(after: item) + } + } } + // V1 parity: top inset matches the spacing. + .padding(.top, layout.spacing) + .animation(.default, value: viewModel.isAspectRatioModeEnabled) + } + .refreshable { await viewModel.refresh(pullToRefresh: true) } + } + .task { tracker.track(.mediaLibraryOpened) } + .task(id: viewModel.filter) { await viewModel.refresh() } + .task(id: viewModel.filter) { await viewModel.handleDataChanges() } + .task(id: searchText) { + // SwiftUI cancels this on every searchText change and on + // view dismantle. try-await propagates cancellation so the + // setFilter hop never fires after dismissal or mid-typing. + do { + try await Task.sleep(for: .milliseconds(300)) + } catch { + return + } + viewModel.setFilter(viewModel.filter.with(search: searchText)) } - .refreshable { await viewModel.pullToRefresh() } - // Two .task modifiers run concurrently from view appearance. Refresh - // is now deterministic on its own (calls loadItems directly after - // network success), so the observer task is purely for subsequent - // updates (browser-side edits etc.). - .task { await viewModel.handleDataChanges() } - .task { - tracker.track(.mediaLibraryOpened) - // performInitialLoad() owns isRefreshing across the entire - // loadCachedItems + refresh sequence so the empty state can't - // flash between them on cold-cache first open. - await viewModel.performInitialLoad() + .task(id: retryToken) { + // Skip the initial value — first appearance is driven by the + // `.task(id: viewModel.filter)` modifier above. + guard retryToken > 0 else { return } + await viewModel.refresh() } .navigationTitle(Strings.title) - // Single overlay with explicit precedence — three separate overlays - // could stack (e.g., empty + error both true after a failed cold- - // cache refresh). Error wins, then empty, then loading. - .overlay { - if let error = viewModel.errorToDisplay() { - errorView(error) - } else if viewModel.shouldDisplayEmptyView { - emptyView - } else if viewModel.shouldDisplayInitialLoading { - ProgressView() + .searchable(text: $searchText, prompt: Strings.searchPrompt) + .autocorrectionDisabled() + .textInputAutocapitalization(.never) + .toolbar { filterMenu } + .overlay { contentOverlay } + } + + @ToolbarContentBuilder private var filterMenu: some ToolbarContent { + ToolbarItem(placement: .topBarTrailing) { + Menu { + Section { + filterButton(for: nil) + ForEach(MediaKind.allCases, id: \.self) { kind in + filterButton(for: kind) + } + } + Section { + Button { + viewModel.isAspectRatioModeEnabled.toggle() + } label: { + Label( + viewModel.isAspectRatioModeEnabled ? Strings.squareGrid : Strings.aspectRatioGrid, + systemImage: viewModel.isAspectRatioModeEnabled + ? "rectangle.arrowtriangle.2.outward" + : "rectangle.arrowtriangle.2.inward" + ) + } + } + } label: { + Image(systemName: "line.3.horizontal.decrease") + } + } + } + + @ViewBuilder private func filterButton(for kind: MediaKind?) -> some View { + let title = kind?.title ?? Strings.filterAll + let isSelected = kind == viewModel.filter.kind + Button { + viewModel.setFilter(viewModel.filter.with(kind: kind)) + } label: { + if isSelected { + Label(title, systemImage: "checkmark") + } else if let systemImage = kind?.systemImageName { + Label(title, systemImage: systemImage) + } else { + Text(title) } } } - private var emptyView: some View { - ContentUnavailableView( - Strings.empty, - systemImage: "photo.on.rectangle" - ) + @ViewBuilder private var contentOverlay: some View { + if viewModel.shouldDisplayInitialLoading { + ProgressView() + } else if let error = viewModel.errorToDisplay() { + errorView(error) + } else if viewModel.shouldDisplaySearchEmpty { + ContentUnavailableView.search(text: viewModel.filter.search) + } else if viewModel.shouldDisplayFilterEmpty { + ContentUnavailableView(Strings.emptyFiltered, systemImage: "photo.on.rectangle") + } else if viewModel.shouldDisplayEmpty { + ContentUnavailableView(Strings.empty, systemImage: "photo.on.rectangle") + } } private func errorView(_ error: Error) -> some View { @@ -55,7 +134,7 @@ struct MediaLibraryView: View { .multilineTextAlignment(.center) .padding(.horizontal) Button(Strings.errorRetry) { - Task { await viewModel.refresh() } + retryToken += 1 } .buttonStyle(.borderedProminent) } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift index 09b808dd5bec..ff1d03bf9d5f 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift @@ -8,207 +8,261 @@ import WordPressCore final class MediaLibraryViewModel: ObservableObject { private let client: WordPressClient private let tracker: any MediaTracker + + /// Cached lazy resolution of `WpService` + a + /// `MediaMetadataCollectionWithEditContext` for the current `filter`. + /// Replaced on every `setFilter(_:)`; evicted on non-cancellation + /// throws in `resolveCollection`. private var collectionTask: Task? - private var isLoadingNextPage = false - @Published private(set) var items: [MediaListItem] = [] + /// Monotonically increments on every `setFilter(_:)`. Methods that + /// publish into `items` / `listInfo` / `error` from an `await` snapshot + /// this at entry and guard every state write on + /// `self.generation == snapshot && !Task.isCancelled`. Both conditions + /// are load-bearing: the generation check rejects writes from an older + /// filter whose body is still finishing its `await` chain, and the + /// cancellation check rejects writes after view dismantle. Swift task + /// cancellation is cooperative — an FFI await may resume normally + /// without throwing `CancellationError`, so the cancellation flag has + /// to be consulted explicitly at every write site. + private var generation: UInt64 = 0 + + @Published private(set) var filter: MediaGridFilter = .initial + @Published private(set) var items: [MediaGridItem] = [] @Published private(set) var listInfo: ListInfo? @Published private(set) var error: Error? - /// Set to true while `refresh()` is in flight. Drives the cold-cache - /// initial-loading spinner explicitly, instead of inferring from the - /// wp-rs collection's `listInfo.isSyncing` (which only updates after - /// the cache observer wakes — racy). @Published private(set) var isRefreshing = false + @Published var isAspectRatioModeEnabled: Bool { + didSet { + AspectRatioPreference.save(isAspectRatioModeEnabled) + tracker.track(.mediaLibraryGridModeToggled(isAspectRatio: isAspectRatioModeEnabled)) + } + } + + private var isLoadingNextPage = false + var shouldDisplayInitialLoading: Bool { items.isEmpty && isRefreshing } - var shouldDisplayEmptyView: Bool { items.isEmpty && !isRefreshing && error == nil } + var shouldDisplayEmpty: Bool { + items.isEmpty && !isRefreshing && error == nil + && filter.search.isEmpty && filter.kind == nil + } + var shouldDisplayFilterEmpty: Bool { + items.isEmpty && !isRefreshing && error == nil + && filter.search.isEmpty && filter.kind != nil + } + var shouldDisplaySearchEmpty: Bool { + items.isEmpty && !isRefreshing && error == nil && !filter.search.isEmpty + } func errorToDisplay() -> Error? { items.isEmpty ? error : nil } init(client: WordPressClient, tracker: any MediaTracker) { self.client = client self.tracker = tracker + self.isAspectRatioModeEnabled = AspectRatioPreference.load() + // Eagerly schedule the first collection task so `.task(id: filter)` + // can await it on first appearance without re-creation. + self.collectionTask = makeCollectionTask(for: .initial) } - /// Resolves WpService and constructs the collection, exactly once. - /// Cached as a Task so concurrent callers (the cache observer task and - /// the refresh task both wake at view appearance) await the same - /// construction. Same pattern WordPressClient itself uses for site - /// info / current user. - private func resolveCollection() async throws -> MediaMetadataCollectionWithEditContext { - if let collectionTask { - return try await collectionTask.value - } - let task = Task { [client] in + /// Builds a Task that awaits `client.service` (actor-isolated) and + /// constructs a `MediaMetadataCollectionWithEditContext` for the given + /// filter. The Task does not capture `self`; it captures only `client`. + private func makeCollectionTask( + for filter: MediaGridFilter + ) -> Task { + Task { [client] in let service = try await client.service return service.media() .createMediaMetadataCollectionWithEditContext( - filter: MediaListFilter(), + filter: filter.asMediaListFilter(), perPage: 100 ) } - self.collectionTask = task - return try await task.value } - /// Loads cached items into `items` immediately so the first paint isn't - /// blocked on the network round-trip. - func loadCachedItems() async { + /// Awaits the cached collection task. On a non-cancellation throw, + /// evicts the cached task IF the generation hasn't advanced — so the + /// next call (e.g. Retry) builds a fresh task. Cancellation throws + /// don't evict because they're expected during filter change and + /// `setFilter` already installed the replacement. + private func resolveCollection() async throws -> MediaMetadataCollectionWithEditContext { + if collectionTask == nil { + collectionTask = makeCollectionTask(for: filter) + } + let task = collectionTask! + let snapshot = generation do { - let collection = try await resolveCollection() - await loadItems(from: collection) + return try await task.value } catch { - Loggers.mediaLibrary.error("Failed to load cached items: \(error)") + if !(error is CancellationError), generation == snapshot { + collectionTask = nil + } + throw error } } - /// Single entry point for the SwiftUI .task block. Owns isRefreshing - /// across BOTH loadCachedItems() and refresh() so the empty state can't - /// flash in the microsecond window between them on cold-cache first - /// open. SwiftUI re-evaluates body on every @Published change, so even - /// a single MainActor scheduling hop where (items.isEmpty && - /// !isRefreshing && error == nil) holds will paint the empty view. - func performInitialLoad() async { - isRefreshing = true - defer { isRefreshing = false } - await loadCachedItems() - await refresh() + func setFilter(_ newFilter: MediaGridFilter) { + guard filter != newFilter else { return } + let oldFilter = filter + collectionTask?.cancel() + generation &+= 1 + withAnimation { + filter = newFilter + items = [] + listInfo = nil + error = nil + // Flip the loading overlay on in the SAME render tick that + // clears `items` so the between-task render gap doesn't paint + // the empty / filter-empty / search-empty state for one frame + // before refresh() runs on the next MainActor turn. + isRefreshing = true + } + collectionTask = makeCollectionTask(for: newFilter) + if newFilter.kind != oldFilter.kind { + tracker.track(.mediaLibraryFilterChanged(kind: newFilter.kind)) + } + if newFilter.search != oldFilter.search, !newFilter.search.isEmpty { + tracker.track(.mediaLibrarySearched(queryLength: newFilter.search.count)) + } } - func refresh() async { - // isRefreshing drives the cold-cache initial-loading UI deterministically, - // independent of the wp-rs cache observer's wake timing. + func refresh(pullToRefresh: Bool = false) async { + let snapshot = generation + // Idempotent — already true after setFilter; covers first .task + // invocation on view appearance (where setFilter never ran). isRefreshing = true - defer { isRefreshing = false } + // Clear stale error so a successful retry that returns zero items + // shows the empty state rather than the previous failure. setFilter + // also clears errors, but Retry and pull-to-refresh come through + // here without changing the filter. + error = nil + defer { + if generation == snapshot, !Task.isCancelled { + isRefreshing = false + } + } + if !pullToRefresh { + await loadCachedItems(snapshot: snapshot) + } + await fetchPageOne(snapshot: snapshot) + } - // Clear stale error from any previous failed attempt so a successful - // retry unblocks the empty/list UI even when the new fetch returns - // zero items. Without this, errorToDisplay() would keep showing the - // old error because items.isEmpty stays true. - self.error = nil + private func loadCachedItems(snapshot: UInt64) async { + do { + let collection = try await resolveCollection() + await loadItems(from: collection, snapshot: snapshot) + } catch { + if !(error is CancellationError) { + Loggers.mediaLibrary.error("Failed to load cached items: \(error)") + } + } + } + private func fetchPageOne(snapshot: UInt64) async { do { let collection = try await resolveCollection() + guard generation == snapshot, !Task.isCancelled else { return } _ = try await collection.refresh() - // Reload items directly after refresh succeeds, instead of - // relying on the cache observer to wake up. SwiftUI doesn't - // order sibling .task modifiers, so handleDataChanges() may not - // have subscribed before refresh wrote to the cache. This makes - // the cold-cache first load deterministic; handleDataChanges() - // handles subsequent updates only. - await loadItems(from: collection) + await loadItems(from: collection, snapshot: snapshot) } catch { - Loggers.mediaLibrary.error("Media library refresh failed: \(error)") - show(error: error) + if !(error is CancellationError) { + Loggers.mediaLibrary.error("Media library refresh failed: \(error)") + show(error: error, snapshot: snapshot) + } } } - func pullToRefresh() async { await refresh() } - - /// Long-running cache observer for SUBSEQUENT updates only — the initial - /// load is handled deterministically by `refresh()` calling - /// `loadItems(from:)` directly. + /// Long-running cache observer. Restarted by `.task(id: viewModel.filter)` + /// on every filter change, so the snapshot captured here stays tied to + /// the active-at-start generation. func handleDataChanges() async { + let snapshot = generation let collection: MediaMetadataCollectionWithEditContext do { collection = try await resolveCollection() } catch { - // Collection couldn't be constructed; nothing to observe. return } - // The filter closure runs synchronously on whichever thread the - // upstream publisher emits on — for wp-rs's `databaseUpdatesPublisher()` - // that's the SQLite worker thread (NotificationCenter post from the - // rusqlite update hook). Marking it `@Sendable` opts out of the - // implicit `@MainActor` isolation that Swift 6 would otherwise - // inherit from the enclosing class, so the cheap `isRelevantUpdate` - // check stays on the background thread without tripping the runtime - // MainActor assertion. The downstream `.collect(.byTime(DispatchQueue.main, …))` - // hops to main before delivering batches, so the `for await` body - // runs on main where it mutates `@Published` state. let batches = await client.cache.databaseUpdatesPublisher() - .filter { @Sendable [weak collection] in collection?.isRelevantUpdate(hook: $0) == true } + .filter { @Sendable [weak collection] in + collection?.isRelevantUpdate(hook: $0) == true + } .collect(.byTime(DispatchQueue.main, .milliseconds(50))) .values for await _ in batches { - await loadItems(from: collection) + await loadItems(from: collection, snapshot: snapshot) } } - // TODO: Pagination is temporary for M1. Future milestones will switch - // the Media Library to a full-library sync model rather than per-page - // fetches. func loadNextPage() async throws { - // Two guards: - // 1. !isRefreshing — warm-cache first open paints cached rows - // while the initial refresh is still in flight; trailing-row - // .onAppear can fire loadNextPage concurrently with that - // refresh. Defer pagination until the initial load completes. - // 2. !isLoadingNextPage — trailing-N rows can each fire .onAppear - // in quick succession before the collection's listInfo - // reflects the sync, producing duplicate fetches and noisy - // StaleLoadMore errors. Mirrors the isLoadingMore guard in - // CustomPostListView. guard !isRefreshing, !isLoadingNextPage else { return } isLoadingNextPage = true defer { isLoadingNextPage = false } + let snapshot = generation let collection = try await resolveCollection() guard !collection.isSyncing, - collection.hasMorePages() ?? true + collection.hasMorePages() ?? true, + generation == snapshot, + !Task.isCancelled else { return } + if collection.listInfo()?.currentPage == nil { _ = try await collection.refresh() } else { _ = try await collection.loadNextPage() } - await loadItems(from: collection) + await loadItems(from: collection, snapshot: snapshot) } - // TODO: Pagination is temporary for M1 — see loadNextPage(). - func loadNextPageIfNeeded(after item: MediaListItem) async { - // Trigger a fetch only when the row that just appeared is one of the - // last few rows we have loaded — protects against firing for every - // single .onAppear above the fold. + func loadNextPageIfNeeded(after item: MediaGridItem) async { let trailingThreshold = 10 guard items.suffix(trailingThreshold).contains(where: { $0.id == item.id }) else { return } + // Capture the snapshot BEFORE the await so a filter change during + // pagination can't route the error into the new generation's state. + let snapshot = generation do { try await loadNextPage() } catch { - Loggers.mediaLibrary.error("Media library loadNextPage failed: \(error)") - show(error: error) + if !(error is CancellationError) { + Loggers.mediaLibrary.error("Media library loadNextPage failed: \(error)") + show(error: error, snapshot: snapshot) + } } } - /// Reads the current snapshot from the collection and updates @Published - /// state. Shared by `loadCachedItems()`, `refresh()`, `loadNextPage()`, - /// and `handleDataChanges()` so all reload paths funnel through the - /// same code. - private func loadItems(from collection: MediaMetadataCollectionWithEditContext) async { + private func loadItems( + from collection: MediaMetadataCollectionWithEditContext, + snapshot: UInt64 + ) async { do { - self.listInfo = collection.listInfo() + let listInfo = collection.listInfo() let metadataItems = try await collection.loadItems() + guard generation == snapshot, !Task.isCancelled else { return } + self.listInfo = listInfo withAnimation { - self.items = metadataItems.map(MediaListItem.init(item:)) + self.items = metadataItems.map(MediaGridItem.init(item:)) } } catch { - Loggers.mediaLibrary.error("Failed to load items: \(error)") + if !(error is CancellationError) { + Loggers.mediaLibrary.error("Failed to load items: \(error)") + } } } - private func show(error: Error) { + private func show(error: Error, snapshot: UInt64) { if case FetchError.StaleLoadMore = error { return } + guard generation == snapshot, !Task.isCancelled else { return } self.error = error } } -// Mirrors the private extension in CustomPostListViewModel. -// `isSyncing` is NOT part of the wordpress-rs ListInfo surface (which -// exposes only state, currentPage, totalPages, totalItems, and perPage); -// this extension fills the gap. +// Mirrors the private extension in `CustomPostListViewModel.swift`. private extension ListInfo { var isSyncing: Bool { state == .fetchingFirstPage || state == .fetchingNextPage From affa0bc9042e0e2ae3ebcce95ad00273257ab64b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:35:12 +1200 Subject: [PATCH 17/21] Address code review findings for M2 grid Distinct accessibility label for failed-without-data placeholders so VoiceOver doesn't read "Loading media" over an error icon. Inner image content takes the 4pt rounding in aspect-ratio mode, leaving the outer cell square for non-square images to match V1's container-level rounding. Comment updates for loadItems' intentional log-only error path, setFilter analytics flow, and MediaGridDuration's deliberate locale-neutral format. --- .../Models/MediaGridDuration.swift | 9 +++++-- .../Models/MediaGridItem.swift | 13 ++++++++-- .../Strings/Strings.swift | 6 +++++ .../Views/MediaGridCell.swift | 26 +++++++++++++++---- .../Views/MediaLibraryViewModel.swift | 15 +++++++++++ 5 files changed, 60 insertions(+), 9 deletions(-) diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift index e1df99acb30b..47dca2be37dd 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridDuration.swift @@ -1,8 +1,13 @@ import Foundation /// Formats a video duration (seconds) as `m:ss` for durations under one hour -/// and `h:mm:ss` from one hour up. Matches the V1 reference at -/// `SiteMediaCollectionCellViewModel.swift`'s `makeString(forDuration:)`. +/// and `h:mm:ss` from one hour up. **Intentionally locale-neutral**: V1's +/// `DateComponentsFormatter`-based output varies in non-Latin-numeral +/// locales (Arabic, Hindi, etc.), but the duration badge uses a +/// `.monospaced` font and reads more like a timecode than a sentence — a +/// stable `digit:digit` output is the better fit. This is a small, +/// deliberate deviation from V1's `SiteMediaCollectionCellViewModel.swift`'s +/// `makeString(forDuration:)`. enum MediaGridDuration { static func string(forSeconds seconds: UInt32) -> String { let total = Int(seconds) diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift index bddfcb7338f8..b8f567122e2a 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift @@ -72,7 +72,11 @@ struct MediaGridItem: Identifiable, Equatable { } /// Designated initializer for payload-less states. Initializes every - /// stored property exactly once. + /// stored property exactly once. The accessibility label branches on + /// `state` because the same initializer covers both `.fetching` / + /// `.missing` (genuinely loading) and `.failed` (error without payload): + /// VoiceOver shouldn't hear "Loading media" while the cell shows an + /// error icon. private init(placeholderID id: Int64, state: State) { self.id = id self.kind = .image // best-effort placeholder; cell renders a uniformly-grey square under the loading / error overlay @@ -81,7 +85,12 @@ struct MediaGridItem: Identifiable, Equatable { self.aspectRatio = nil self.durationString = nil self.state = state - self.accessibilityLabel = Strings.accessibilityLoadingMedia + switch state { + case .error: + self.accessibilityLabel = Strings.accessibilityErrorMedia + case .loading, .loaded: + self.accessibilityLabel = Strings.accessibilityLoadingMedia + } } private static func makeTitle(media: MediaWithEditContext) -> String { diff --git a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift index 1422d5ea0d2b..b231d6beff67 100644 --- a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift +++ b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift @@ -115,4 +115,10 @@ enum Strings { value: "Loading media", comment: "Accessibility label for a cell that is still loading its data" ) + + static let accessibilityErrorMedia = NSLocalizedString( + "mediaLibrary.accessibility.error", + value: "Media failed to load", + comment: "Accessibility label for a cell whose underlying media couldn't be loaded" + ) } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift index ad943f7cd548..4e1e42524fe8 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift @@ -14,11 +14,25 @@ struct MediaGridCell: View { stateOverlay } .aspectRatio(1, contentMode: .fit) - .clipShape(RoundedRectangle(cornerRadius: isAspectRatioMode ? 4 : 0)) + .clipShape(RoundedRectangle(cornerRadius: outerCornerRadius)) .accessibilityElement(children: .ignore) .accessibilityLabel(item.accessibilityLabel) } + /// V1 parity: in aspect-ratio mode, image cells with a non-square aspect + /// ratio leave the OUTER cell square (sharp corners) and apply the 4pt + /// rounding to the inner image container only — see `imageContent` below + /// for the inner clip. For square images and non-image kinds, V1's + /// imageContainerView fills the cell, so cell-level rounding matches + /// V1's container-level rounding. + private var outerCornerRadius: CGFloat { + guard isAspectRatioMode else { return 0 } + if item.kind == .image, let ratio = item.aspectRatio, ratio != 1.0 { + return 0 + } + return 4 + } + @ViewBuilder private var content: some View { switch item.kind { case .image: @@ -35,9 +49,11 @@ struct MediaGridCell: View { } /// V1 parity for aspect-ratio mode: the inner image container takes the - /// media's natural aspect ratio centered inside the square cell. In - /// default mode the container fills the cell and the image crops. See - /// `SiteMediaCollectionCell.swift:113-125` for the V1 reference. + /// media's natural aspect ratio centered inside the square cell and + /// gets the 4pt rounding applied to itself (not to the surrounding + /// letterbox area). In default mode the container fills the cell and + /// the image crops with no rounding. See `SiteMediaCollectionCell.swift:113-125` + /// for the V1 reference. @ViewBuilder private var imageContent: some View { let cached = CachedAsyncImage(url: item.thumbnailURL) { image in image.resizable().aspectRatio(contentMode: .fill) @@ -47,7 +63,7 @@ struct MediaGridCell: View { if isAspectRatioMode, let ratio = item.aspectRatio { cached .aspectRatio(ratio, contentMode: .fit) - .clipped() + .clipShape(RoundedRectangle(cornerRadius: 4)) } else { cached.clipped() } diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift index ff1d03bf9d5f..4257915dceae 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift @@ -119,6 +119,11 @@ final class MediaLibraryViewModel: ObservableObject { isRefreshing = true } collectionTask = makeCollectionTask(for: newFilter) + // Analytics fire synchronously, after the state mutation above. Each + // event is guarded so filter-only changes don't fire a stale-search + // event and search-only changes don't fire a filter event; clearing + // the search to empty doesn't fire (the !newFilter.search.isEmpty + // guard) because that's not a user-initiated search. if newFilter.kind != oldFilter.kind { tracker.track(.mediaLibraryFilterChanged(kind: newFilter.kind)) } @@ -236,6 +241,16 @@ final class MediaLibraryViewModel: ObservableObject { } } + /// Reads the current snapshot from the collection and updates the + /// published state. Errors from `collection.loadItems()` (a SQLite read) + /// are logged only — never surfaced via `show(error:)`. This is the + /// observer-loop path's behavior carried forward from M1, and is shared + /// with the user-visible paths (`fetchPageOne`, `loadNextPage`) for + /// simplicity. A SQLite-read failure after a successful network refresh + /// or pagination would currently show stale rows with only a log line; + /// M3+ should split the user-visible callers off so they can surface + /// the error to `show(error:)`. The cache-corruption failure mode is + /// rare enough that this trade-off is acceptable for M2. private func loadItems( from collection: MediaMetadataCollectionWithEditContext, snapshot: UInt64 From db0fc0473fb96a754228fcc73153e2699a27d69b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 14:51:23 +1200 Subject: [PATCH 18/21] Address second-round code review findings for M2 grid Video cells now render via CachedAsyncImage(videoUrl:) instead of the generic play icon, restoring V1 surface parity for video thumbnails. Documents-filter narrowing vs V1 documented inline on MediaKind so the wp-rs Phase 0 follow-up is visible from the code. Adds eight unit tests for MediaGridLayoutMath covering V1's 4-per-row / 5-per-row threshold, spacing modes, and cell-size rounding. --- .../Models/MediaGridItem.swift | 7 ++- .../Models/MediaKind.swift | 11 ++++ .../Views/MediaGridCell.swift | 26 ++++++++-- .../MediaGridLayoutMathTests.swift | 50 +++++++++++++++++++ 4 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift index b8f567122e2a..ad9bf0be65b7 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift @@ -10,7 +10,7 @@ struct MediaGridItem: Identifiable, Equatable { let id: Int64 let kind: MediaKind let displayTitle: String - let thumbnailURL: URL? // image kind only + let thumbnailURL: URL? // image or video kind; the cell picks the right CachedAsyncImage initializer based on kind let aspectRatio: CGFloat? // image kind only; width / height let durationString: String? // video kind only let state: State @@ -61,7 +61,10 @@ struct MediaGridItem: Identifiable, Equatable { } self.durationString = nil case .video(let videoDetails): - self.thumbnailURL = nil + // For video, `thumbnailURL` carries the video file URL itself — + // the cell renders it via `CachedAsyncImage(videoUrl:)`, which + // extracts a frame for the thumbnail (V1 parity). + self.thumbnailURL = URL(string: media.sourceUrl) self.aspectRatio = nil self.durationString = MediaGridDuration.string(forSeconds: videoDetails.length) case .audio, .document, .none: diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift index 54a4b8bcbde6..e8d36cb1afd7 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift @@ -22,6 +22,17 @@ public enum MediaKind: String, CaseIterable, Hashable, Sendable { } } + /// Maps the V2 grid filter selection to the wordpress-rs REST query + /// parameter. **Known narrowing for `.document`:** V1's "Documents" + /// bucket includes attachments the system classifies as `text/*` (e.g. + /// `.txt`, `.md`) alongside the `application/*` MIME family, because V1 + /// builds its filter locally against Core Data's `mediaTypeString`. V2 + /// goes through the wordpress-rs `MediaListFilter.mediaType` parameter, + /// which is a single optional `MediaTypeParam` — we can't OR + /// `.application` and `.text` in one query without an upstream + /// wordpress-rs change. M2 ships with the narrower `.application`-only + /// document bucket; full V1 parity here depends on the wordpress-rs + /// Phase 0 surface gaining multi-value media-type filtering. var asMediaTypeParam: MediaTypeParam { switch self { case .image: .image diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift index 4e1e42524fe8..762c8a728acb 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift @@ -38,9 +38,7 @@ struct MediaGridCell: View { case .image: imageContent case .video: - Image(systemName: "play.rectangle.fill") - .font(.title) - .foregroundStyle(.secondary) + videoContent case .audio: kindIcon(systemImage: "waveform", title: item.displayTitle) case .document: @@ -48,6 +46,28 @@ struct MediaGridCell: View { } } + /// Video thumbnail via AsyncImageKit's `CachedAsyncImage(videoUrl:)`, + /// which extracts a frame from the video file at `thumbnailURL` (the + /// video's `sourceUrl`). The duration badge sits on top via the + /// `durationOverlay` modifier. Falls back to a centered play-rectangle + /// icon when the URL is missing. + @ViewBuilder private var videoContent: some View { + if let url = item.thumbnailURL { + CachedAsyncImage(videoUrl: url) { image in + image.resizable().aspectRatio(contentMode: .fill) + } placeholder: { + Image(systemName: "play.rectangle.fill") + .font(.title) + .foregroundStyle(.secondary) + } + .clipped() + } else { + Image(systemName: "play.rectangle.fill") + .font(.title) + .foregroundStyle(.secondary) + } + } + /// V1 parity for aspect-ratio mode: the inner image container takes the /// media's natural aspect ratio centered inside the square cell and /// gets the 4pt rounding applied to itself (not to the surrounding diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift new file mode 100644 index 000000000000..c1f2b4537ad1 --- /dev/null +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift @@ -0,0 +1,50 @@ +import Testing + +@testable import WordPressMediaLibrary + +struct MediaGridLayoutMathTests { + @Test func fourPerRowBelowFiveHundredPoints() { + let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) + #expect(layout.itemsPerRow == 4) + } + + @Test func fivePerRowAtAndAboveFiveHundredPoints() { + let layout = MediaGridLayoutMath(availableWidth: 500, isAspectRatioMode: false) + #expect(layout.itemsPerRow == 5) + } + + @Test func defaultSpacingIsTwo() { + let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) + #expect(layout.spacing == 2) + } + + @Test func aspectRatioSpacingIsEight() { + let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: true) + #expect(layout.spacing == 8) + } + + @Test func cellSizeRoundsDownForPhoneDefault() { + // 390 - 2 * 3 = 384; / 4 = 96.0 + let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) + #expect(layout.cellSize == 96) + } + + @Test func cellSizeRoundsDownForPhoneAspectRatio() { + // 390 - 8 * 3 = 366; / 4 = 91.5 → rounds down to 91 + let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: true) + #expect(layout.cellSize == 91) + } + + @Test func cellSizeRoundsDownForPad() { + // 768 - 2 * 4 = 760; / 5 = 152 + let layout = MediaGridLayoutMath(availableWidth: 768, isAspectRatioMode: false) + #expect(layout.cellSize == 152) + } + + @Test func columnsCountMatchesItemsPerRow() { + let phone = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) + let pad = MediaGridLayoutMath(availableWidth: 768, isAspectRatioMode: false) + #expect(phone.columns.count == 4) + #expect(pad.columns.count == 5) + } +} From 26c397a00e0e71d00dfedb5cafef9c3d28acf0f0 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 15:39:36 +1200 Subject: [PATCH 19/21] Fix aspect-ratio grid image overlap Inner image .aspectRatio(.fit) on a .resizable() CachedAsyncImage was rendering at intrinsic pixel-derived size under an infinite frame proposal, overflowing the cell horizontally into adjacent grid positions. Wraps the cell body in a GeometryReader so the inner image content gets an explicit square frame constraint. Accepts the small V1-parity deviation flagged by code-review-1 finding 2 (outer cell rounds whole vs V1's inner-container-only) because the alternative caused this overflow bug. --- .../Views/MediaGridCell.swift | 75 ++++++++++--------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift index 762c8a728acb..8d74f882c135 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift @@ -6,33 +6,29 @@ struct MediaGridCell: View { let isAspectRatioMode: Bool var body: some View { - ZStack(alignment: .bottomTrailing) { - Color(uiColor: .secondarySystemBackground) - content - .frame(maxWidth: .infinity, maxHeight: .infinity) - durationOverlay - stateOverlay + // GeometryReader inside .aspectRatio(1, .fit) gives the cell a + // square frame and a known size. Without this, `.resizable()` + + // `.aspectRatio(.fit)` on the image renders at the image's + // intrinsic pixel-derived size and overflows the cell horizontally + // into adjacent grid positions. + GeometryReader { proxy in + let side = min(proxy.size.width, proxy.size.height) + ZStack(alignment: .bottomTrailing) { + Color(uiColor: .secondarySystemBackground) + content + .frame(width: side, height: side) + .clipped() + durationOverlay + stateOverlay + } + .frame(width: side, height: side) } .aspectRatio(1, contentMode: .fit) - .clipShape(RoundedRectangle(cornerRadius: outerCornerRadius)) + .clipShape(RoundedRectangle(cornerRadius: isAspectRatioMode ? 4 : 0)) .accessibilityElement(children: .ignore) .accessibilityLabel(item.accessibilityLabel) } - /// V1 parity: in aspect-ratio mode, image cells with a non-square aspect - /// ratio leave the OUTER cell square (sharp corners) and apply the 4pt - /// rounding to the inner image container only — see `imageContent` below - /// for the inner clip. For square images and non-image kinds, V1's - /// imageContainerView fills the cell, so cell-level rounding matches - /// V1's container-level rounding. - private var outerCornerRadius: CGFloat { - guard isAspectRatioMode else { return 0 } - if item.kind == .image, let ratio = item.aspectRatio, ratio != 1.0 { - return 0 - } - return 4 - } - @ViewBuilder private var content: some View { switch item.kind { case .image: @@ -68,24 +64,31 @@ struct MediaGridCell: View { } } - /// V1 parity for aspect-ratio mode: the inner image container takes the - /// media's natural aspect ratio centered inside the square cell and - /// gets the 4pt rounding applied to itself (not to the surrounding - /// letterbox area). In default mode the container fills the cell and - /// the image crops with no rounding. See `SiteMediaCollectionCell.swift:113-125` + /// Aspect-ratio mode letterboxes the image inside the square cell using + /// `.fit` on the resizable image — this keeps the rendered image + /// constrained to cell bounds and avoids the overflow that an outer + /// `.aspectRatio(ratio, .fit)` modifier on the CachedAsyncImage wrapper + /// produced (image escaping its cell horizontally into adjacent + /// positions). Default mode uses `.fill` so the image crops to cover + /// the cell. The whole cell rounds at 4pt in aspect-ratio mode (see + /// outer `.clipShape` in `body`); this is a small deviation from V1 + /// which rounds only the inner image container, but the alternative + /// caused a much worse rendering bug. See `SiteMediaCollectionCell.swift:113-125` /// for the V1 reference. @ViewBuilder private var imageContent: some View { - let cached = CachedAsyncImage(url: item.thumbnailURL) { image in - image.resizable().aspectRatio(contentMode: .fill) - } placeholder: { - Color(uiColor: .secondarySystemBackground) - } - if isAspectRatioMode, let ratio = item.aspectRatio { - cached - .aspectRatio(ratio, contentMode: .fit) - .clipShape(RoundedRectangle(cornerRadius: 4)) + if isAspectRatioMode { + CachedAsyncImage(url: item.thumbnailURL) { image in + image.resizable().aspectRatio(contentMode: .fit) + } placeholder: { + Color(uiColor: .secondarySystemBackground) + } } else { - cached.clipped() + CachedAsyncImage(url: item.thumbnailURL) { image in + image.resizable().aspectRatio(contentMode: .fill) + } placeholder: { + Color(uiColor: .secondarySystemBackground) + } + .clipped() } } From d9038f59124cbd6a0704ef2e606846a0dcc53e41 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 21:46:10 +1200 Subject: [PATCH 20/21] Update code comments --- .../WordPressMediaLibrary/Models/MediaGridItem.swift | 8 ++++---- .../Models/MediaGridLayoutMath.swift | 7 +++---- .../Sources/WordPressMediaLibrary/Models/MediaKind.swift | 8 ++++---- .../Sources/WordPressMediaLibrary/Strings/Strings.swift | 4 ---- .../WordPressMediaLibrary/Views/MediaGridCell.swift | 4 ++-- .../Views/MediaLibraryViewModel.swift | 9 ++++----- .../WordPressMediaLibraryTests/MediaKindTests.swift | 6 +++--- .../MediaThumbnailURLTests.swift | 4 ++-- 8 files changed, 22 insertions(+), 28 deletions(-) diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift index ad9bf0be65b7..02c532b751ef 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridItem.swift @@ -109,10 +109,10 @@ struct MediaGridItem: Identifiable, Equatable { private static func makeAccessibilityLabel(media: MediaWithEditContext, kind: MediaKind) -> String { // `WpGmtDateTime` is a typealias for `Date` in the wordpress-rs Swift - // binding (wp_api.swift:167229), so `media.dateGmt` is already a - // proper Date — no string parsing needed. The DateFormatter applies - // the user's locale + time zone, so a UTC dateGmt renders as local - // time, matching the V1 cell view-model's behavior. + // binding, so `media.dateGmt` is already a proper Date — no string + // parsing needed. The DateFormatter applies the user's locale + time + // zone, so a UTC dateGmt renders as local time, matching the V1 cell + // view-model's behavior. let date = MediaGridItem.accessibilityDateFormatter.string(from: media.dateGmt) switch kind { case .image: diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift index 76f16d800321..977213effc62 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift @@ -2,10 +2,9 @@ import Foundation import SwiftUI /// Pure layout math for the Media Library grid. Mirrors V1's -/// `SiteMediaCollectionViewController.updateFlowLayoutItemSize` at lines -/// 148-158: 4 cells per row below 500pt width, 5 otherwise; spacing 2 in -/// default mode, 8 in aspect-ratio mode; cell size rounded down to avoid -/// fractional pixels. +/// `SiteMediaCollectionViewController.updateFlowLayoutItemSize`: 4 cells per +/// row below 500pt width, 5 otherwise; spacing 2 in default mode, 8 in +/// aspect-ratio mode; cell size rounded down to avoid fractional pixels. struct MediaGridLayoutMath { let availableWidth: CGFloat let isAspectRatioMode: Bool diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift index e8d36cb1afd7..bd73bfbfb628 100644 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift +++ b/Modules/Sources/WordPressMediaLibrary/Models/MediaKind.swift @@ -6,7 +6,7 @@ import WordPressAPIInternal /// can carry it across the module boundary; the app-target analytics /// adapter reads `rawValue` for its property dict. The `MediaDetailsPayload` /// initializer, the `MediaTypeParam` mapping, and the UI helpers -/// (`title` / `systemImageName`, added in Task 11) all stay module-internal +/// (`title` / `systemImageName`) all stay module-internal /// — they're used only inside the module and in `@testable` tests, so /// there's no reason to leak `WordPressAPIInternal` types through the /// public surface. @@ -30,9 +30,9 @@ public enum MediaKind: String, CaseIterable, Hashable, Sendable { /// goes through the wordpress-rs `MediaListFilter.mediaType` parameter, /// which is a single optional `MediaTypeParam` — we can't OR /// `.application` and `.text` in one query without an upstream - /// wordpress-rs change. M2 ships with the narrower `.application`-only - /// document bucket; full V1 parity here depends on the wordpress-rs - /// Phase 0 surface gaining multi-value media-type filtering. + /// wordpress-rs change. V2 ships with the narrower `.application`-only + /// document bucket; full V1 parity here depends on a wordpress-rs + /// change to support multi-value media-type filtering. var asMediaTypeParam: MediaTypeParam { switch self { case .image: .image diff --git a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift index b231d6beff67..1ce8d57a05ca 100644 --- a/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift +++ b/Modules/Sources/WordPressMediaLibrary/Strings/Strings.swift @@ -1,8 +1,6 @@ import Foundation enum Strings { - // MARK: - M1 - static let title = NSLocalizedString( "mediaLibrary.screen.title", value: "Media", @@ -27,8 +25,6 @@ enum Strings { comment: "Placeholder shown for media items with no title" ) - // MARK: - M2 - static let searchPrompt = NSLocalizedString( "mediaLibrary.search.prompt", value: "Search media", diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift index 8d74f882c135..93c9bfe4e0a2 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaGridCell.swift @@ -73,8 +73,8 @@ struct MediaGridCell: View { /// the cell. The whole cell rounds at 4pt in aspect-ratio mode (see /// outer `.clipShape` in `body`); this is a small deviation from V1 /// which rounds only the inner image container, but the alternative - /// caused a much worse rendering bug. See `SiteMediaCollectionCell.swift:113-125` - /// for the V1 reference. + /// caused a much worse rendering bug. See `SiteMediaCollectionCell` for + /// the V1 reference. @ViewBuilder private var imageContent: some View { if isAspectRatioMode { CachedAsyncImage(url: item.thumbnailURL) { image in diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift index 4257915dceae..8c69365a2004 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryViewModel.swift @@ -243,14 +243,13 @@ final class MediaLibraryViewModel: ObservableObject { /// Reads the current snapshot from the collection and updates the /// published state. Errors from `collection.loadItems()` (a SQLite read) - /// are logged only — never surfaced via `show(error:)`. This is the - /// observer-loop path's behavior carried forward from M1, and is shared + /// are logged only — never surfaced via `show(error:)`. This is shared /// with the user-visible paths (`fetchPageOne`, `loadNextPage`) for /// simplicity. A SQLite-read failure after a successful network refresh /// or pagination would currently show stale rows with only a log line; - /// M3+ should split the user-visible callers off so they can surface - /// the error to `show(error:)`. The cache-corruption failure mode is - /// rare enough that this trade-off is acceptable for M2. + /// a future change could split the user-visible callers off so they can + /// surface the error to `show(error:)`. The cache-corruption failure + /// mode is rare enough that this trade-off is acceptable for now. private func loadItems( from collection: MediaMetadataCollectionWithEditContext, snapshot: UInt64 diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift index c918dc86e8cb..aae88708275d 100644 --- a/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaKindTests.swift @@ -24,9 +24,9 @@ struct MediaKindTests { } @Test func payloadAudioMapsToAudio() { - // AudioMediaDetails has 13 init parameters per wp_api.swift:32603 — - // fileSize, length (UInt64), lengthFormatted, plus ten optional - // metadata fields. Pass minimal valid values. + // AudioMediaDetails has 13 init parameters: fileSize, length (UInt64), + // lengthFormatted, plus ten optional metadata fields. Pass minimal + // valid values. let details = AudioMediaDetails( fileSize: 0, length: 0, diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift index 0df7075b39cc..f647268a46d5 100644 --- a/Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift +++ b/Modules/Tests/WordPressMediaLibraryTests/MediaThumbnailURLTests.swift @@ -9,8 +9,8 @@ struct MediaThumbnailURLTests { } private func scaled(_ urlString: String) -> ScaledImageDetails { - // ScaledImageDetails init signature per wp_api.swift:65820 — - // file / width / height / sourceUrl (no mimeType). + // ScaledImageDetails init signature: file / width / height / sourceUrl + // (no mimeType). ScaledImageDetails(file: "x", width: 0, height: 0, sourceUrl: urlString) } From 18ef2952e8e4093edc24387d114f66b005524ca2 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 15 May 2026 10:02:50 +1200 Subject: [PATCH 21/21] Simplify grid view layout --- .../Models/MediaGridLayoutMath.swift | 27 ---------- .../Views/MediaLibraryView.swift | 45 +++++++++-------- .../MediaGridLayoutMathTests.swift | 50 ------------------- 3 files changed, 24 insertions(+), 98 deletions(-) delete mode 100644 Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift delete mode 100644 Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift diff --git a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift b/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift deleted file mode 100644 index 977213effc62..000000000000 --- a/Modules/Sources/WordPressMediaLibrary/Models/MediaGridLayoutMath.swift +++ /dev/null @@ -1,27 +0,0 @@ -import Foundation -import SwiftUI - -/// Pure layout math for the Media Library grid. Mirrors V1's -/// `SiteMediaCollectionViewController.updateFlowLayoutItemSize`: 4 cells per -/// row below 500pt width, 5 otherwise; spacing 2 in default mode, 8 in -/// aspect-ratio mode; cell size rounded down to avoid fractional pixels. -struct MediaGridLayoutMath { - let availableWidth: CGFloat - let isAspectRatioMode: Bool - - var spacing: CGFloat { isAspectRatioMode ? 8 : 2 } - - var itemsPerRow: Int { availableWidth < 500 ? 4 : 5 } - - var cellSize: CGFloat { - let raw = (availableWidth - spacing * CGFloat(itemsPerRow - 1)) / CGFloat(itemsPerRow) - return raw.rounded(.down) - } - - var columns: [GridItem] { - Array( - repeating: GridItem(.fixed(cellSize), spacing: spacing), - count: itemsPerRow - ) - } -} diff --git a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift index e6b59984789e..ffe4196b96a8 100644 --- a/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift +++ b/Modules/Sources/WordPressMediaLibrary/Views/MediaLibraryView.swift @@ -4,6 +4,7 @@ struct MediaLibraryView: View { @ObservedObject var viewModel: MediaLibraryViewModel let tracker: any MediaTracker + @Environment(\.horizontalSizeClass) private var sizeClass @State private var searchText: String = "" /// Incremented by the Retry button. A `.task(id: retryToken)` modifier /// listens for changes; the initial value 0 is ignored so we don't @@ -14,32 +15,34 @@ struct MediaLibraryView: View { /// outlive view dismantle. @State private var retryToken: Int = 0 + private var spacing: CGFloat { viewModel.isAspectRatioModeEnabled ? 8 : 2 } + + private var columns: [GridItem] { + Array( + repeating: GridItem(.flexible(), spacing: spacing), + count: sizeClass == .regular ? 5 : 4 + ) + } + var body: some View { - GeometryReader { proxy in - let layout = MediaGridLayoutMath( - availableWidth: proxy.size.width, - isAspectRatioMode: viewModel.isAspectRatioModeEnabled - ) - ScrollView { - LazyVGrid(columns: layout.columns, spacing: layout.spacing) { - ForEach(viewModel.items) { item in - MediaGridCell( - item: item, - isAspectRatioMode: viewModel.isAspectRatioModeEnabled - ) - // Cell-scoped pagination trigger. SwiftUI cancels - // this task on cell disappear AND on view dismantle. - .task(id: item.id) { - await viewModel.loadNextPageIfNeeded(after: item) - } + ScrollView { + LazyVGrid(columns: columns, spacing: spacing) { + ForEach(viewModel.items) { item in + MediaGridCell( + item: item, + isAspectRatioMode: viewModel.isAspectRatioModeEnabled + ) + // Cell-scoped pagination trigger. SwiftUI cancels + // this task on cell disappear AND on view dismantle. + .task(id: item.id) { + await viewModel.loadNextPageIfNeeded(after: item) } } - // V1 parity: top inset matches the spacing. - .padding(.top, layout.spacing) - .animation(.default, value: viewModel.isAspectRatioModeEnabled) } - .refreshable { await viewModel.refresh(pullToRefresh: true) } + .padding(.top, spacing) + .animation(.default, value: viewModel.isAspectRatioModeEnabled) } + .refreshable { await viewModel.refresh(pullToRefresh: true) } .task { tracker.track(.mediaLibraryOpened) } .task(id: viewModel.filter) { await viewModel.refresh() } .task(id: viewModel.filter) { await viewModel.handleDataChanges() } diff --git a/Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift b/Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift deleted file mode 100644 index c1f2b4537ad1..000000000000 --- a/Modules/Tests/WordPressMediaLibraryTests/MediaGridLayoutMathTests.swift +++ /dev/null @@ -1,50 +0,0 @@ -import Testing - -@testable import WordPressMediaLibrary - -struct MediaGridLayoutMathTests { - @Test func fourPerRowBelowFiveHundredPoints() { - let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) - #expect(layout.itemsPerRow == 4) - } - - @Test func fivePerRowAtAndAboveFiveHundredPoints() { - let layout = MediaGridLayoutMath(availableWidth: 500, isAspectRatioMode: false) - #expect(layout.itemsPerRow == 5) - } - - @Test func defaultSpacingIsTwo() { - let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) - #expect(layout.spacing == 2) - } - - @Test func aspectRatioSpacingIsEight() { - let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: true) - #expect(layout.spacing == 8) - } - - @Test func cellSizeRoundsDownForPhoneDefault() { - // 390 - 2 * 3 = 384; / 4 = 96.0 - let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) - #expect(layout.cellSize == 96) - } - - @Test func cellSizeRoundsDownForPhoneAspectRatio() { - // 390 - 8 * 3 = 366; / 4 = 91.5 → rounds down to 91 - let layout = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: true) - #expect(layout.cellSize == 91) - } - - @Test func cellSizeRoundsDownForPad() { - // 768 - 2 * 4 = 760; / 5 = 152 - let layout = MediaGridLayoutMath(availableWidth: 768, isAspectRatioMode: false) - #expect(layout.cellSize == 152) - } - - @Test func columnsCountMatchesItemsPerRow() { - let phone = MediaGridLayoutMath(availableWidth: 390, isAspectRatioMode: false) - let pad = MediaGridLayoutMath(availableWidth: 768, isAspectRatioMode: false) - #expect(phone.columns.count == 4) - #expect(pad.columns.count == 5) - } -}