Skip to content

Commit

Permalink
Reader: Enhance cover image (#23897)
Browse files Browse the repository at this point in the history
* Replace DimensionFetcher usage

* Remove ImageLoader usage from ReaderDetailFeaturedImageView

* Remove ImageLoader from StatsLatestPostSummaryInsightsCell

* Remove ImageDimensionsFetcher usage

* Remove unused useCompatibilityMode

* Move reading preferences to the more menu

* Add a bit more spacing for posts in Reader

* Refine AsyncImageView API

* Fix dark/light transition on scroll

* Update release notes
  • Loading branch information
kean authored and jkmassel committed Jan 16, 2025
1 parent b81af25 commit 1880073
Show file tree
Hide file tree
Showing 16 changed files with 134 additions and 154 deletions.
2 changes: 2 additions & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* [*] Fix minor appearance issues in the Blaze campaign list [#23891]
* [*] Improve the sidebar animations and layout on some iPad models [#23886]
* [*] Fix an issue with posts shown embedded in the notifications popover on iPad [#23889]
* [*] The post cover now uses the standard aspect ratio for covers, so there is no jumping. There are also a few minor improvements to the layout and animations of the cover [#23897]
* [*] Move the "Reading Preferences" button to the "More" menu [#23897]

25.5
-----
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Extensions/Interpolation.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Foundation
import UIKit

extension CGFloat {
/// Interpolates a CGFloat
Expand Down
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()
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

0 comments on commit 1880073

Please sign in to comment.