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

Reader: Enhance cover image #23897

Merged
merged 11 commits into from
Dec 17, 2024
8 changes: 2 additions & 6 deletions WordPress/Classes/Networking/MediaHost+ReaderPost.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import Foundation
/// initialize it from a given `Blog`.
///
extension MediaHost {
enum ReaderPostError: Swift.Error {
case baseInitializerError(error: Error)
}

init(with post: ReaderPost, failure: (ReaderPostError) -> ()) {
init(with post: ReaderPost) {
let isAccessibleThroughWPCom = post.isWPCom || post.isJetpack

// This is the only way in which we can obtain the username and authToken here.
Expand All @@ -28,7 +24,7 @@ extension MediaHost {
username: username,
authToken: authToken,
failure: { error in
failure(ReaderPostError.baseInitializerError(error: error))
WordPressAppDelegate.crashLogging?.logError(error)
}
)
}
Expand Down
79 changes: 62 additions & 17 deletions WordPress/Classes/Utility/Media/AsyncImageView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,59 @@ import Gifu
/// (see ``AnimatedImage``).
@MainActor
final class AsyncImageView: UIView {
let imageView = GIFImageView()

private let imageView = GIFImageView()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I enhanced the API so that the implementation details (GIFImageView) don't leak and AsyncImageView has full control over the image. The image property will now also correctly support AnimagedImage, both setter and getter.

private var errorView: UIImageView?
private var spinner: UIActivityIndicatorView?
private let controller = ImageViewController()

enum LoadingStyle {
/// Shows a secondary background color during the download.
case background
/// Shows a spinner during the download.
case spinner
}

var isErrorViewEnabled = true
var loadingStyle = LoadingStyle.background
struct Configuration {
/// Image tint color.
var tintColor: UIColor?

/// Image view content mode.
var contentMode: UIView.ContentMode?

/// Enabled by default and shows an error icon on failures.
var isErrorViewEnabled = true

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

var configuration = Configuration() {
didSet { didUpdateConfiguration(configuration) }
}

/// The currently displayed image. If the image is animated, returns an
/// instance of ``AnimatedImage``.
var image: UIImage? {
didSet {
if let image {
imageView.configure(image: image)
} else {
imageView.prepareForReuse()
}
}
}

override init(frame: CGRect) {
super.init(frame: frame)
setupView()
}

required init?(coder: NSCoder) {
super.init(coder: coder)
setupView()
}

private func setupView() {
controller.onStateChanged = { [weak self] in self?.setState($0) }

addSubview(imageView)
Expand All @@ -35,18 +71,10 @@ final class AsyncImageView: UIView {
backgroundColor = .secondarySystemBackground
}

required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

/// Removes the current image and stops the outstanding downloads.
func prepareForReuse() {
controller.prepareForReuse()

if imageView.isAnimatingGIF {
imageView.prepareForReuse()
} else {
imageView.image = nil
}
image = nil
}

/// - parameter size: Target image size in pixels.
Expand All @@ -66,23 +94,32 @@ final class AsyncImageView: UIView {

switch state {
case .loading:
switch loadingStyle {
switch configuration.loadingStyle {
case .background:
backgroundColor = .secondarySystemBackground
case .spinner:
makeSpinner().startAnimating()
}
case .success(let image):
imageView.configure(image: image)
self.image = image
imageView.isHidden = false
backgroundColor = .clear
case .failure:
if isErrorViewEnabled {
if configuration.isErrorViewEnabled {
makeErrorView().isHidden = false
}
}
}

private func didUpdateConfiguration(_ configuration: Configuration) {
if let tintColor = configuration.tintColor {
imageView.tintColor = tintColor
}
if let contentMode = configuration.contentMode {
imageView.contentMode = contentMode
}
}

private func makeSpinner() -> UIActivityIndicatorView {
if let spinner {
return spinner
Expand Down Expand Up @@ -119,4 +156,12 @@ extension GIFImageView {
self.image = image
}
}

private func prepareForReuse() {
if isAnimatingGIF {
prepareForReuse()
} else {
image = nil
}
}
}
5 changes: 1 addition & 4 deletions WordPress/Classes/Utility/Media/ImageLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,7 @@ import WordPressShared
@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, failure: { error in
WordPressAppDelegate.crashLogging?.logError(error)
})

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,7 @@ private extension RichCommentContentRenderer {
WordPressAppDelegate.crashLogging?.logError(error)
})
} else if let post = comment.post as? ReaderPost, post.isBlogPrivate {
return 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)
})
return MediaHost(with: post)
}

return .publicSite
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private final class ReaderPostCellView: UIView {
avatarView.centerYAnchor.constraint(equalTo: timeLabel.centerYAnchor),
avatarView.trailingAnchor.constraint(equalTo: headerView.leadingAnchor, constant: -8),

headerView.topAnchor.constraint(equalTo: topAnchor, constant: 4),
headerView.topAnchor.constraint(equalTo: topAnchor, constant: 6),
headerView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: insets.left),
headerView.trailingAnchor.constraint(lessThanOrEqualTo: trailingAnchor, constant: -50),

Expand Down Expand Up @@ -391,7 +391,7 @@ private func makeButton(systemImage: String, font: UIFont = UIFont.preferredFont
configuration.imagePadding = 6
configuration.preferredSymbolConfigurationForImage = UIImage.SymbolConfiguration(font: font)
configuration.baseForegroundColor = .secondaryLabel
configuration.contentInsets = .init(top: 16, leading: 12, bottom: 14, trailing: 12)
configuration.contentInsets = .init(top: 16, leading: 12, bottom: 16, trailing: 12)

let button = UIButton(configuration: configuration)
if #available(iOS 17.0, *) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,20 +481,12 @@ class ReaderDetailCoordinator {

/// Show the featured image fullscreen
///
private func showFeaturedImage(_ sender: CachedAnimatedImageView) {
guard let post else {
return
}

var controller: WPImageViewController
if post.featuredImageURL.isGif, let data = sender.animatedGifData {
controller = WPImageViewController(gifData: data)
} else if let featuredImage = sender.image {
controller = WPImageViewController(image: featuredImage)
} else {
private func showFeaturedImage(_ sender: AsyncImageView) {
guard let post, let imageURL = post.featuredImage.flatMap(URL.init) else {
return
}

let controller = WPImageViewController(url: imageURL)
controller.readerPost = post
controller.modalTransitionStyle = .crossDissolve
controller.modalPresentationStyle = .fullScreen
viewController?.present(controller, animated: true)
Expand Down Expand Up @@ -722,7 +714,7 @@ extension ReaderDetailCoordinator: ReaderDetailHeaderViewDelegate {

// MARK: - ReaderDetailFeaturedImageViewDelegate
extension ReaderDetailCoordinator: ReaderDetailFeaturedImageViewDelegate {
func didTapFeaturedImage(_ sender: CachedAnimatedImageView) {
func didTapFeaturedImage(_ sender: AsyncImageView) {
showFeaturedImage(sender)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,6 @@ class ReaderDetailViewController: UIViewController, ReaderDetailView {

private func configureHeader() {
header.displaySetting = displaySetting
header.useCompatibilityMode = useCompatibilityMode
header.delegate = coordinator
headerContainerView.addSubview(header)
headerContainerView.translatesAutoresizingMaskIntoConstraints = false
Expand Down Expand Up @@ -719,7 +718,7 @@ class ReaderDetailViewController: UIViewController, ReaderDetailView {
coordinator?.openInBrowser()
}

@objc func didTapDisplaySettingButton(_ sender: UIBarButtonItem) {
@objc func didTapDisplaySettingButton() {
let viewController = ReaderDisplaySettingViewController(initialSetting: displaySetting,
source: .readerPostNavBar) { [weak self] newSetting in
// no need to refresh if there are no changes to the display setting.
Expand Down Expand Up @@ -1051,8 +1050,7 @@ private extension ReaderDetailViewController {
let rightItems = [
moreButtonItem(enabled: enableRightBarButtons),
shareButtonItem(enabled: enableRightBarButtons),
safariButtonItem(),
displaySettingButtonItem()
safariButtonItem()
]
navigationItem.largeTitleDisplayMode = .never
navigationItem.rightBarButtonItems = rightItems.compactMap({ $0 })
Expand Down Expand Up @@ -1095,17 +1093,6 @@ private extension ReaderDetailViewController {
dismiss(animated: true)
}

func displaySettingButtonItem() -> UIBarButtonItem? {
guard ReaderDisplaySetting.customizationEnabled,
let icon = UIImage(named: "reader-reading-preferences") else {
return nil
}
let button = barButtonItem(with: icon, action: #selector(didTapDisplaySettingButton(_:)))
button.accessibilityLabel = Strings.displaySettingAccessibilityLabel

return button
}

func safariButtonItem() -> UIBarButtonItem? {
let button = barButtonItem(with: .gridicon(.globe), action: #selector(didTapBrowserButton(_:)))
button.accessibilityLabel = Strings.safariButtonAccessibilityLabel
Expand All @@ -1132,12 +1119,20 @@ private extension ReaderDetailViewController {
guard let post else {
return []
}
return ReaderPostMenu(
var elements = ReaderPostMenu(
post: post,
topic: nil,
anchor: anchor,
viewController: self
).makeMenu()

if ReaderDisplaySetting.customizationEnabled {
elements.append(UIAction(title: Strings.displaySettingsLabel, image: UIImage(systemName: "textformat.size")) { [weak self] _ in
self?.didTapDisplaySettingButton()
})
}

return elements
}

func shareButtonItem(enabled: Bool = true) -> UIBarButtonItem? {
Expand Down Expand Up @@ -1186,8 +1181,8 @@ extension ReaderDetailViewController {
value: "Dismiss",
comment: "Spoken accessibility label"
)
static let displaySettingAccessibilityLabel = NSLocalizedString(
"readerDetail.displaySettingButton.accessibilityLabel",
static let displaySettingsLabel = NSLocalizedString(
"readerDetail.displaySettingButton.displaySettingsLabel",
value: "Reading Preferences",
comment: "Spoken accessibility label for the Reading Preferences menu.")
static let safariButtonAccessibilityLabel = NSLocalizedString(
Expand Down
Loading