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: Fix cropped recommended sites #23802

Merged
merged 8 commits into from
Nov 14, 2024
Merged
Changes from 1 commit
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
Next Next commit
Integrate new ReaderRecommendedSitesCell
kean committed Nov 14, 2024
commit d9eaad590b8c83f23c2138e44796ffc0b5fcc0ab
Original file line number Diff line number Diff line change
@@ -40,7 +40,12 @@ struct SiteIconViewModel {
init(readerSiteTopic: ReaderSiteTopic, size: Size = .regular) {
self.size = size
self.firstLetter = readerSiteTopic.title.first
self.imageURL = SiteIconViewModel.makeReaderSiteIconURL(iconURL: readerSiteTopic.siteBlavatar, siteID: readerSiteTopic.siteID.intValue, size: size.size)
let scale = UITraitCollection.current.displayScale
self.imageURL = SiteIconViewModel.makeReaderSiteIconURL(
iconURL: readerSiteTopic.siteBlavatar,
siteID: readerSiteTopic.siteID.intValue,
size: size.size.scaled(by: scale)
)
}
}

@@ -113,6 +118,7 @@ extension SiteIconViewModel {

extension SiteIconViewModel {
/// - parameter isBlavatar: A hint to skip the "is icon blavatar" check.
/// - parameter size: Size in pixels.
static func makeReaderSiteIconURL(iconURL: String?, isBlavatar: Bool = false, siteID: Int?, size: CGSize) -> URL? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: may as well rename the label size: -> pixelSize: 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually size in points 🤦

This is how it's used:

  static func optimizedBlavatarURL(from path: String, imageSize: CGSize) -> URL? {
        let size = imageSize.scaled(by: UITraitCollection.current.displayScale)
        let query = String(format: "d=404&s=%d", Int(max(size.width, size.height)))
        return parseURL(path: path, query: query)
    }

I'm going to revert it.

Yeah, it's a low-level method. The app uses SiteIconViewModel.Size enum in most cases. It also ensures that you don't create too many different variants of the the same icon but off by a few points.

guard let iconURL, !iconURL.isEmpty else {
if let siteID {
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import SwiftUI
import UIKit
import WordPressUI

final class ReaderRecommendedSitesCell: UITableViewCell {
private let sitesStackView = UIStackView(axis: .vertical, spacing: 16, [])

override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) {
super.init(style: style, reuseIdentifier: reuseIdentifier)

setupView()
}

required init?(coder: NSCoder) {
fatalError("Not implemented")
}

override func prepareForReuse() {
super.prepareForReuse()

for view in sitesStackView.subviews {
view.removeFromSuperview()
}
}

private func setupView() {
selectionStyle = .none

let backgroundView = UIView()
backgroundView.backgroundColor = .secondarySystemBackground
backgroundView.layer.cornerRadius = 8

contentView.addSubview(backgroundView)
backgroundView.pinEdges(insets: UIEdgeInsets(horizontal: 16, vertical: 0))

let titleLabel = UILabel()
titleLabel.font = .preferredFont(forTextStyle: .subheadline)
titleLabel.textColor = .secondaryLabel
titleLabel.text = Strings.title

let stackView = UIStackView(axis: .vertical, spacing: 16, [titleLabel, sitesStackView])

backgroundView.addSubview(stackView)
stackView.pinEdges(insets: {
var insets = UIEdgeInsets(.all, 16)
insets.right = 6 // Buttons insets take care of it
return insets
}())
}

func configure(with sites: [ReaderSiteTopic]) {
for site in sites {
let siteView = makeSiteView(for: site)
sitesStackView.addArrangedSubview(siteView)
}
}

private func makeSiteView(for site: ReaderSiteTopic) -> UIView {
let view = ReaderRecommendedSitesCellView()
view.configure(with: site)
return view
}
}

/// Presentation-agnostic view for displaying post cells.
private final class ReaderRecommendedSitesCellView: UIView {
let siteIconView = SiteIconHostingView()
let titleLabel = UILabel()
let subtitleLabel = UILabel()
let buttonSubscribe = UIButton(configuration: {
var configuration = UIButton.Configuration.plain()
configuration.image = UIImage(systemName: "plus.circle")
configuration.baseForegroundColor = UIAppColor.brand
configuration.contentInsets = .zero
return configuration
}())

private let iconSize: SiteIconViewModel.Size = .regular

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

titleLabel.font = .preferredFont(forTextStyle: .callout).withWeight(.medium)
subtitleLabel.font = .preferredFont(forTextStyle: .footnote)
subtitleLabel.textColor = .secondaryLabel

NSLayoutConstraint.activate([
siteIconView.widthAnchor.constraint(equalToConstant: iconSize.width),
siteIconView.heightAnchor.constraint(equalToConstant: iconSize.width),
])

NSLayoutConstraint.activate([
buttonSubscribe.widthAnchor.constraint(equalToConstant: 40),
buttonSubscribe.heightAnchor.constraint(equalToConstant: 40),
])

buttonSubscribe.setContentCompressionResistancePriority(.required, for: .horizontal)

let stackView = UIStackView(alignment: .center, spacing: 6, [
siteIconView,
UIStackView(axis: .vertical, alignment: .leading, spacing: 2, [
titleLabel, subtitleLabel,
]),
buttonSubscribe
])
stackView.setCustomSpacing(14, after: siteIconView)
addSubview(stackView)
stackView.pinEdges()

buttonSubscribe.addTarget(self, action: #selector(buttonSubscribeTapped), for: .touchUpInside)
}

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

func configure(with site: ReaderSiteTopic) {
siteIconView.setIcon(with: .init(readerSiteTopic: site, size: iconSize))
titleLabel.text = site.title
subtitleLabel.text = site.siteDescription
}

@objc private func buttonSubscribeTapped() {
// TODO: implement
}
}

private enum Strings {
static let title = NSLocalizedString("reader.suggested.blogs.title", value: "Blogs to subscribe to", comment: "A suggestion of topics the user might want to subscribe to")
}
Original file line number Diff line number Diff line change
@@ -196,7 +196,7 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController {
// all the cell types have been registered by that time.
// see: https://github.com/wordpress-mobile/WordPress-iOS/pull/23368
tableView.register(ReaderTopicsCardCell.defaultNib, forCellReuseIdentifier: readerCardTopicsIdentifier)
tableView.register(ReaderSitesCardCell.self, forCellReuseIdentifier: readerCardSitesIdentifier)
tableView.register(ReaderRecommendedSitesCell.self, forCellReuseIdentifier: readerCardSitesIdentifier)
}

required init?(coder: NSCoder) {
@@ -263,9 +263,10 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController {
}

func cell(for sites: [ReaderSiteTopic]) -> UITableViewCell {
let cell = tableView.dequeueReusableCell(withIdentifier: readerCardSitesIdentifier) as! ReaderSitesCardCell
cell.configure(sites)
cell.delegate = self
let cell = tableView.dequeueReusableCell(withIdentifier: readerCardSitesIdentifier) as! ReaderRecommendedSitesCell
cell.configure(with: sites)
// TODO: implement delegate
// cell.delegate = self
hideSeparator(for: cell)
return cell
}