Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace WPImageViewController with LightboxViewController #23922

Merged
merged 18 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
25.7
-----
* [**] Add new lightbox screen for images with modern transitions and enhanced performance [#23922]

25.6
-----
Expand Down
21 changes: 0 additions & 21 deletions WordPress/Classes/Networking/MediaHost+AbstractPost.swift

This file was deleted.

38 changes: 0 additions & 38 deletions WordPress/Classes/Networking/MediaHost+Blog.swift

This file was deleted.

80 changes: 80 additions & 0 deletions WordPress/Classes/Networking/MediaHost+Extensions.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import Foundation
import WordPressMedia

/// Extends `MediaRequestAuthenticator.MediaHost` so that we can easily
/// initialize it from a given `AbstractPost`.
///
extension MediaHost {
init(_ post: AbstractPost) {
self.init(with: post.blog, failure: { error in
// We just associate a post with the underlying error for simpler debugging.
WordPressAppDelegate.crashLogging?.logError(error)
})
}
}

/// Extends `MediaRequestAuthenticator.MediaHost` so that we can easily
/// initialize it from a given `Blog`.
///
extension MediaHost {
enum BlogError: Swift.Error {
case baseInitializerError(error: Error)
}

init(with blog: Blog) {
self.init(with: blog) { error in
// We'll log the error, so we know it's there, but we won't halt execution.
WordPressAppDelegate.crashLogging?.logError(error)
}
}

init(with blog: Blog, failure: (BlogError) -> ()) {
let isAtomic = blog.isAtomic()
self.init(with: blog, isAtomic: isAtomic, failure: failure)
}

init(with blog: Blog, isAtomic: Bool, failure: (BlogError) -> ()) {
self.init(
isAccessibleThroughWPCom: blog.isAccessibleThroughWPCom(),
isPrivate: blog.isPrivate(),
isAtomic: isAtomic,
siteID: blog.dotComID?.intValue,
username: blog.usernameForSite,
authToken: blog.authToken,
failure: { error in
// We just associate a blog with the underlying error for simpler debugging.
failure(BlogError.baseInitializerError(error: error))
}
)
}
}

/// Extends `MediaRequestAuthenticator.MediaHost` so that we can easily
/// initialize it from a given `Blog`.
///
extension MediaHost {
init(_ post: ReaderPost) {
let isAccessibleThroughWPCom = post.isWPCom || post.isJetpack

// This is the only way in which we can obtain the username and authToken here.
// It'd be nice if all data was associated with an account instead, for transparency
// and cleanliness of the code - but this'll have to do for now.

// We allow a nil account in case the user connected only self-hosted sites.
let account = try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)
let username = account?.username
let authToken = account?.authToken

self.init(
isAccessibleThroughWPCom: isAccessibleThroughWPCom,
isPrivate: post.isBlogPrivate,
isAtomic: post.isBlogAtomic,
siteID: post.siteID?.intValue,
username: username,
authToken: authToken,
failure: { error in
WordPressAppDelegate.crashLogging?.logError(error)
}
)
}
}
32 changes: 0 additions & 32 deletions WordPress/Classes/Networking/MediaHost+ReaderPost.swift

This file was deleted.

1 change: 0 additions & 1 deletion WordPress/Classes/System/WordPress-Bridging-Header.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
#import "WPAuthTokenIssueSolver.h"
#import "WPUploadStatusButton.h"
#import "WPError.h"
#import "WPImageViewController.h"
#import "WPStyleGuide+Pages.h"
#import "WPStyleGuide+WebView.h"
#import "WPTableViewHandler.h"
Expand Down
7 changes: 3 additions & 4 deletions WordPress/Classes/Utility/ContentCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,9 @@ struct DefaultContentCoordinator: ContentCoordinator {
}

func displayFullscreenImage(_ image: UIImage) {
let imageViewController = WPImageViewController(image: image)
imageViewController.modalTransitionStyle = .crossDissolve
imageViewController.modalPresentationStyle = .fullScreen
controller?.present(imageViewController, animated: true)
let lightboxVC = LightboxViewController(.image(image))
lightboxVC.configureZoomTransition()
controller?.present(lightboxVC, animated: true)
}

func displayPlugin(withSlug pluginSlug: String, on siteSlug: String) throws {
Expand Down
14 changes: 12 additions & 2 deletions WordPress/Classes/Utility/Media/AsyncImageView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ final class AsyncImageView: UIView {
private let imageView = GIFImageView()
private var errorView: UIImageView?
private var spinner: UIActivityIndicatorView?
private let controller = ImageViewController()
private let controller = ImageLoadingController()

enum LoadingStyle {
/// Shows a secondary background color during the download.
Expand All @@ -30,6 +30,8 @@ final class AsyncImageView: UIView {

/// By default, `background`.
var loadingStyle = LoadingStyle.background

var passTouchesToSuperview = false
}

var configuration = Configuration() {
Expand Down Expand Up @@ -88,7 +90,7 @@ final class AsyncImageView: UIView {
controller.setImage(with: imageURL, host: host, size: size, completion: completion)
}

private func setState(_ state: ImageViewController.State) {
private func setState(_ state: ImageLoadingController.State) {
imageView.isHidden = true
errorView?.isHidden = true
spinner?.stopAnimating()
Expand Down Expand Up @@ -145,6 +147,14 @@ final class AsyncImageView: UIView {
self.errorView = errorView
return errorView
}

override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {
if configuration.passTouchesToSuperview && self.bounds.contains(point) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was needed to make it work in WPRichTextImage (UIControl, needed to pass gestures to the superview)

// Pass the touch to the superview
return nil
}
return super.hitTest(point, with: event)
}
}

extension GIFImageView {
Expand Down
9 changes: 2 additions & 7 deletions WordPress/Classes/Utility/Media/ImageLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,13 @@ import WordPressMedia
@objc(loadImageWithURL:fromPost:preferredSize:placeholder:success:error:)
func loadImage(with url: URL, from post: AbstractPost, preferredSize size: CGSize = .zero, placeholder: UIImage?, success: ImageLoaderSuccessBlock?, error: ImageLoaderFailureBlock?) {

let host = MediaHost(with: post, failure: { error in
WordPressAppDelegate.crashLogging?.logError(error)
})

let host = MediaHost(post)
loadImage(with: url, from: host, preferredSize: size, placeholder: placeholder, success: success, error: error)
}

@objc(loadImageWithURL:fromReaderPost:preferredSize:placeholder:success:error:)
func loadImage(with url: URL, from readerPost: ReaderPost, preferredSize size: CGSize = .zero, placeholder: UIImage?, success: ImageLoaderSuccessBlock?, error: ImageLoaderFailureBlock?) {

let host = MediaHost(with: readerPost)
loadImage(with: url, from: host, preferredSize: size, placeholder: placeholder, success: success, error: error)
loadImage(with: url, from: MediaHost(readerPost), preferredSize: size, placeholder: placeholder, success: success, error: error)
}

/// Load an image from a specific post, using the given URL. Supports animated images (gifs) as well.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import WordPressMedia

/// A convenience class for managing image downloads for individual views.
@MainActor
final class ImageViewController {
final class ImageLoadingController {
var downloader: ImageDownloader = .shared
var service: MediaImageService = .shared
var onStateChanged: (State) -> Void = { _ in }

private(set) var task: Task<Void, Never>?
Expand Down Expand Up @@ -61,4 +62,29 @@ final class ImageViewController {
}
}
}

func setImage(
with media: Media,
size: MediaImageService.ImageSize
) {
task?.cancel()

if let image = service.getCachedThumbnail(for: .init(media), size: size) {
onStateChanged(.success(image))
} else {
onStateChanged(.loading)
task = Task { @MainActor [service, weak self] in
do {
let image = try await service.image(for: media, size: size)
// This line guarantees that if you cancel on the main thread,
// none of the `onStateChanged` callbacks get called.
guard !Task.isCancelled else { return }
self?.onStateChanged(.success(image))
} catch {
guard !Task.isCancelled else { return }
self?.onStateChanged(.failure(error))
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ struct ImageViewExtensions {
controller.setImage(with: imageURL, host: host, size: size, completion: completion)
}

var controller: ImageViewController {
if let controller = objc_getAssociatedObject(imageView, ImageViewExtensions.controllerKey) as? ImageViewController {
var controller: ImageLoadingController {
if let controller = objc_getAssociatedObject(imageView, ImageViewExtensions.controllerKey) as? ImageLoadingController {
return controller
}
let controller = ImageViewController()
let controller = ImageLoadingController()
controller.onStateChanged = { [weak imageView] in
guard let imageView else { return }
setState($0, for: imageView)
Expand All @@ -44,7 +44,7 @@ struct ImageViewExtensions {
return controller
}

private func setState(_ state: ImageViewController.State, for imageView: UIImageView) {
private func setState(_ state: ImageLoadingController.State, for imageView: UIImageView) {
switch state {
case .loading:
break
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,9 @@ final class BlazePostPreviewView: UIView {

if let url = post.featuredImageURL {
featuredImageView.isHidden = false
let host = MediaHost(with: post, failure: { error in
// We'll log the error, so we know it's there, but we won't halt execution.
WordPressAppDelegate.crashLogging?.logError(error)
})
let preferredSize = CGSize(width: featuredImageView.frame.width, height: featuredImageView.frame.height)
.scaled(by: UITraitCollection.current.displayScale)
featuredImageView.setImage(with: url, host: host, size: preferredSize)
featuredImageView.setImage(with: url, host: MediaHost(post), size: preferredSize)

} else {
featuredImageView.isHidden = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import WordPressShared
import WordPressMedia

final class MediaItemHeaderView: UIView {
private let imageView = CachedAnimatedImageView()
let imageView = CachedAnimatedImageView()
private let errorView = UIImageView()
private let videoIconView = PlayIconView()
private let loadingIndicator = UIActivityIndicatorView(style: .large)
Expand Down
21 changes: 0 additions & 21 deletions WordPress/Classes/ViewRelated/Cells/PostFeaturedImageCell.h

This file was deleted.

Loading