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.

Sign up for GitHub

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

Refactor: Remove ImageLoader usages from Blaze #23891

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Refactor: Remove ImageLoader usages from Blaze #23891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07a4605
Remove ImageLoader usages from Blaze
kean Dec 12, 2024
73f82b3
Update release notes
kean Dec 13, 2024
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
2 changes: 2 additions & 0 deletions RELEASE-NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
* [*] (Hidden under a feature flag) User Management for self-hosted sites. [#23768]
* [*] Enable quick access to notifications from Reader on iPad [#23882]
* [*] Add support for restricted posts in Reader [#23853]
* [*] Fix minor appearance issues in the Blaze campaign list [#23891]
* [*] Improve the sidebar animations and layout on some iPad models [#23886]

25.5
-----
Expand Down
1 change: 1 addition & 0 deletions WordPress/Classes/Utility/Media/AsyncImageView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ final class AsyncImageView: UIView {
}
}

/// - parameter size: Target image size in pixels.
func setImage(
with imageURL: URL,
host: MediaHost? = nil,
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Utility/Media/ImageRequest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ final class ImageRequest {
}

struct ImageRequestOptions {
/// Resize the thumbnail to the given size. By default, `nil`.
/// Resize the thumbnail to the given size (in pixels). By default, `nil`.
var size: CGSize?
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like I have commented about this before... I apologize if you have already answered this question... 😅

I think it's good to express "the given size (in pixels)" in code. Like sizeInPixel: CGSize, or creating a struct PixelSize { pixelWidth, pixelHeight } type.

It's easy to miss documentation, but it's very hard to miss if you have to write "pixel" in code.

Copy link
Contributor Author

@kean kean Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. I already attempted it once by adding an ImageSize struct, but then realized the name was already taken – ugh! I added a note to try it again later 😄

PixelSize sounds. I can introduce it in a separate PR. It could also have a convenience initializer that takes points and multiplies by the display scale.


/// If enabled, uses ``MemoryCache`` for caching decompressed images.
Expand Down
22 changes: 5 additions & 17 deletions WordPress/Classes/ViewRelated/Blaze Campaigns/BlazeCampaignTableViewCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,9 @@ final class BlazeCampaignTableViewCell: UITableViewCell, Reusable {
return stackView
}()

private lazy var featuredImageView: CachedAnimatedImageView = {
let imageView = CachedAnimatedImageView()
private lazy var featuredImageView: AsyncImageView = {
let imageView = AsyncImageView()
imageView.translatesAutoresizingMaskIntoConstraints = false
imageView.contentMode = .scaleAspectFill
imageView.clipsToBounds = true
imageView.layer.cornerRadius = Metrics.featuredImageCornerRadius
return imageView
Expand All @@ -85,12 +84,6 @@ final class BlazeCampaignTableViewCell: UITableViewCell, Reusable {
return imageView
}()

// MARK: - Properties

private lazy var imageLoader: ImageLoader = {
return ImageLoader(imageView: featuredImageView, gifStrategy: .mediumGIFs)
}()

// MARK: - Initializers

override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) {
Expand All @@ -110,15 +103,15 @@ final class BlazeCampaignTableViewCell: UITableViewCell, Reusable {

titleLabel.text = viewModel.title

imageLoader.prepareForReuse()
featuredImageView.prepareForReuse()
featuredImageView.isHidden = viewModel.imageURL == nil
if let imageURL = viewModel.imageURL {
let host = MediaHost(with: blog, failure: { error in
WordPressAppDelegate.crashLogging?.logError(error)
})

let preferredSize = CGSize(width: Metrics.featuredImageSize, height: Metrics.featuredImageSize)
imageLoader.loadImage(with: imageURL, from: host, preferredSize: preferredSize)
.scaled(by: UITraitCollection.current.displayScale)
featuredImageView.setImage(with: imageURL, host: host, size: preferredSize)
}

statsStackView.arrangedSubviews.forEach { $0.removeFromSuperview() }
Expand All @@ -131,11 +124,6 @@ final class BlazeCampaignTableViewCell: UITableViewCell, Reusable {

private func commonInit() {
setupViews()
applyStyle()
}

private func applyStyle() {
backgroundColor = .systemGroupedBackground
}

private func setupViews() {
Expand Down
15 changes: 6 additions & 9 deletions WordPress/Classes/ViewRelated/Blaze/Overlay/BlazePostPreviewView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ final class BlazePostPreviewView: UIView {
return label
}()

private lazy var featuredImageView: CachedAnimatedImageView = {
let imageView = CachedAnimatedImageView()
private lazy var featuredImageView: AsyncImageView = {
let imageView = AsyncImageView()
imageView.translatesAutoresizingMaskIntoConstraints = false

NSLayoutConstraint.activate([
Expand All @@ -66,10 +66,6 @@ final class BlazePostPreviewView: UIView {

private let post: AbstractPost

private lazy var imageLoader: ImageLoader = {
return ImageLoader(imageView: featuredImageView, gifStrategy: .mediumGIFs)
}()

// MARK: - Initializers

init(post: AbstractPost) {
Expand All @@ -95,16 +91,17 @@ final class BlazePostPreviewView: UIView {
}

private func setupFeaturedImage() {
featuredImageView.prepareForReuse()

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)
imageLoader.loadImage(with: url, from: host, preferredSize: preferredSize)
.scaled(by: UITraitCollection.current.displayScale)
featuredImageView.setImage(with: url, host: host, size: preferredSize)

} else {
featuredImageView.isHidden = true
Expand Down
10 changes: 5 additions & 5 deletions ...ress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Blaze/DashboardBlazeCampaignView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ import WordPressKit
final class DashboardBlazeCampaignView: UIView {
private let statusView = BlazeCampaignStatusView()
private let titleLabel = UILabel()
private let imageView = CachedAnimatedImageView()
private let imageView = AsyncImageView()

private lazy var statsView: UIStackView = {
let stackView = UIStackView()
stackView.distribution = .fillEqually
return stackView
}()

private lazy var imageLoader = ImageLoader(imageView: imageView)

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

Expand Down Expand Up @@ -59,13 +57,15 @@ final class DashboardBlazeCampaignView: UIView {

titleLabel.text = viewModel.title

imageLoader.prepareForReuse()
imageView.prepareForReuse()
imageView.isHidden = viewModel.imageURL == nil
if let imageURL = viewModel.imageURL {
let host = MediaHost(with: blog, failure: { error in
WordPressAppDelegate.crashLogging?.logError(error)
})
imageLoader.loadImage(with: imageURL, from: host, preferredSize: Constants.imageSize)
let targetSize = Constants.imageSize
.scaled(by: UITraitCollection.current.displayScale)
imageView.setImage(with: imageURL, host: host, size: targetSize)
}

statsView.isHidden = !viewModel.isShowingStats
Expand Down