From 4bc0cb647ad4dd626cfb2df004ebad00d0a457bf Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Wed, 13 Nov 2024 20:25:40 -0500 Subject: [PATCH 01/28] Fix background color for site icon placeholers (#23798) --- .../Reader/Sidebar/ReaderSidebarViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift index 016e0fee79d0..c24a611e661c 100644 --- a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift @@ -113,10 +113,10 @@ private struct ReaderSidebarView: View { .withDisabledSelection(isEditing) ReaderSidebarSubscriptionsSection(viewModel: viewModel) + .environment(\.siteIconBackgroundColor, Color(viewModel.isCompact ? .secondarySystemBackground : .systemBackground)) } makeSection(Strings.lists, isExpanded: $isSectionListsExpanded) { ReaderSidebarListsSection(viewModel: viewModel) - .environment(\.siteIconBackgroundColor, Color(viewModel.isCompact ? .secondarySystemBackground : .systemBackground)) } makeSection(Strings.tags, isExpanded: $isSectionTagsExpanded) { ReaderSidebarTagsSection(viewModel: viewModel) From a3aede2bf8496044eb3a7cb531eb1143f6871d91 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Wed, 13 Nov 2024 20:59:33 -0500 Subject: [PATCH 02/28] Reader: Fix cropped recommended sites (#23802) * Integrate new ReaderRecommendedSitesCell * Implement subscribe/unsubscribe flow * Implement show details * Remove ReaderRecommendedSiteCardCell * Remove ReaderSitesCardCell * Remove ReaderTopicsTableCardCell * Add vertical spacing for recommended sites * Revert size change --- .../BlogList/SiteIconViewModel.swift | 7 +- .../Cards/ReaderRecommendedSiteCardCell.swift | 95 ---------- .../Cards/ReaderRecommendedSiteCardCell.xib | 116 ------------ .../Cards/ReaderRecommendedSitesCell.swift | 170 +++++++++++++++++ .../ReaderDiscoverViewController.swift | 21 +-- .../Controllers/ReaderSitesCardCell.swift | 73 -------- .../ReaderStreamViewController.swift | 17 +- .../Controllers/ReaderTableCardCell.swift | 175 ------------------ .../Controllers/ReaderTopicsCardCell.swift | 2 +- .../ReaderSubscriptionHelper.swift | 17 +- 10 files changed, 199 insertions(+), 494 deletions(-) delete mode 100644 WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.swift delete mode 100644 WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.xib create mode 100644 WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift delete mode 100644 WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSitesCardCell.swift delete mode 100644 WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableCardCell.swift diff --git a/WordPress/Classes/ViewRelated/Blog/Site Picker/BlogList/SiteIconViewModel.swift b/WordPress/Classes/ViewRelated/Blog/Site Picker/BlogList/SiteIconViewModel.swift index 37767373ceaa..c36e37817f45 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Picker/BlogList/SiteIconViewModel.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Picker/BlogList/SiteIconViewModel.swift @@ -40,7 +40,11 @@ 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) + self.imageURL = SiteIconViewModel.makeReaderSiteIconURL( + iconURL: readerSiteTopic.siteBlavatar, + siteID: readerSiteTopic.siteID.intValue, + size: size.size + ) } } @@ -113,6 +117,7 @@ extension SiteIconViewModel { extension SiteIconViewModel { /// - parameter isBlavatar: A hint to skip the "is icon blavatar" check. + /// - parameter size: Size in points. static func makeReaderSiteIconURL(iconURL: String?, isBlavatar: Bool = false, siteID: Int?, size: CGSize) -> URL? { guard let iconURL, !iconURL.isEmpty else { if let siteID { diff --git a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.swift b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.swift deleted file mode 100644 index 52bbd98c2202..000000000000 --- a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.swift +++ /dev/null @@ -1,95 +0,0 @@ -import UIKit - -class ReaderRecommendedSiteCardCell: UITableViewCell { - @IBOutlet weak var iconImageView: UIImageView! - @IBOutlet weak var blogNameLabel: UILabel! - @IBOutlet weak var hostNameLabel: UILabel! - @IBOutlet weak var followButton: UIButton! - @IBOutlet weak var descriptionLabel: UILabel? - @IBOutlet weak var headerStackView: UIStackView! - - weak var delegate: ReaderRecommendedSitesCardCellDelegate? - - override func awakeFromNib() { - super.awakeFromNib() - - applyStyles() - } - - func configure(_ topic: ReaderSiteTopic) { - separatorInset = UIEdgeInsets.zero - - followButton.isSelected = topic.following - blogNameLabel.text = topic.title - hostNameLabel.text = URL(string: topic.siteURL)?.host - descriptionLabel?.text = topic.siteDescription - descriptionLabel?.isHidden = topic.siteDescription.isEmpty - - configureSiteIcon(topic) - configureFollowButtonVisibility() - - applyStyles() - } - - @IBAction func didTapFollowButton(_ sender: Any) { - // Optimistically change the value - followButton.isSelected = !followButton.isSelected - - applyFollowButtonStyles() - configureFollowButtonVisibility() - - delegate?.handleFollowActionForCell(self) - } - - private func configureFollowButtonVisibility() { - let isLoggedIn = ReaderHelpers.isLoggedIn() - followButton.isHidden = !isLoggedIn - } - - private func configureSiteIcon(_ topic: ReaderSiteTopic) { - let placeholder = UIImage.siteIconPlaceholder - - guard - !topic.siteBlavatar.isEmpty, - let url = URL(string: topic.siteBlavatar) - else { - iconImageView.image = placeholder - return - } - - iconImageView.downloadImage(from: url, placeholderImage: placeholder) - } - - override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { - configureFollowButtonVisibility() - - if traitCollection.hasDifferentColorAppearance(comparedTo: previousTraitCollection) { - applyFollowButtonStyles() - } - } - - private func applyStyles() { - backgroundColor = .secondarySystemGroupedBackground - - // Blog Name - blogNameLabel.font = WPStyleGuide.fontForTextStyle(.subheadline, fontWeight: .semibold) - blogNameLabel.textColor = .label - - // Host Label - hostNameLabel.font = WPStyleGuide.fontForTextStyle(.footnote) - hostNameLabel.textColor = .secondaryLabel - - applyFollowButtonStyles() - headerStackView.spacing = 12.0 - } - - private func applyFollowButtonStyles() { - let contentInsets = NSDirectionalEdgeInsets(top: 8.0, leading: 16.0, bottom: 8.0, trailing: 16.0) - WPStyleGuide.applyReaderFollowButtonStyle(followButton, contentInsets: contentInsets) - } - -} - -protocol ReaderRecommendedSitesCardCellDelegate: AnyObject { - func handleFollowActionForCell(_ cell: ReaderRecommendedSiteCardCell) -} diff --git a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.xib b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.xib deleted file mode 100644 index 17fa5f550dff..000000000000 --- a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSiteCardCell.xib +++ /dev/null @@ -1,116 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift new file mode 100644 index 000000000000..e448c6447791 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift @@ -0,0 +1,170 @@ +import SwiftUI +import UIKit +import WordPressUI +import Combine + +protocol ReaderRecommendedSitesCellDelegate: AnyObject { + func didSelect(topic: ReaderAbstractTopic) +} + +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(.all, 16)) + + 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], delegate: ReaderRecommendedSitesCellDelegate) { + for site in sites { + let siteView = ReaderRecommendedSitesCellView() + siteView.configure(with: site) + siteView.delegate = delegate + sitesStackView.addArrangedSubview(siteView) + } + } +} + +/// Presentation-agnostic view for displaying post cells. +private final class ReaderRecommendedSitesCellView: UIView { + let siteIconView = SiteIconHostingView() + let titleLabel = UILabel() + let subtitleLabel = UILabel() + let buttonShowDetails = UIButton(type: .system) + let buttonSubscribe = UIButton(configuration: { + var configuration = UIButton.Configuration.plain() + configuration.image = UIImage(systemName: "plus.circle") + configuration.baseForegroundColor = UIAppColor.brand + configuration.contentInsets = .zero + return configuration + }()) + + weak var delegate: ReaderRecommendedSitesCellDelegate? + + private let iconSize: SiteIconViewModel.Size = .regular + private var site: ReaderSiteTopic? + private var cancellable: AnyCancellable? + + 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() + + stackView.addSubview(buttonShowDetails) + buttonShowDetails.pinEdges(insets: UIEdgeInsets(.trailing, 40)) + + buttonShowDetails.addTarget(self, action: #selector(buttonShowDetailsTapped), for: .touchUpInside) + + buttonSubscribe.addTarget(self, action: #selector(buttonSubscribeTapped), for: .touchUpInside) + } + + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + func configure(with site: ReaderSiteTopic) { + self.site = site + + siteIconView.setIcon(with: .init(readerSiteTopic: site, size: iconSize)) + titleLabel.text = site.title + subtitleLabel.text = site.siteDescription + + cancellable = site.publisher(for: \.following, options: [.initial, .new]) + .sink { [weak self] isFollowing in + self?.buttonSubscribe.configuration?.image = UIImage(systemName: isFollowing ? "checkmark.circle.fill" : "plus.circle") + } + } + + @objc private func buttonShowDetailsTapped() { + guard let site else { + return wpAssertionFailure("site missing") + } + delegate?.didSelect(topic: site) + } + + @objc private func buttonSubscribeTapped() { + guard let site else { + return wpAssertionFailure("site missing") + } + + var properties = [String: Any]() + properties[WPAppAnalyticsKeyFollowAction] = !site.following + properties[WPAppAnalyticsKeyBlogID] = site.siteID + + WPAnalytics.trackReader(.readerSuggestedSiteToggleFollow, properties: properties) + + buttonSubscribe.configuration?.showsActivityIndicator = true + buttonSubscribe.configuration?.baseForegroundColor = .secondaryLabel + ReaderSubscriptionHelper().toggleFollowingForSite(site) { [weak self] _ in + self?.buttonSubscribe.configuration?.showsActivityIndicator = false + self?.buttonSubscribe.configuration?.baseForegroundColor = UIAppColor.brand + } + } +} + +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") +} diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift index d56795982d20..57387fb37f7a 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift @@ -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,8 @@ 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, delegate: self) hideSeparator(for: cell) return cell } @@ -366,9 +365,9 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController { } } -// MARK: - ReaderTopicsTableCardCellDelegate +// MARK: - ReaderRecommendedSitesCellDelegate -extension ReaderDiscoverStreamViewController: ReaderTopicsTableCardCellDelegate { +extension ReaderDiscoverStreamViewController: ReaderRecommendedSitesCellDelegate { func didSelect(topic: ReaderAbstractTopic) { if topic as? ReaderTagTopic != nil { WPAnalytics.trackReader(.readerDiscoverTopicTapped) @@ -385,13 +384,3 @@ extension ReaderDiscoverStreamViewController: ReaderTopicsTableCardCellDelegate } } } - -// MARK: - ReaderSitesCardCellDelegate - -extension ReaderDiscoverStreamViewController: ReaderSitesCardCellDelegate { - func handleFollowActionForTopic(_ topic: ReaderAbstractTopic, for cell: ReaderSitesCardCell) { - toggleFollowingForTopic(topic) { success in - cell.didToggleFollowing(topic, with: success) - } - } -} diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSitesCardCell.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSitesCardCell.swift deleted file mode 100644 index d2803f619daa..000000000000 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSitesCardCell.swift +++ /dev/null @@ -1,73 +0,0 @@ -import UIKit - -/// A cell that displays topics the user might like -/// -class ReaderSitesCardCell: ReaderTopicsTableCardCell { - private let cellIdentifier = "SitesTopicCell" - - override func configure(_ data: [ReaderAbstractTopic]) { - super.configure(data) - - headerTitle = Constants.title - } - - override func setupTableView() { - super.setupTableView() - - let cell = UINib(nibName: "ReaderRecommendedSiteCardCell", bundle: Bundle.main) - tableView.register(cell, forCellReuseIdentifier: cellIdentifier) - } - - override func cell(forRowAt indexPath: IndexPath, tableView: UITableView, topic: ReaderAbstractTopic?) -> UITableViewCell { - guard - let siteTopic = topic as? ReaderSiteTopic, - let cell = tableView.dequeueReusableCell(withIdentifier: cellIdentifier, for: indexPath as IndexPath) as? ReaderRecommendedSiteCardCell - else { - return UITableViewCell() - } - - cell.configure(siteTopic) - cell.delegate = self - return cell - } - - func didToggleFollowing(_ topic: ReaderAbstractTopic, with success: Bool) { - guard let row = data.firstIndex(of: topic) else { - return - } - - tableView.reloadRows(at: [IndexPath(row: row, section: 0)], with: .none) - } - - private enum Constants { - 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" - ) - } -} - -protocol ReaderSitesCardCellDelegate: ReaderTopicsTableCardCellDelegate { - func handleFollowActionForTopic(_ topic: ReaderAbstractTopic, for cell: ReaderSitesCardCell) -} - -extension ReaderSitesCardCell: ReaderRecommendedSitesCardCellDelegate { - func handleFollowActionForCell(_ cell: ReaderRecommendedSiteCardCell) { - guard - let indexPath = self.tableView.indexPath(for: cell), - let topic = data[indexPath.row] as? ReaderSiteTopic - else { - return - } - - (delegate as? ReaderSitesCardCellDelegate)?.handleFollowActionForTopic(topic, for: self) - - // Track Follow Action - var properties = [String: Any]() - properties[WPAppAnalyticsKeyFollowAction] = !topic.following - properties[WPAppAnalyticsKeyBlogID] = topic.siteID - - WPAnalytics.trackReader(.readerSuggestedSiteToggleFollow, properties: properties) - } -} diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift index 761cdb6e6cc2..58889bfe52ba 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift @@ -1117,7 +1117,7 @@ import AutomatticTracks if let topic = topic as? ReaderTagTopic { toggleFollowingForTag(topic, completion: completion) } else if let topic = topic as? ReaderSiteTopic { - toggleFollowingForSite(topic, completion: completion) + ReaderSubscriptionHelper().toggleFollowingForSite(topic, completion: completion) } else { wpAssertionFailure("unexpected topic", userInfo: ["type": String(describing: topic)]) } @@ -1139,21 +1139,6 @@ import AutomatticTracks completion?(false) }) } - - private func toggleFollowingForSite(_ topic: ReaderSiteTopic, completion: ((Bool) -> Void)?) { - if topic.following { - ReaderSubscribingNotificationAction().execute(for: siteID, context: viewContext, subscribe: false) - } - - let service = ReaderTopicService(coreDataStack: ContextManager.shared) - service.toggleFollowing(forSite: topic, success: { follow in - ReaderHelpers.dispatchToggleFollowSiteMessage(site: topic, follow: follow, success: true) - completion?(true) - }, failure: { (follow, error) in - ReaderHelpers.dispatchToggleFollowSiteMessage(site: topic, follow: follow, success: false) - completion?(false) - }) - } } // MARK: - ReaderStreamHeaderDelegate diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableCardCell.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableCardCell.swift deleted file mode 100644 index ab019a41b528..000000000000 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableCardCell.swift +++ /dev/null @@ -1,175 +0,0 @@ -import UIKit - -/// A cell that contains a table that displays a list of ReaderAbstractTopic's -/// -class ReaderTopicsTableCardCell: UITableViewCell { - private let containerView = UIView() - - let tableView: UITableView = ReaderTopicsTableView() - - private(set) var data: [ReaderAbstractTopic] = [] { - didSet { - guard oldValue != data else { - return - } - - tableView.reloadData() - } - } - - /// Constraints to be activated in compact horizontal size class - private var compactConstraints: [NSLayoutConstraint] = [] - - /// Constraints to be activated in regular horizontal size class - private var regularConstraints: [NSLayoutConstraint] = [] - - weak var delegate: ReaderTopicsTableCardCellDelegate? - - // Subclasses should configure these properties - var headerTitle: String? - - override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) { - super.init(style: style, reuseIdentifier: reuseIdentifier) - setupTableView() - applyStyles() - - // Since iOS 14, the contentView is in the top of the view hierarchy. - // This conflicts with the tableView interaction, so we disable it. - contentView.isUserInteractionEnabled = false - } - - required init?(coder: NSCoder) { - super.init(coder: coder) - } - - override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { - super.traitCollectionDidChange(previousTraitCollection) - refreshHorizontalConstraints() - } - - func configure(_ data: [ReaderAbstractTopic]) { - self.data = data - } - - func setupTableView() { - addSubview(containerView) - containerView.translatesAutoresizingMaskIntoConstraints = false - pinSubviewToSafeArea(containerView, insets: Constants.containerInsets) - containerView.addSubview(tableView) - tableView.translatesAutoresizingMaskIntoConstraints = false - let tableViewMargin = 16.0 - NSLayoutConstraint.activate([ - tableView.topAnchor.constraint(equalTo: containerView.topAnchor, constant: tableViewMargin), - tableView.bottomAnchor.constraint(equalTo: containerView.bottomAnchor, constant: -tableViewMargin) - ]) - - // Constraints for regular horizontal size class - regularConstraints = [ - tableView.leadingAnchor.constraint(equalTo: readableContentGuide.leadingAnchor, constant: tableViewMargin), - tableView.trailingAnchor.constraint(equalTo: readableContentGuide.trailingAnchor, constant: -tableViewMargin) - ] - - // Constraints for compact horizontal size class - compactConstraints = [ - tableView.leadingAnchor.constraint(equalTo: containerView.leadingAnchor, constant: tableViewMargin), - tableView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor, constant: -tableViewMargin) - ] - - tableView.isScrollEnabled = false - tableView.dataSource = self - tableView.delegate = self - } - - private func applyStyles() { - containerView.backgroundColor = .systemBackground - tableView.backgroundColor = .secondarySystemBackground - tableView.layer.cornerRadius = 10.0 - tableView.separatorColor = .clear - - backgroundColor = .clear - contentView.backgroundColor = .clear - - refreshHorizontalConstraints() - } - - func cell(forRowAt indexPath: IndexPath, tableView: UITableView, topic: ReaderAbstractTopic?) -> UITableViewCell { - return UITableViewCell() - } - - // Activate and deactivate constraints based on horizontal size class - private func refreshHorizontalConstraints() { - let isCompact = (traitCollection.horizontalSizeClass == .compact) - - compactConstraints.forEach { $0.isActive = isCompact } - regularConstraints.forEach { $0.isActive = !isCompact } - } - - private enum Constants { - static let containerInsets = UIEdgeInsets(top: 0, left: 0, bottom: 0, right: 0) - static let headerInsets = UIEdgeInsets(top: 8, left: 16, bottom: 0, right: 0) - static let tableFooterHeight: CGFloat = 8.0 - } -} - -extension ReaderTopicsTableCardCell: UITableViewDataSource { - func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { - return data.count - } - - func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { - let tableCell = cell(forRowAt: indexPath, tableView: tableView, topic: data[indexPath.row]) - - tableCell.backgroundColor = .clear - tableCell.contentView.backgroundColor = .clear - - return tableCell - } - - func tableView(_ tableView: UITableView, viewForFooterInSection section: Int) -> UIView? { - return UIView(frame: .zero) - } - - func tableView(_ tableView: UITableView, heightForFooterInSection section: Int) -> CGFloat { - return Constants.tableFooterHeight - } -} - -extension ReaderTopicsTableCardCell: UITableViewDelegate { - func tableView(_ tableView: UITableView, viewForHeaderInSection section: Int) -> UIView? { - guard let title = headerTitle else { - return nil - } - let header = UIView() - let headerTitle = UILabel() - headerTitle.text = title - header.addSubview(headerTitle) - headerTitle.translatesAutoresizingMaskIntoConstraints = false - header.pinSubviewToAllEdges(headerTitle, insets: Constants.headerInsets) - headerTitle.font = WPStyleGuide.fontForTextStyle(.footnote) - headerTitle.textColor = .secondaryLabel - return header - } - - func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { - let topic = data[indexPath.row] - delegate?.didSelect(topic: topic) - tableView.deselectSelectedRowWithAnimation(true) - } -} - -private class ReaderTopicsTableView: UITableView { - override var intrinsicContentSize: CGSize { - self.layoutIfNeeded() - return self.contentSize - } - - override var contentSize: CGSize { - didSet { - self.invalidateIntrinsicContentSize() - } - } -} - -protocol ReaderTopicsTableCardCellDelegate: AnyObject { - func didSelect(topic: ReaderAbstractTopic) -} diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift index 48e87abff312..5bfde0e5a421 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift @@ -19,7 +19,7 @@ class ReaderTopicsCardCell: UITableViewCell, NibLoadable { } } - weak var delegate: ReaderTopicsTableCardCellDelegate? + weak var delegate: ReaderRecommendedSitesCellDelegate? static var defaultNibName: String { "ReaderTopicsCardCell" } diff --git a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift index 9e87187ea0bf..0574dcc86c02 100644 --- a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift +++ b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift @@ -54,7 +54,22 @@ struct ReaderSubscriptionHelper { }) } - // MARK: Unsubscribe + // MARK: Subscribe/Unsubscribe (ReaderSiteTopic) + + func toggleFollowingForSite(_ topic: ReaderSiteTopic, completion: ((Bool) -> Void)? = nil) { + if topic.following { + ReaderSubscribingNotificationAction().execute(for: topic.siteID, context: contextManager.mainContext, subscribe: false) + } + + let service = ReaderTopicService(coreDataStack: contextManager) + service.toggleFollowing(forSite: topic, success: { follow in + ReaderHelpers.dispatchToggleFollowSiteMessage(site: topic, follow: follow, success: true) + completion?(true) + }, failure: { (follow, error) in + ReaderHelpers.dispatchToggleFollowSiteMessage(site: topic, follow: follow, success: false) + completion?(false) + }) + } @MainActor func unfollow(_ site: ReaderSiteTopic) { From a254d337a64f50acfe23c02a5b24345a7582dff0 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 Nov 2024 22:20:26 +1300 Subject: [PATCH 03/28] Use a sheet to present deleting user flow (#23791) * Use a sheet to present deleting user flow * Use default Hashable implementation * Update localizable string keys * Add localized strings --- .../Views/Users/ViewModel/DisplayUser.swift | 2 +- .../Views/Users/Views/UserDeleteView.swift | 70 ------ .../Views/Users/Views/UserDetailsView.swift | 215 ++++++++++++++++-- 3 files changed, 202 insertions(+), 85 deletions(-) delete mode 100644 Modules/Sources/WordPressUI/Views/Users/Views/UserDeleteView.swift diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/DisplayUser.swift b/Modules/Sources/WordPressUI/Views/Users/ViewModel/DisplayUser.swift index 995f4aa9625f..34c892c63354 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/DisplayUser.swift +++ b/Modules/Sources/WordPressUI/Views/Users/ViewModel/DisplayUser.swift @@ -1,7 +1,7 @@ import Foundation import WordPressShared -public struct DisplayUser: Identifiable, Codable { +public struct DisplayUser: Identifiable, Codable, Hashable { public let id: Int32 public let handle: String public let username: String diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserDeleteView.swift b/Modules/Sources/WordPressUI/Views/Users/Views/UserDeleteView.swift deleted file mode 100644 index 0f9be1230c03..000000000000 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserDeleteView.swift +++ /dev/null @@ -1,70 +0,0 @@ -import SwiftUI - -struct UserDeleteView: View { - - @StateObject - private var viewModel: UserDeleteViewModel - private let userProvider: UserDataProvider - private let actionDispatcher: UserManagementActionDispatcher - - @Environment(\.dismiss) - var dismissAction: DismissAction - - var parentDismissAction: DismissAction? - - init(user: DisplayUser, userProvider: UserDataProvider, actionDispatcher: UserManagementActionDispatcher, dismiss: DismissAction? = nil) { - self.userProvider = userProvider - self.actionDispatcher = actionDispatcher - _viewModel = StateObject(wrappedValue: UserDeleteViewModel(user: user, userProvider: userProvider, actionDispatcher: actionDispatcher)) - parentDismissAction = dismiss - } - - var body: some View { - Form { - if let error = viewModel.error { - Text(error.localizedDescription) - } - else if viewModel.isFetchingOtherUsers { - LabeledContent("Attribute all content to:") { - ProgressView() - } - } else { - Picker("Attribute all content to:", selection: $viewModel.otherUserId.animation()) { - ForEach(viewModel.otherUsers) { user in - Text(user.username).tag(user.id) - } - } - } - Section { - Button(role: .destructive) { - viewModel.didTapDeleteUser { - self.dismissAction() - self.parentDismissAction?() - } - } label: { - HStack { - Spacer() - Text("Delete User") - .font(.headline) - .padding(4) - Spacer() - if viewModel.isDeletingUser { - ProgressView().tint(.white) - } - } - }.buttonStyle(.borderedProminent) - .disabled(viewModel.deleteButtonIsDisabled) - } - .listRowBackground(Color.clear) - .listRowInsets(.zero) - } - .navigationTitle("Delete User") - .task { await viewModel.fetchOtherUsers() } - } -} - -#Preview { - NavigationStack { - UserDeleteView(user: .MockUser, userProvider: MockUserProvider(), actionDispatcher: UserManagementActionDispatcher()) - } -} diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift b/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift index 6a74429f9cd5..5a4927751e24 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift +++ b/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift @@ -2,8 +2,8 @@ import SwiftUI struct UserDetailsView: View { - private let userProvider: UserDataProvider - private let actionDispatcher: UserManagementActionDispatcher + fileprivate let userProvider: UserDataProvider + fileprivate let actionDispatcher: UserManagementActionDispatcher let user: DisplayUser @State private var presentPasswordAlert: Bool = false { @@ -12,11 +12,18 @@ struct UserDetailsView: View { newPasswordConfirmation = "" } } - @State private var newPassword: String = "" - @State private var newPasswordConfirmation: String = "" + @State fileprivate var newPassword: String = "" + @State fileprivate var newPasswordConfirmation: String = "" + @State fileprivate var presentUserPicker: Bool = false + @State fileprivate var selectedUser: DisplayUser? = nil + @State fileprivate var presentDeleteConfirmation: Bool = false + @State fileprivate var presentDeleteUserError: Bool = false + + @StateObject + fileprivate var viewModel: UserDetailViewModel @StateObject - var viewModel: UserDetailViewModel + fileprivate var deleteUserViewModel: UserDeleteViewModel @Environment(\.dismiss) var dismissAction: DismissAction @@ -26,6 +33,7 @@ struct UserDetailsView: View { self.userProvider = userProvider self.actionDispatcher = actionDispatcher _viewModel = StateObject(wrappedValue: UserDetailViewModel(userProvider: userProvider)) + _deleteUserViewModel = StateObject(wrappedValue: UserDeleteViewModel(user: user, userProvider: userProvider, actionDispatcher: actionDispatcher)) } var body: some View { @@ -58,14 +66,16 @@ struct UserDetailsView: View { Button(Strings.setNewPasswordActionTitle) { presentPasswordAlert = true } - - NavigationLink { - // Pass this view's dismiss action, because if we delete a user, we want that screen *and* this one gone - UserDeleteView(user: user, userProvider: userProvider, actionDispatcher: actionDispatcher, dismiss: dismissAction) + Button(role: .destructive) { + presentUserPicker = true } label: { - Text(Strings.deleteUserActionTitle) - .foregroundStyle(Color.red) + Text( + deleteUserViewModel.isDeletingUser ? + Strings.deletingUserActionTitle + : Strings.deleteUserActionTitle + ) } + .disabled(deleteUserViewModel.isDeletingUser) } } } @@ -92,8 +102,10 @@ struct UserDetailsView: View { Text(Strings.newPasswordAlertMessage) } ) + .deleteUser(in: self) .task { await viewModel.loadCurrentUserRole() + await deleteUserViewModel.fetchOtherUsers() } } @@ -152,6 +164,12 @@ struct UserDetailsView: View { comment: "The 'Delete User' button on the user profile – matches what's in /wp-admin/profile.php" ) + static let deletingUserActionTitle = NSLocalizedString( + "userDetails.deletingUserActionTitle", + value: "Deleting User…", + comment: "The 'Deleting User…' button on the user profile" + ) + static let newPasswordAlertMessage = NSLocalizedString( "userDetails.newPasswordAlertMessage", value: "Enter a new password for this user", @@ -175,12 +193,175 @@ struct UserDetailsView: View { value: "Update", comment: "The 'Update' button to set a new password on the user profile" ) + + static let deleteUserConfirmationTitle = NSLocalizedString( + "userDetails.alert.deleteUserConfirmationTitle", + value: "Are you sure?", + comment: "The title of the alert that appears when deleting a user" + ) + + static func deleteUserConfirmationMessage(username: String) -> String { + let format = NSLocalizedString( + "userDetails.alert.deleteUserConfirmationMessage", + value: "Are you sure you want to delete this user and attribute all content to %@?", + comment: "The message in the alert that appears when deleting a user. The first argument is the display name of the user to which content will be attributed" + ) + return String(format: format, username) + } + + static let deleteUserConfirmButtonTitle = NSLocalizedString( + "userDetails.alert.deleteUserConfirmButtonTitle", + value: "Yes, delete user", + comment: "The title of the confirmation button in the alert that appears when deleting a user" + ) + + static let deleteUserErrorAlertTitle = NSLocalizedString( + "userDetails.alert.deleteUserErrorAlertTitle", + value: "Error", + comment: "The title of the alert that appears when deleting a user" + ) + + static let deleteUserErrorAlertMessage = NSLocalizedString( + "userDetails.alert.deleteUserErrorAlertMessage", + value: "There was an error deleting the user.", + comment: "The message in the alert that appears when deleting a user" + ) + + static let deleteUserErrorAlertOkButton = NSLocalizedString( + "userDetails.alert.deleteUserErrorAlertOkButton", + value: "OK", + comment: "The title of the OK button in the alert that appears when deleting a user" + ) + + static let deleteUserAttributionMessage = NSLocalizedString( + "userDetails.alert.deleteUserAttributionMessage", + value: "You have specified this user for deletion:", + comment: "The message that appears when deleting a user." + ) + + static let attributeContentToUserLabel = NSLocalizedString( + "userDetails.alert.attributeContentToUserLabel", + value: "Attribute content to user:", + comment: "The label that appears in the alert that appears when deleting a user" + ) + + static let attributeContentConfirmationTitle = NSLocalizedString( + "userDetails.alert.deleteUserConfirmationTitle", + value: "Delete Confirmation", + comment: "The title of the confirmation alert that appears when deleting a user" + ) + + static let attributeContentConfirmationCancelButton = NSLocalizedString( + "userDetails.alert.deleteUserConfirmationCancelButton", + value: "Cancel", + comment: "The title of the cancel button in the confirmation alert that appears when deleting a user" + ) + + static let attributeContentConfirmationDeleteButton = NSLocalizedString( + "userDetails.alert.deleteUserConfirmationDeleteButton", + value: "Delete", + comment: "The title of the delete button in the confirmation alert that appears when deleting a user" + ) + } } -#Preview { - NavigationStack { - UserDetailsView(user: DisplayUser.MockUser, userProvider: MockUserProvider(), actionDispatcher: UserManagementActionDispatcher()) +private extension View { + typealias Strings = UserDetailsView.Strings + + @ViewBuilder + func deleteUser(in view: UserDetailsView) -> some View { + sheet( + isPresented: view.$presentUserPicker, + onDismiss: { + view.presentUserPicker = false + }, + content: { + pickAnotherUser(in: view) + } + ) + .alert( + UserDetailsView.Strings.deleteUserConfirmationTitle, + isPresented: view.$presentDeleteConfirmation, + presenting: view.selectedUser, + actions: { attribution in + Button(role: .destructive) { + Task { + view.deleteUserViewModel.didTapDeleteUser { + view.dismissAction() + } + } + } label: { + Text(UserDetailsView.Strings.deleteUserConfirmButtonTitle) + } + }, + message: { + Text(UserDetailsView.Strings.deleteUserConfirmationMessage(username: $0.displayName)) + } + ) + .alert( + Strings.deleteUserErrorAlertTitle, + isPresented: view.$presentDeleteUserError, + presenting: view.deleteUserViewModel.error, + actions: { _ in + Button(Strings.deleteUserErrorAlertOkButton) { + view.presentDeleteUserError = false + } + }, + message: { error in + Text(Strings.deleteUserErrorAlertMessage) + // TODO: Use appropriate localized error message + Text(error.localizedDescription) + }) + } + + @ViewBuilder + func pickAnotherUser(in view: UserDetailsView) -> some View { + NavigationView { + Form { + VStack(alignment: .leading) { + Text(Strings.deleteUserAttributionMessage) + Text("ID #\(view.user.id): \(view.user.username)") + } + .frame(maxWidth: .infinity) + .listRowBackground(Color.clear) + .listRowInsets(.zero) + + Section { + if view.deleteUserViewModel.isFetchingOtherUsers { + LabeledContent(Strings.attributeContentToUserLabel) { + ProgressView() + } + } else { + Picker(Strings.attributeContentToUserLabel, selection: view.$selectedUser) { + ForEach(view.deleteUserViewModel.otherUsers) { user in + Text("\(user.displayName) (\(user.username))").tag(user) + } + } + } + } + } + .navigationTitle(Strings.attributeContentConfirmationTitle) + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .cancellationAction) { + Button(role: .cancel) { + view.presentUserPicker = false + } label: { + Text(Strings.attributeContentConfirmationCancelButton) + } + } + ToolbarItem(placement: .destructiveAction) { + Button(role: .destructive) { + view.presentDeleteConfirmation = true + } label: { + Text(Strings.attributeContentConfirmationDeleteButton) + } + .disabled(view.selectedUser == nil) + } + } + } + .presentationDetents([.medium]) } } @@ -196,3 +377,9 @@ private extension String { return result.url } } + +#Preview { + NavigationStack { + UserDetailsView(user: DisplayUser.MockUser, userProvider: MockUserProvider(), actionDispatcher: UserManagementActionDispatcher()) + } +} From dfdc8de833f04448304c7e41a6633bf3b8c4e386 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 15 Nov 2024 10:47:58 +1300 Subject: [PATCH 04/28] Append "+testing" to feedback form email for Automatticians (#23804) --- WordPress/Classes/Utility/ZendeskUtils.swift | 34 ++++++++++++++++-- .../Support/SupportTableViewController.swift | 3 +- WordPress/WordPress.xcodeproj/project.pbxproj | 4 +++ .../ZendeskUtilsTests+A8CEmail.swift | 35 +++++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 WordPress/WordPressTest/ZendeskUtilsTests+A8CEmail.swift diff --git a/WordPress/Classes/Utility/ZendeskUtils.swift b/WordPress/Classes/Utility/ZendeskUtils.swift index 696a45881647..22bc6cff823a 100644 --- a/WordPress/Classes/Utility/ZendeskUtils.swift +++ b/WordPress/Classes/Utility/ZendeskUtils.swift @@ -42,7 +42,6 @@ protocol ZendeskUtilsProtocol { /// as well as displaying views for the Help Center, new tickets, and ticket list. /// @objc class ZendeskUtils: NSObject, ZendeskUtilsProtocol { - // MARK: - Public Properties static var sharedInstance: ZendeskUtils = ZendeskUtils(contextManager: ContextManager.shared) @@ -63,7 +62,15 @@ protocol ZendeskUtilsProtocol { private var sourceTag: WordPressSupportSourceTag? private var userName: String? - private var userEmail: String? + private var userEmail: String? { + set { + _userEmail = newValue.map(ZendeskUtils.a8cTestEmail(_:)) + } + get { + _userEmail + } + } + private var _userEmail: String? private var userNameConfirmed = false private var deviceID: String? @@ -1201,3 +1208,26 @@ extension ZendeskUtils { } } } + +extension ZendeskUtils { + static var automatticEmails = ["@automattic.com", "@a8c.com"] + + /// Insert "+testing" to Automattic email address. + /// - SeeAlso: https://github.com/wordpress-mobile/WordPress-iOS/issues/23794 + static func a8cTestEmail(_ email: String) -> String { + guard let suffix = ZendeskUtils.automatticEmails.first(where: { email.lowercased().hasSuffix($0) }) else { + return email + } + + let atSymbolIndex = email.index(email.endIndex, offsetBy: -suffix.count) + + // Do nothing if the email is already an "alias email". + if email[email.startIndex.. Date: Fri, 15 Nov 2024 11:09:53 +1300 Subject: [PATCH 05/28] Merge UserDataProvider and UserManagementActionDispatcher into one type (#23792) * Merge UserDataProvider and UserManagementActionDispatcher into one type * Cache current user info in UserService * Minor change to function declarations * Add unit tests for UserService * Use AsyncStream to publish updates instead of Combine publisher * Remove some whitespaces * Terminate users updates stream upon deallocating * Use `onAppear` instead of `Task` to avoid duplicated calls --- .../Views/Users/Components/UserListItem.swift | 12 +- .../Views/Users/UserProvider.swift | 61 ++++--- .../Users/ViewModel/UserDeleteViewModel.swift | 91 ++++------ .../Users/ViewModel/UserDetailViewModel.swift | 28 +--- .../Users/ViewModel/UserListViewModel.swift | 55 +++--- .../Views/Users/Views/UserDetailsView.swift | 38 +++-- .../Views/Users/Views/UserListView.swift | 18 +- WordPress/Classes/Services/UserService.swift | 156 +++++++++--------- ...DetailsViewController+SectionHelpers.swift | 6 +- WordPress/WordPress.xcodeproj/project.pbxproj | 4 + .../WordPressTest/UserServiceTests.swift | 124 ++++++++++++++ 11 files changed, 345 insertions(+), 248 deletions(-) create mode 100644 WordPress/WordPressTest/UserServiceTests.swift diff --git a/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift b/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift index c528fe6e57cb..93c21253aa7a 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift +++ b/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift @@ -9,18 +9,16 @@ struct UserListItem: View { var dynamicTypeSize private let user: DisplayUser - private let userProvider: UserDataProvider - private let actionDispatcher: UserManagementActionDispatcher + private let userService: UserServiceProtocol - init(user: DisplayUser, userProvider: UserDataProvider, actionDispatcher: UserManagementActionDispatcher) { + init(user: DisplayUser, userService: UserServiceProtocol) { self.user = user - self.userProvider = userProvider - self.actionDispatcher = actionDispatcher + self.userService = userService } var body: some View { NavigationLink { - UserDetailsView(user: user, userProvider: userProvider, actionDispatcher: actionDispatcher) + UserDetailsView(user: user, userService: userService) } label: { HStack(alignment: .top) { if !dynamicTypeSize.isAccessibilitySize { @@ -36,5 +34,5 @@ struct UserListItem: View { } #Preview { - UserListItem(user: DisplayUser.MockUser, userProvider: MockUserProvider(), actionDispatcher: UserManagementActionDispatcher()) + UserListItem(user: DisplayUser.MockUser, userService: MockUserProvider()) } diff --git a/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift b/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift index 4adea4d82f8c..59b1cda47221 100644 --- a/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift +++ b/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift @@ -1,32 +1,20 @@ import Foundation +import Combine -public protocol UserDataProvider { +public protocol UserServiceProtocol: Actor { + var users: [DisplayUser]? { get } + nonisolated var usersUpdates: AsyncStream<[DisplayUser]> { get } - typealias CachedUserListCallback = ([WordPressUI.DisplayUser]) async -> Void + func fetchUsers() async throws -> [DisplayUser] - func fetchCurrentUserCan(_ capability: String) async throws -> Bool - func fetchUsers(cachedResults: CachedUserListCallback?) async throws -> [WordPressUI.DisplayUser] + func isCurrentUserCapableOf(_ capability: String) async throws -> Bool - func invalidateCaches() async throws -} - -/// Subclass this and register it with the SwiftUI `.environmentObject` method -/// to perform user management actions. -/// -/// The default implementation is set up for testing with SwiftUI Previews -open class UserManagementActionDispatcher: ObservableObject { - public init() {} - - open func setNewPassword(id: Int32, newPassword: String) async throws { - try await Task.sleep(for: .seconds(2)) - } + func setNewPassword(id: Int32, newPassword: String) async throws - open func deleteUser(id: Int32, reassigningPostsTo userId: Int32) async throws { - try await Task.sleep(for: .seconds(2)) - } + func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws } -package struct MockUserProvider: UserDataProvider { +package actor MockUserProvider: UserServiceProtocol { enum Scenario { case infinitLoading @@ -36,29 +24,48 @@ package struct MockUserProvider: UserDataProvider { var scenario: Scenario + package nonisolated let usersUpdates: AsyncStream<[DisplayUser]> + private let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation + + package private(set) var users: [DisplayUser]? { + didSet { + if let users { + usersUpdatesContinuation.yield(users) + } + } + } + init(scenario: Scenario = .dummyData) { self.scenario = scenario + (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream() } - package func fetchUsers(cachedResults: CachedUserListCallback? = nil) async throws -> [WordPressUI.DisplayUser] { + package func fetchUsers() async throws -> [DisplayUser] { switch scenario { case .infinitLoading: - try await Task.sleep(for: .seconds(1 * 24 * 60 * 60)) + // Do nothing + try await Task.sleep(for: .seconds(24 * 60 * 60)) return [] case .dummyData: let dummyDataUrl = URL(string: "https://my.api.mockaroo.com/users.json?key=067c9730")! let response = try await URLSession.shared.data(from: dummyDataUrl) - return try JSONDecoder().decode([DisplayUser].self, from: response.0) + let users = try JSONDecoder().decode([DisplayUser].self, from: response.0) + self.users = users + return users case .error: throw URLError(.timedOut) } } - package func fetchCurrentUserCan(_ capability: String) async throws -> Bool { + package func isCurrentUserCapableOf(_ capability: String) async throws -> Bool { true } - package func invalidateCaches() async throws { - // Do nothing + package func setNewPassword(id: Int32, newPassword: String) async throws { + // Not used in Preview + } + + package func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { + // Not used in Preview } } diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift index c96607a9d324..6736f1e8365c 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift +++ b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift @@ -13,7 +13,7 @@ public class UserDeleteViewModel: ObservableObject { private(set) var error: Error? = nil @Published - var otherUserId: Int32 = 0 + var selectedUser: DisplayUser? = nil @Published private(set) var otherUsers: [DisplayUser] = [] @@ -21,84 +21,49 @@ public class UserDeleteViewModel: ObservableObject { @Published private(set) var deleteButtonIsDisabled: Bool = true - private let userProvider: UserDataProvider - private let actionDispatcher: UserManagementActionDispatcher + private let userService: UserServiceProtocol let user: DisplayUser - init(user: DisplayUser, userProvider: UserDataProvider, actionDispatcher: UserManagementActionDispatcher) { + init(user: DisplayUser, userService: UserServiceProtocol) { self.user = user - self.userProvider = userProvider - self.actionDispatcher = actionDispatcher - } + self.userService = userService - func fetchOtherUsers() async { - withAnimation { - isFetchingOtherUsers = true - deleteButtonIsDisabled = true - } + // Default `selectedUser` to be the first one in `otherUsers`. + // Using Combine here because `didSet` observers don't work with `@Published` properties. + // + // The implementation is equivalent to `if selectedUser == nil { selectedUser = otherUsers.first }` + $otherUsers.combineLatest($selectedUser) + .filter { _, selectedUser in selectedUser == nil } + .map { others, _ in others.first } + .assign(to: &$selectedUser) - do { - let otherUsers = try await userProvider.fetchUsers { self.didReceiveUsers($0) } + } - self.didReceiveUsers(otherUsers) - } catch { - withAnimation { - self.error = error - deleteButtonIsDisabled = true - } - } + func fetchOtherUsers() async { + isFetchingOtherUsers = true + deleteButtonIsDisabled = true - withAnimation { + defer { isFetchingOtherUsers = false + deleteButtonIsDisabled = otherUsers.isEmpty } - } - - func didReceiveUsers(_ users: [DisplayUser]) { - withAnimation { - if otherUserId == 0 { - otherUserId = otherUsers.first?.id ?? 0 - } - otherUsers = users + do { + let users = try await userService.fetchUsers() + self.otherUsers = users .filter { $0.id != self.user.id } // Don't allow re-assigning to yourself .sorted(using: KeyPathComparator(\.username)) - error = nil - deleteButtonIsDisabled = false - isFetchingOtherUsers = false + } catch { + self.error = error } } - func didTapDeleteUser(callback: @escaping () -> Void) { - debugPrint("Deleting \(user.username) and re-assigning their content to \(otherUserId)") + func deleteUser() async throws { + guard let otherUserId = selectedUser?.id, otherUserId != user.id else { return } - withAnimation { - error = nil - } + isDeletingUser = true + defer { isDeletingUser = false } - Task { - await MainActor.run { - withAnimation { - isDeletingUser = true - } - } - - do { - try await actionDispatcher.deleteUser(id: user.id, reassigningPostsTo: otherUserId) - } catch { - debugPrint(error.localizedDescription) - await MainActor.run { - withAnimation { - self.error = error - } - } - } - - await MainActor.run { - withAnimation { - isDeletingUser = false - callback() - } - } - } + try await userService.deleteUser(id: user.id, reassigningPostsTo: otherUserId) } } diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift index e479d5740bdc..87179642d603 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift +++ b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift @@ -2,7 +2,7 @@ import SwiftUI @MainActor class UserDetailViewModel: ObservableObject { - private let userProvider: UserDataProvider + private let userService: UserServiceProtocol @Published private(set) var currentUserCanModifyUsers: Bool = false @@ -13,30 +13,20 @@ class UserDetailViewModel: ObservableObject { @Published private(set) var error: Error? = nil - init(userProvider: UserDataProvider) { - self.userProvider = userProvider + init(userService: UserServiceProtocol) { + self.userService = userService } func loadCurrentUserRole() async { - withAnimation { - isLoadingCurrentUser = true - } + error = nil - do { - let hasPermissions = try await userProvider.fetchCurrentUserCan("edit_users") - error = nil + isLoadingCurrentUser = true + defer { isLoadingCurrentUser = false} - withAnimation { - currentUserCanModifyUsers = hasPermissions - } + do { + currentUserCanModifyUsers = try await userService.isCurrentUserCapableOf("edit_users") } catch { - withAnimation { - self.error = error - } - } - - withAnimation { - isLoadingCurrentUser = false + self.error = error } } } diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift index eb621c5cac1a..8ecab8e027d3 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift +++ b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift @@ -1,4 +1,5 @@ import SwiftUI +import Combine import WordPressShared @MainActor @@ -11,9 +12,13 @@ class UserListViewModel: ObservableObject { } /// The initial set of users fetched by `fetchItems` - private var users: [DisplayUser] = [] - private let userProvider: UserDataProvider - + private var users: [DisplayUser] = [] { + didSet { + sortedUsers = self.sortUsers(users) + } + } + private var updateUsersTask: Task? + private let userService: UserServiceProtocol private var initialLoad = false @Published @@ -37,43 +42,41 @@ class UserListViewModel: ObservableObject { } } - init(userProvider: UserDataProvider) { - self.userProvider = userProvider + init(userService: UserServiceProtocol) { + self.userService = userService + } + + deinit { + updateUsersTask?.cancel() } func onAppear() async { + if updateUsersTask == nil { + updateUsersTask = Task { @MainActor [weak self, usersUpdates = userService.usersUpdates] in + for await users in usersUpdates { + guard let self else { break } + + self.users = users + } + } + } + if !initialLoad { initialLoad = true await fetchItems() } } - func fetchItems() async { - withAnimation { - isLoadingItems = true - } + private func fetchItems() async { + isLoadingItems = true + defer { isLoadingItems = false } - do { - let users = try await userProvider.fetchUsers { cachedResults in - self.setUsers(cachedResults) - } - setUsers(users) - } catch { - self.error = error - isLoadingItems = false - } + _ = try? await userService.fetchUsers() } @Sendable func refreshItems() async { - do { - let users = try await userProvider.fetchUsers { cachedResults in - self.setUsers(cachedResults) - } - setUsers(users) - } catch { - // Do nothing for now – this should probably show a "Toast" notification or something - } + _ = try? await userService.fetchUsers() } func setUsers(_ newValue: [DisplayUser]) { diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift b/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift index 5a4927751e24..39d8e0457f47 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift +++ b/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift @@ -2,8 +2,7 @@ import SwiftUI struct UserDetailsView: View { - fileprivate let userProvider: UserDataProvider - fileprivate let actionDispatcher: UserManagementActionDispatcher + fileprivate let userService: UserServiceProtocol let user: DisplayUser @State private var presentPasswordAlert: Bool = false { @@ -16,7 +15,6 @@ struct UserDetailsView: View { @State fileprivate var newPasswordConfirmation: String = "" @State fileprivate var presentUserPicker: Bool = false - @State fileprivate var selectedUser: DisplayUser? = nil @State fileprivate var presentDeleteConfirmation: Bool = false @State fileprivate var presentDeleteUserError: Bool = false @@ -28,12 +26,11 @@ struct UserDetailsView: View { @Environment(\.dismiss) var dismissAction: DismissAction - init(user: DisplayUser, userProvider: UserDataProvider, actionDispatcher: UserManagementActionDispatcher) { + init(user: DisplayUser, userService: UserServiceProtocol) { self.user = user - self.userProvider = userProvider - self.actionDispatcher = actionDispatcher - _viewModel = StateObject(wrappedValue: UserDetailViewModel(userProvider: userProvider)) - _deleteUserViewModel = StateObject(wrappedValue: UserDeleteViewModel(user: user, userProvider: userProvider, actionDispatcher: actionDispatcher)) + self.userService = userService + _viewModel = StateObject(wrappedValue: UserDetailViewModel(userService: userService)) + _deleteUserViewModel = StateObject(wrappedValue: UserDeleteViewModel(user: user, userService: userService)) } var body: some View { @@ -87,7 +84,7 @@ struct UserDetailsView: View { SecureField(Strings.newPasswordConfirmationPlaceholder, text: $newPasswordConfirmation) Button(Strings.updatePasswordButton) { Task { - try await self.actionDispatcher.setNewPassword(id: user.id, newPassword: newPassword) + try await self.userService.setNewPassword(id: user.id, newPassword: newPassword) } } .disabled(newPassword.isEmpty || newPassword != newPasswordConfirmation) @@ -103,9 +100,11 @@ struct UserDetailsView: View { } ) .deleteUser(in: self) - .task { - await viewModel.loadCurrentUserRole() - await deleteUserViewModel.fetchOtherUsers() + .onAppear() { + Task { + await viewModel.loadCurrentUserRole() + await deleteUserViewModel.fetchOtherUsers() + } } } @@ -283,12 +282,15 @@ private extension View { .alert( UserDetailsView.Strings.deleteUserConfirmationTitle, isPresented: view.$presentDeleteConfirmation, - presenting: view.selectedUser, + presenting: view.deleteUserViewModel.selectedUser, actions: { attribution in Button(role: .destructive) { - Task { - view.deleteUserViewModel.didTapDeleteUser { + Task { @MainActor in + do { + try await view.deleteUserViewModel.deleteUser() view.dismissAction() + } catch { + view.presentDeleteUserError = true } } } label: { @@ -333,7 +335,7 @@ private extension View { ProgressView() } } else { - Picker(Strings.attributeContentToUserLabel, selection: view.$selectedUser) { + Picker(Strings.attributeContentToUserLabel, selection: view.$deleteUserViewModel.selectedUser) { ForEach(view.deleteUserViewModel.otherUsers) { user in Text("\(user.displayName) (\(user.username))").tag(user) } @@ -357,7 +359,7 @@ private extension View { } label: { Text(Strings.attributeContentConfirmationDeleteButton) } - .disabled(view.selectedUser == nil) + .disabled(view.deleteUserViewModel.deleteButtonIsDisabled) } } } @@ -380,6 +382,6 @@ private extension String { #Preview { NavigationStack { - UserDetailsView(user: DisplayUser.MockUser, userProvider: MockUserProvider(), actionDispatcher: UserManagementActionDispatcher()) + UserDetailsView(user: DisplayUser.MockUser, userService: MockUserProvider()) } } diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift b/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift index a141e0036d60..31cb09e7775a 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift +++ b/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift @@ -4,13 +4,11 @@ public struct UserListView: View { @StateObject private var viewModel: UserListViewModel - private let userProvider: UserDataProvider - private let actionDispatcher: UserManagementActionDispatcher + private let userService: UserServiceProtocol - public init(userProvider: UserDataProvider, actionDispatcher: UserManagementActionDispatcher) { - self.userProvider = userProvider - self.actionDispatcher = actionDispatcher - _viewModel = StateObject(wrappedValue: UserListViewModel(userProvider: userProvider)) + public init(userService: UserServiceProtocol) { + self.userService = userService + _viewModel = StateObject(wrappedValue: UserListViewModel(userService: userService)) } public var body: some View { @@ -33,7 +31,7 @@ public struct UserListView: View { .listRowBackground(Color.clear) } else { ForEach(section.users) { user in - UserListItem(user: user, userProvider: userProvider, actionDispatcher: actionDispatcher) + UserListItem(user: user, userService: userService) } } } @@ -72,18 +70,18 @@ public struct UserListView: View { #Preview("Loading") { NavigationView { - UserListView(userProvider: MockUserProvider(scenario: .infinitLoading), actionDispatcher: UserManagementActionDispatcher()) + UserListView(userService: MockUserProvider()) } } #Preview("Error") { NavigationView { - UserListView(userProvider: MockUserProvider(scenario: .error), actionDispatcher: UserManagementActionDispatcher()) + UserListView(userService: MockUserProvider(scenario: .error)) } } #Preview("List") { NavigationView { - UserListView(userProvider: MockUserProvider(scenario: .dummyData), actionDispatcher: UserManagementActionDispatcher()) + UserListView(userService: MockUserProvider(scenario: .dummyData)) } } diff --git a/WordPress/Classes/Services/UserService.swift b/WordPress/Classes/Services/UserService.swift index f57a50d26332..f684dc20b93f 100644 --- a/WordPress/Classes/Services/UserService.swift +++ b/WordPress/Classes/Services/UserService.swift @@ -1,107 +1,113 @@ import Foundation +import Combine import WordPressAPI import WordPressUI /// UserService is responsible for fetching user acounts via the .org REST API – it's the replacement for `UsersService` (the XMLRPC-based approach) /// -struct UserService { +actor UserService: UserServiceProtocol { + private let client: WordPressClient - final class ActionDispatcher: UserManagementActionDispatcher { + private var fetchUsersTask: Task<[DisplayUser], Error>? - private let client: WordPressClient - - init(client: WordPressClient) { - self.client = client - super.init() + private(set) var users: [DisplayUser]? { + didSet { + if let users { + usersUpdatesContinuation.yield(users) + } } + } + nonisolated let usersUpdates: AsyncStream<[DisplayUser]> + private nonisolated let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation - override func setNewPassword(id: Int32, newPassword: String) async throws { - _ = try await client.api.users.update( - userId: Int32(id), - params: UserUpdateParams(password: newPassword) - ) - } + private var currentUser: UserWithEditContext? - override func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { - _ = try await client.api.users.delete( - userId: id, - params: UserDeleteParams(reassign: newUserId) - ) - } + init(client: WordPressClient) { + self.client = client + (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream() + } + + deinit { + usersUpdatesContinuation.finish() + fetchUsersTask?.cancel() } - final actor UserCache { - var users: [DisplayUser] = [] + func fetchUsers() async throws -> [DisplayUser] { + let users = try await createFetchUsersTaskIfNeeded().value + self.users = users + return users + } - func setUsers(_ newValue: [DisplayUser]) { - self.users = newValue + private func createFetchUsersTaskIfNeeded() -> Task<[DisplayUser], Error> { + if let fetchUsersTask { + return fetchUsersTask } - - func clear() { - self.users = [] + let task = Task { [client] in + try await client + .api + .users + .listWithEditContext(params: UserListParams(perPage: 100)) + .compactMap { DisplayUser(user: $0) } } + fetchUsersTask = task + return task } - private let apiClient: WordPressClient - private let currentUserId: Int - fileprivate let userCache = UserCache() - - let actionDispatcher: ActionDispatcher + func isCurrentUserCapableOf(_ capability: String) async throws -> Bool { + let currentUser: UserWithEditContext + if let cached = self.currentUser { + currentUser = cached + } else { + currentUser = try await self.client.api.users.retrieveMeWithEditContext() + self.currentUser = currentUser + } - init(api: WordPressClient, currentUserId: Int) { - self.apiClient = api - self.currentUserId = currentUserId - self.actionDispatcher = ActionDispatcher(client: apiClient) + return currentUser.capabilities.keys.contains(capability) } -} -extension UserService: WordPressUI.UserDataProvider { + func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { + let result = try await client.api.users.delete( + userId: id, + params: UserDeleteParams(reassign: newUserId) + ) - func fetchCurrentUserCan(_ capability: String) async throws -> Bool { - try await apiClient.api.users.retrieveMeWithEditContext().capabilities.keys.contains(capability) + // Remove the deleted user from the cached users list. + if result.deleted, let index = users?.firstIndex(where: { $0.id == id }) { + users?.remove(at: index) + } } - func fetchUsers(cachedResults: CachedUserListCallback? = nil) async throws -> [WordPressUI.DisplayUser] { - - if await !userCache.users.isEmpty { - await cachedResults?(await userCache.users) - } + func setNewPassword(id: Int32, newPassword: String) async throws { + _ = try await client.api.users.update( + userId: Int32(id), + params: UserUpdateParams(password: newPassword) + ) + } - let users: [DisplayUser] = try await apiClient - .api - .users - .listWithEditContext(params: UserListParams(perPage: 100)) - .compactMap { - - guard let role = $0.roles.first else { - return nil - } - - return DisplayUser( - id: $0.id, - handle: $0.slug, - username: $0.username, - firstName: $0.firstName, - lastName: $0.lastName, - displayName: $0.name, - profilePhotoUrl: profilePhotoUrl(for: $0), - role: role, - emailAddress: $0.email, - websiteUrl: $0.link, - biography: $0.description - ) - } - - await userCache.setUsers(users) +} - return users - } +private extension DisplayUser { + init?(user: UserWithEditContext) { + guard let role = user.roles.first else { + return nil + } - func invalidateCaches() async throws { - await userCache.clear() + self.init( + id: user.id, + handle: user.slug, + username: user.username, + firstName: user.firstName, + lastName: user.lastName, + displayName: user.name, + profilePhotoUrl: Self.profilePhotoUrl(for: user), + role: role, + emailAddress: user.email, + websiteUrl: user.link, + biography: user.description + ) } - func profilePhotoUrl(for user: UserWithEditContext) -> URL? { + static func profilePhotoUrl(for user: UserWithEditContext) -> URL? { // The key is the size of the avatar. Get the largetst one, which is 96x96px. // https://github.com/WordPress/wordpress-develop/blob/6.6.2/src/wp-includes/rest-api.php#L1253-L1260 guard let url = user.avatarUrls? diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift index 0d2ae83ea00f..65fe07c64dd6 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift +++ b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift @@ -159,14 +159,14 @@ extension BlogDetailsViewController { } @objc func showUsers() { - guard let presentationDelegate, let userId = self.blog.userID?.intValue else { + guard let presentationDelegate else { return } let feature = NSLocalizedString("applicationPasswordRequired.feature.users", value: "User Management", comment: "Feature name for managing users in the app") let rootView = ApplicationPasswordRequiredView(blog: self.blog, localizedFeatureName: feature) { client in - let service = UserService(api: client, currentUserId: userId) - return UserListView(userProvider: service, actionDispatcher: service.actionDispatcher) + let service = UserService(client: client) + return UserListView(userService: service) } presentationDelegate.presentBlogDetailsViewController(UIHostingController(rootView: rootView)) } diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index a09fd05df2b9..4f5c29b75ea8 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -454,6 +454,7 @@ 4A76A4BB29D4381100AABF4B /* CommentService+LikesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A76A4BA29D4381000AABF4B /* CommentService+LikesTests.swift */; }; 4A76A4BD29D43BFD00AABF4B /* CommentService+MorderationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A76A4BC29D43BFD00AABF4B /* CommentService+MorderationTests.swift */; }; 4A76A4BF29D4F0A500AABF4B /* reader-post-comments-success.json in Resources */ = {isa = PBXBuildFile; fileRef = 4A76A4BE29D4F0A500AABF4B /* reader-post-comments-success.json */; }; + 4A76F9112CE207FA0025F7FA /* UserServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A76F9102CE207FA0025F7FA /* UserServiceTests.swift */; }; 4A9314DC297790C300360232 /* PeopleServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A9314DB297790C300360232 /* PeopleServiceTests.swift */; }; 4A9948E229714EF1006282A9 /* AccountSettingsServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A9948E129714EF1006282A9 /* AccountSettingsServiceTests.swift */; }; 4AA7EE0F2ADF7367007D261D /* PostRepositoryPostsListTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AA7EE0E2ADF7367007D261D /* PostRepositoryPostsListTests.swift */; }; @@ -2340,6 +2341,7 @@ 4A76A4BA29D4381000AABF4B /* CommentService+LikesTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "CommentService+LikesTests.swift"; sourceTree = ""; }; 4A76A4BC29D43BFD00AABF4B /* CommentService+MorderationTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "CommentService+MorderationTests.swift"; sourceTree = ""; }; 4A76A4BE29D4F0A500AABF4B /* reader-post-comments-success.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "reader-post-comments-success.json"; sourceTree = ""; }; + 4A76F9102CE207FA0025F7FA /* UserServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserServiceTests.swift; sourceTree = ""; }; 4A9314DB297790C300360232 /* PeopleServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PeopleServiceTests.swift; sourceTree = ""; }; 4A98A9022C2274E100A2CE58 /* WordPressKitTests.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = WordPressKitTests.xcconfig; sourceTree = ""; }; 4A98A9032C2274E100A2CE58 /* WordPressKit.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = WordPressKit.xcconfig; sourceTree = ""; }; @@ -6407,6 +6409,7 @@ 4A9314DB297790C300360232 /* PeopleServiceTests.swift */, FEAA6F78298CE4A600ADB44C /* PluginJetpackProxyServiceTests.swift */, 1D91080629F847A2003F9A5E /* MediaServiceUpdateTests.m */, + 4A76F9102CE207FA0025F7FA /* UserServiceTests.swift */, ); name = Services; sourceTree = ""; @@ -10190,6 +10193,7 @@ 4AB6A3602B7C3EB500769115 /* PinghubWebSocketTests.swift in Sources */, F46546352AF550A20017E3D1 /* AllDomainsListItem+Helpers.swift in Sources */, FEA312842987FB0100FFD405 /* BlogJetpackTests.swift in Sources */, + 4A76F9112CE207FA0025F7FA /* UserServiceTests.swift in Sources */, 0C35FFF429CBA6DA00D224EB /* BlogDashboardPersonalizationViewModelTests.swift in Sources */, 7E8980B922E73F4000C567B0 /* EditorSettingsServiceTests.swift in Sources */, 1797373720EBAA4100377B4E /* RouteMatcherTests.swift in Sources */, diff --git a/WordPress/WordPressTest/UserServiceTests.swift b/WordPress/WordPressTest/UserServiceTests.swift new file mode 100644 index 000000000000..a00a4650f3ff --- /dev/null +++ b/WordPress/WordPressTest/UserServiceTests.swift @@ -0,0 +1,124 @@ +import Foundation +import XCTest +import OHHTTPStubs +import OHHTTPStubsSwift +import Combine +import WordPressUI + +@testable import WordPress + +class UserServiceTests: XCTestCase { + var service: UserService! + + override func setUpWithError() throws { + try super.setUpWithError() + + let client = try WordPressClient(api: .init(urlSession: .shared, baseUrl: .parse(input: "https://example.com"), authenticationStategy: .none), rootUrl: .parse(input: "https://example.com")) + service = UserService(client: client) + } + + override func tearDown() { + super.tearDown() + + HTTPStubs.removeAllStubs() + } + + func testMultipleFetchUsersTriggerOneUpdate() async throws { + stubSuccessfullUsersFetch() + + let expectation = XCTestExpectation(description: "Updated after fetch") + let task = Task.detached { [self] in + for await _ in self.service.usersUpdates { + expectation.fulfill() + } + } + + _ = try await [ + self.service.fetchUsers(), + self.service.fetchUsers(), + self.service.fetchUsers(), + self.service.fetchUsers(), + self.service.fetchUsers() + ] + + await fulfillment(of: [expectation], timeout: 0.3) + task.cancel() + } + + func testSequentialFetchUsersTriggerOneUpdateForEachFetch() async throws { + stubSuccessfullUsersFetch() + + let expectation = XCTestExpectation(description: "Updated after fetch") + expectation.expectedFulfillmentCount = 5 + let task = Task.detached { [self] in + for await _ in self.service.usersUpdates { + expectation.fulfill() + } + } + + for _ in 1...expectation.expectedFulfillmentCount { + _ = try await service.fetchUsers() + } + + await fulfillment(of: [expectation], timeout: 0.3) + + task.cancel() + } + + func testStreamTerminates() async throws { + stubSuccessfullUsersFetch() + + let termination = XCTestExpectation(description: "Stream has finished") + let task = Task.detached { [self] in + for await _ in self.service.usersUpdates { + // Do nothing + } + termination.fulfill() + } + + _ = try await service.fetchUsers() + _ = try await service.fetchUsers() + _ = try await service.fetchUsers() + + // Stream should be terminated once `service` is deallocated. + service = nil + + await fulfillment(of: [termination], timeout: 0.3) + + task.cancel() + } + + func testDeleteUserTriggersUsersUpdate() async throws { + stubSuccessfullUsersFetch() + stubDeleteUser(id: 34) + + _ = try await service.fetchUsers() + let userFetched = await service.users?.contains { $0.id == 34 } == true + XCTAssertTrue(userFetched) + + try await service.deleteUser(id: 34, reassigningPostsTo: 1) + let userDeleted = await service.users?.contains { $0.id == 34 } == false + XCTAssertTrue(userDeleted) + } + + private func stubSuccessfullUsersFetch() { + stub(condition: isPath("/wp-json/wp/v2/users")) { _ in + let json = #"[{"id":1,"username":"demo","name":"demo","first_name":"","last_name":"","email":"tony.li@automattic.com","url":"https:\/\/yellow-lemming-rail.jurassic.ninja","description":"","link":"https:\/\/yellow-lemming-rail.jurassic.ninja\/author\/demo\/","locale":"en_US","nickname":"demo","slug":"demo","roles":["administrator"],"registered_date":"2024-11-03T21:43:36+00:00","capabilities":{"switch_themes":true,"edit_themes":true,"activate_plugins":true,"edit_plugins":true,"edit_users":true,"edit_files":true,"manage_options":true,"moderate_comments":true,"manage_categories":true,"manage_links":true,"upload_files":true,"import":true,"unfiltered_html":true,"edit_posts":true,"edit_others_posts":true,"edit_published_posts":true,"publish_posts":true,"edit_pages":true,"read":true,"level_10":true,"level_9":true,"level_8":true,"level_7":true,"level_6":true,"level_5":true,"level_4":true,"level_3":true,"level_2":true,"level_1":true,"level_0":true,"edit_others_pages":true,"edit_published_pages":true,"publish_pages":true,"delete_pages":true,"delete_others_pages":true,"delete_published_pages":true,"delete_posts":true,"delete_others_posts":true,"delete_published_posts":true,"delete_private_posts":true,"edit_private_posts":true,"read_private_posts":true,"delete_private_pages":true,"edit_private_pages":true,"read_private_pages":true,"delete_users":true,"create_users":true,"unfiltered_upload":true,"edit_dashboard":true,"update_plugins":true,"delete_plugins":true,"install_plugins":true,"update_themes":true,"install_themes":true,"update_core":true,"list_users":true,"remove_users":true,"promote_users":true,"edit_theme_options":true,"delete_themes":true,"export":true,"administrator":true},"extra_capabilities":{"administrator":true},"avatar_urls":{"24":"https:\/\/secure.gravatar.com\/avatar\/ac05cde1cb014070c625b139e53a2899?s=24&d=mm&r=g","48":"https:\/\/secure.gravatar.com\/avatar\/ac05cde1cb014070c625b139e53a2899?s=48&d=mm&r=g","96":"https:\/\/secure.gravatar.com\/avatar\/ac05cde1cb014070c625b139e53a2899?s=96&d=mm&r=g"},"meta":{"persisted_preferences":{"core":{"isComplementaryAreaVisible":true},"core\/edit-post":{"welcomeGuide":false},"_modified":"2024-11-06T09:23:14.009Z"}},"_links":{"self":[{"href":"https:\/\/yellow-lemming-rail.jurassic.ninja\/wp-json\/wp\/v2\/users\/1"}],"collection":[{"href":"https:\/\/yellow-lemming-rail.jurassic.ninja\/wp-json\/wp\/v2\/users"}]}},{"id":34,"username":"user_1810","name":"User_2718","first_name":"","last_name":"","email":"user_5880@example.com","url":"","description":"","link":"https://yellow-lemming-rail.jurassic.ninja/author/user_1810/","locale":"en_US","nickname":"user_1810","slug":"user_1810","roles":["editor"],"registered_date":"2024-11-05T22:58:27+00:00","capabilities":{"moderate_comments":true,"manage_categories":true,"manage_links":true,"upload_files":true,"unfiltered_html":true,"edit_posts":true,"edit_others_posts":true,"edit_published_posts":true,"publish_posts":true,"edit_pages":true,"read":true,"level_7":true,"level_6":true,"level_5":true,"level_4":true,"level_3":true,"level_2":true,"level_1":true,"level_0":true,"edit_others_pages":true,"edit_published_pages":true,"publish_pages":true,"delete_pages":true,"delete_others_pages":true,"delete_published_pages":true,"delete_posts":true,"delete_others_posts":true,"delete_published_posts":true,"delete_private_posts":true,"edit_private_posts":true,"read_private_posts":true,"delete_private_pages":true,"edit_private_pages":true,"read_private_pages":true,"editor":true},"extra_capabilities":{"editor":true},"avatar_urls":{"24":"https://secure.gravatar.com/avatar/e1cf0e88fb26697102e09f3aa1fd40c7?s=24&d=mm&r=g","48":"https://secure.gravatar.com/avatar/e1cf0e88fb26697102e09f3aa1fd40c7?s=48&d=mm&r=g","96":"https://secure.gravatar.com/avatar/e1cf0e88fb26697102e09f3aa1fd40c7?s=96&d=mm&r=g"},"meta":{"persisted_preferences":[]}}]"# + return HTTPStubsResponse(data: json.data(using: .utf8)!, statusCode: 200, headers: ["Content-Type": "application/json"]) + } + } + + private func stubFailedUsersFetch() { + stub(condition: isPath("/wp-json/wp/v2/users")) { _ in + HTTPStubsResponse(error: URLError(.timedOut)) + } + } + + private func stubDeleteUser(id: Int32) { + stub(condition: isPath("/wp-json/wp/v2/users/\(id)")) { _ in + let json = #"{"deleted":true,"previous":{"id":34,"username":"user_1810","name":"User_2718","first_name":"","last_name":"","email":"user_5880@example.com","url":"","description":"","link":"https://yellow-lemming-rail.jurassic.ninja/author/user_1810/","locale":"en_US","nickname":"user_1810","slug":"user_1810","roles":["editor"],"registered_date":"2024-11-05T22:58:27+00:00","capabilities":{"moderate_comments":true,"manage_categories":true,"manage_links":true,"upload_files":true,"unfiltered_html":true,"edit_posts":true,"edit_others_posts":true,"edit_published_posts":true,"publish_posts":true,"edit_pages":true,"read":true,"level_7":true,"level_6":true,"level_5":true,"level_4":true,"level_3":true,"level_2":true,"level_1":true,"level_0":true,"edit_others_pages":true,"edit_published_pages":true,"publish_pages":true,"delete_pages":true,"delete_others_pages":true,"delete_published_pages":true,"delete_posts":true,"delete_others_posts":true,"delete_published_posts":true,"delete_private_posts":true,"edit_private_posts":true,"read_private_posts":true,"delete_private_pages":true,"edit_private_pages":true,"read_private_pages":true,"editor":true},"extra_capabilities":{"editor":true},"avatar_urls":{"24":"https://secure.gravatar.com/avatar/e1cf0e88fb26697102e09f3aa1fd40c7?s=24&d=mm&r=g","48":"https://secure.gravatar.com/avatar/e1cf0e88fb26697102e09f3aa1fd40c7?s=48&d=mm&r=g","96":"https://secure.gravatar.com/avatar/e1cf0e88fb26697102e09f3aa1fd40c7?s=96&d=mm&r=g"},"meta":{"persisted_preferences":[]}}}"# + return HTTPStubsResponse(data: json.data(using: .utf8)!, statusCode: 200, headers: ["Content-Type": "application/json"]) + } + + } +} From d86215d85178b5b29c8f6e9ae1b1fc8b392f40d2 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 05:58:48 -0500 Subject: [PATCH 06/28] Fix insets in Discover and make the whole scroll view interactive (#23800) --- .../ReaderDiscoverViewController.swift | 2 +- .../ReaderStreamViewController.swift | 20 ++-------- .../Headers/ReaderDiscoverHeaderView.swift | 38 ++++++++++++++----- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift index 57387fb37f7a..4bbdd1f5b3e0 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift @@ -97,7 +97,7 @@ class ReaderDiscoverViewController: UIViewController, ReaderDiscoverHeaderViewDe // Important to set before `viewDidLoad` streamVC.isEmbeddedInDiscover = true - streamVC.setHeaderView(headerView) + streamVC.preferredTableHeaderView = headerView addChild(streamVC) view.addSubview(streamVC.view) diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift index 58889bfe52ba..83a0d9c10e5c 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift @@ -178,6 +178,7 @@ import AutomatticTracks private var showConfirmation = true var isEmbeddedInDiscover = false + var preferredTableHeaderView: UIView? var isCompact = true { didSet { @@ -458,7 +459,8 @@ import AutomatticTracks // MARK: - Configuration / Topic Presentation private func configureStreamHeader() { - guard !isEmbeddedInDiscover else { + if let headerView = preferredTableHeaderView { + setHeaderView(headerView) // Important to set _after_ isCompact is set in viewDidLoad return } guard let headerView = headerForStream(readerTopic, container: tableViewController) else { @@ -476,22 +478,6 @@ import AutomatticTracks (headerView as? ReaderBaseHeaderView)?.isCompact = isCompact tableView.tableHeaderView = headerView streamHeader = headerView as? ReaderStreamHeader - - // This feels somewhat hacky, but it is the only way I found to insert a stack view into the header without breaking the autolayout constraints. - let centerConstraint = headerView.centerXAnchor.constraint(equalTo: tableView.centerXAnchor) - let topConstraint = headerView.topAnchor.constraint(equalTo: tableView.topAnchor) - let headerWidthConstraint = headerView.widthAnchor.constraint(equalTo: tableView.widthAnchor) - headerWidthConstraint.priority = UILayoutPriority(999) - centerConstraint.priority = UILayoutPriority(999) - - NSLayoutConstraint.activate([ - centerConstraint, - headerWidthConstraint, - topConstraint - ]) - - tableView.tableHeaderView?.layoutIfNeeded() - tableView.tableHeaderView = tableView.tableHeaderView } /// Updates the content based on the values of `readerTopic` and `contentType` diff --git a/WordPress/Classes/ViewRelated/Reader/Headers/ReaderDiscoverHeaderView.swift b/WordPress/Classes/ViewRelated/Reader/Headers/ReaderDiscoverHeaderView.swift index de72f9b400c5..eca49f1b5ff6 100644 --- a/WordPress/Classes/ViewRelated/Reader/Headers/ReaderDiscoverHeaderView.swift +++ b/WordPress/Classes/ViewRelated/Reader/Headers/ReaderDiscoverHeaderView.swift @@ -1,4 +1,5 @@ import UIKit +import WordPressUI protocol ReaderDiscoverHeaderViewDelegate: AnyObject { func readerDiscoverHeaderView(_ view: ReaderDiscoverHeaderView, didChangeSelection selection: ReaderDiscoverChannel) @@ -9,6 +10,7 @@ final class ReaderDiscoverHeaderView: ReaderBaseHeaderView, UITextViewDelegate { private let channelsStackView = UIStackView(spacing: 8, []) private let scrollView = UIScrollView() private var channelViews: [ReaderDiscoverChannelView] = [] + private var previousWidth: CGFloat? private var selectedChannel: ReaderDiscoverChannel? @@ -23,9 +25,16 @@ final class ReaderDiscoverHeaderView: ReaderBaseHeaderView, UITextViewDelegate { channelsStackView.pinEdges() scrollView.heightAnchor.constraint(equalTo: channelsStackView.heightAnchor).isActive = true - let stackView = UIStackView(axis: .vertical, spacing: 8, [titleView, scrollView]) - contentView.addSubview(stackView) - stackView.pinEdges() + contentView.addSubview(titleView) + addSubview(scrollView) // Gets added directly to the header to enable interactions when scrolled + + NSLayoutConstraint.activate([ + titleView.topAnchor.constraint(equalTo: contentView.topAnchor), + scrollView.topAnchor.constraint(equalTo: titleView.bottomAnchor, constant: 8), + scrollView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor), + ]) + titleView.pinEdges(.horizontal) + scrollView.pinEdges(.horizontal) titleView.titleLabel.text = SharedStrings.Reader.discover titleView.detailsTextView.attributedText = { @@ -39,14 +48,27 @@ final class ReaderDiscoverHeaderView: ReaderBaseHeaderView, UITextViewDelegate { return details }() titleView.detailsTextView.delegate = self - - updateStyle() } required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } + override func layoutSubviews() { + super.layoutSubviews() + + if previousWidth != bounds.width { + previousWidth = bounds.width + updateScrollViewInsets() + } + } + + private func updateScrollViewInsets() { + scrollView.contentInset.left = contentView.frame.minX - (isCompact ? 0 : 10) + scrollView.contentInset.right = frame.maxX - contentView.frame.maxX + scrollView.contentOffset = CGPoint(x: -scrollView.contentInset.left, y: 0) + } + func configure(channels: [ReaderDiscoverChannel]) { for view in channelViews { view.removeFromSuperview() @@ -64,11 +86,7 @@ final class ReaderDiscoverHeaderView: ReaderBaseHeaderView, UITextViewDelegate { override func didUpdateIsCompact(_ isCompact: Bool) { super.didUpdateIsCompact(isCompact) - updateStyle() - } - - private func updateStyle() { - scrollView.contentInset = UIEdgeInsets(.leading, isCompact ? 0 : -10) // Align the "channels" + updateScrollViewInsets() } // MARK: Channels From b20381ffc9e7c1b3ffdc63c8df19ec0add9973c6 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 06:04:06 -0500 Subject: [PATCH 07/28] Fix broken animations (#23807) --- WordPress/Classes/Utility/WPTableViewHandler.h | 1 + WordPress/Classes/Utility/WPTableViewHandler.m | 8 +++++++- .../Reader/Controllers/ReaderTableContent.swift | 12 ++++++++---- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/WordPress/Classes/Utility/WPTableViewHandler.h b/WordPress/Classes/Utility/WPTableViewHandler.h index f86fe1f800eb..252062620d2e 100644 --- a/WordPress/Classes/Utility/WPTableViewHandler.h +++ b/WordPress/Classes/Utility/WPTableViewHandler.h @@ -106,6 +106,7 @@ @property (nonatomic) UITableViewRowAnimation moveRowAnimation; @property (nonatomic) UITableViewRowAnimation sectionRowAnimation; @property (nonatomic) BOOL listensForContentChanges; +@property (nonatomic) BOOL disableAnimations; - (nonnull instancetype)initWithTableView:(nonnull UITableView *)tableView; - (void)clearCachedRowHeights; diff --git a/WordPress/Classes/Utility/WPTableViewHandler.m b/WordPress/Classes/Utility/WPTableViewHandler.m index 9c0554be482a..0a38306f90a8 100644 --- a/WordPress/Classes/Utility/WPTableViewHandler.m +++ b/WordPress/Classes/Utility/WPTableViewHandler.m @@ -635,6 +635,9 @@ - (void)controllerWillChangeContent:(NSFetchedResultsController *)controller } self.indexPathSelectedBeforeUpdates = [self.tableView indexPathForSelectedRow]; + if (self.disableAnimations) { + [UIView setAnimationsEnabled:NO]; + } [self.tableView beginUpdates]; } @@ -645,6 +648,9 @@ - (void)controllerDidChangeContent:(NSFetchedResultsController *)controller NSError *error; [WPException objcTryBlock:^{ [self.tableView endUpdates]; + if (self.disableAnimations) { + [UIView setAnimationsEnabled:YES]; + } } error:&error]; if (error) { @@ -697,7 +703,7 @@ - (void)controller:(NSFetchedResultsController *)controller // It seems in some cases newIndexPath can be nil for updates newIndexPath = indexPath; } - + switch(type) { case NSFetchedResultsChangeInsert: { diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableContent.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableContent.swift index c5525b89fbcd..4fc3a6de63ff 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableContent.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTableContent.swift @@ -6,10 +6,14 @@ final class ReaderTableContent { private var tableViewHandler: WPTableViewHandler? func initializeContent(tableView: UITableView, delegate: WPTableViewHandlerDelegate) { - tableViewHandler = WPTableViewHandler(tableView: tableView) - tableViewHandler?.cacheRowHeights = false - tableViewHandler?.updateRowAnimation = .none - tableViewHandler?.delegate = delegate + let tableViewHandler = WPTableViewHandler(tableView: tableView) + tableViewHandler.cacheRowHeights = false + tableViewHandler.updateRowAnimation = .none + tableViewHandler.moveRowAnimation = .none + tableViewHandler.insertRowAnimation = .none + tableViewHandler.disableAnimations = true + tableViewHandler.delegate = delegate + self.tableViewHandler = tableViewHandler } func resetResultsController() { From 4827198bfc8735b152ec1b7bf54ff8b1c4999eb3 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 06:09:48 -0500 Subject: [PATCH 08/28] Fix an issue with Reader not showing notification settings for some sites (#23809) --- .../Subscriptions/ReaderSubscriptionHelper.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift index 0574dcc86c02..12b82686d986 100644 --- a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift +++ b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift @@ -122,12 +122,14 @@ enum ReaderSubscriptionNotificationsStatus { case none init?(site: ReaderSiteTopic) { - guard let postSubscription = site.postSubscription, - let emailSubscription = site.emailSubscription else { + guard !site.isExternal else { return nil } - let sendPosts = postSubscription.sendPosts || emailSubscription.sendPosts - let sendComments = emailSubscription.sendComments + let posts = site.postSubscription + let emails = site.emailSubscription + + let sendPosts = (posts?.sendPosts ?? false) || (emails?.sendPosts ?? false) + let sendComments = emails?.sendComments ?? false if sendPosts && sendComments { self = .all } else if sendPosts || sendComments { From 73673b07bda560e2c200e62481a1baad74bf1789 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 06:10:23 -0500 Subject: [PATCH 09/28] Reader: Minor issue with subscritpions (#23810) * Fix leading inset in ReaderSubscriptionCell * Fix an issue with subscriptions not reloading after subscribing using URL * Fix subscriptions not reloading --- WordPress/Classes/Services/ReaderSiteService.h | 12 ------------ WordPress/Classes/Services/ReaderSiteService.m | 14 -------------- .../Reader/Controllers/ReaderHelpers.swift | 2 -- .../Subscriptions/ReaderSubscriptionCell.swift | 2 +- .../ReaderSubscriptionHelper.swift | 18 ++---------------- 5 files changed, 3 insertions(+), 45 deletions(-) diff --git a/WordPress/Classes/Services/ReaderSiteService.h b/WordPress/Classes/Services/ReaderSiteService.h index ba91c509d167..536b9f82351d 100644 --- a/WordPress/Classes/Services/ReaderSiteService.h +++ b/WordPress/Classes/Services/ReaderSiteService.h @@ -74,16 +74,4 @@ extern NSString * const ReaderSiteServiceErrorDomain; */ - (void)syncPostsForFollowedSites; -/** - Returns a ReaderSiteTopic for the given site URL. - - @param siteURL The URL of the site. - @param success block called on a successful fetch containing the ReaderSiteTopic. - @param failure block called if there is any error. `error` can be any underlying network error. - */ -- (void)topicWithSiteURL:(NSURL *)siteURL - success:(void (^)(ReaderSiteTopic *topic))success - failure:(void(^)(NSError *error))failure; - - @end diff --git a/WordPress/Classes/Services/ReaderSiteService.m b/WordPress/Classes/Services/ReaderSiteService.m index 2f1634600c62..5b866b668bae 100644 --- a/WordPress/Classes/Services/ReaderSiteService.m +++ b/WordPress/Classes/Services/ReaderSiteService.m @@ -160,20 +160,6 @@ - (void)syncPostsForFollowedSites }]; } -- (void)topicWithSiteURL:(NSURL *)siteURL success:(void (^)(ReaderSiteTopic *topic))success failure:(void(^)(NSError *error))failure -{ - WordPressComRestApi *api = [self apiForRequest]; - ReaderSiteServiceRemote *service = [[ReaderSiteServiceRemote alloc] initWithWordPressComRestApi:api]; - - [service findSiteIDForURL:siteURL success:^(NSUInteger siteID) { - NSNumber *site = [NSNumber numberWithUnsignedLong:siteID]; - ReaderSiteTopic *topic = [ReaderSiteTopic lookupWithSiteID:site inContext:self.coreDataStack.mainContext]; - success(topic); - } failure:^(NSError *error) { - failure(error); - }]; -} - #pragma mark - Private Methods diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift index 94e41a18c149..b9ae0f5a01b8 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderHelpers.swift @@ -8,8 +8,6 @@ import AutomatticTracks extension NSNotification.Name { // Sent when a site or a tag is unfollowed via Reader Manage screen. static let ReaderTopicUnfollowed = NSNotification.Name(rawValue: "ReaderTopicUnfollowed") - // Sent when a site is followed via Reader Manage screen. - static let ReaderSiteFollowed = NSNotification.Name(rawValue: "ReaderSiteFollowed") // Sent when a post's seen state has been toggled. static let ReaderPostSeenToggled = NSNotification.Name(rawValue: "ReaderPostSeenToggled") // Sent when a site is blocked. diff --git a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionCell.swift b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionCell.swift index a77363dad90e..7820f3d9a327 100644 --- a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionCell.swift +++ b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionCell.swift @@ -21,7 +21,7 @@ struct ReaderSubscriptionCell: View { HStack(spacing: 0) { HStack(spacing: 16) { ReaderSiteIconView(site: site, size: .regular) - .padding(.leading, 4) + .padding(.leading, horizontalSizeClass == .compact ? 0 : 4) VStack(alignment: .leading, spacing: 3) { HStack(alignment: .firstTextBaseline, spacing: 8) { diff --git a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift index 12b82686d986..ee25b2ad788b 100644 --- a/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift +++ b/WordPress/Classes/ViewRelated/Reader/Subscriptions/ReaderSubscriptionHelper.swift @@ -6,12 +6,8 @@ struct ReaderSubscriptionHelper { // MARK: Subscribe func toggleSiteSubscription(forPost post: ReaderPost) { - let siteURL = post.blogURL.flatMap(URL.init) ReaderFollowAction().execute(with: post, context: ContextManager.shared.mainContext, completion: { isFollowing in UINotificationFeedbackGenerator().notificationOccurred(.success) - if isFollowing, let siteURL { - postSiteFollowedNotification(siteURL: siteURL) - } ReaderHelpers.dispatchToggleFollowSiteMessage(post: post, follow: isFollowing, success: true) }, failure: { _, _ in UINotificationFeedbackGenerator().notificationOccurred(.error) @@ -32,7 +28,8 @@ struct ReaderSubscriptionHelper { try await withUnsafeThrowingContinuation { continuation in let service = ReaderSiteService(coreDataStack: contextManager) service.followSite(by: url, success: { - postSiteFollowedNotification(siteURL: url) + ReaderTopicService(coreDataStack: contextManager) + .fetchAllFollowedSites(success: {}, failure: { _ in }) generator.notificationOccurred(.success) continuation.resume(returning: ()) }, failure: { error in @@ -43,17 +40,6 @@ struct ReaderSubscriptionHelper { } } - private func postSiteFollowedNotification(siteURL: URL) { - let service = ReaderSiteService(coreDataStack: contextManager) - service.topic(withSiteURL: siteURL, success: { topic in - if let topic = topic { - NotificationCenter.default.post(name: .ReaderSiteFollowed, object: nil, userInfo: [ReaderNotificationKeys.topic: topic]) - } - }, failure: { error in - DDLogError("Unable to find topic by siteURL: \(String(describing: error?.localizedDescription))") - }) - } - // MARK: Subscribe/Unsubscribe (ReaderSiteTopic) func toggleFollowingForSite(_ topic: ReaderSiteTopic, completion: ((Bool) -> Void)? = nil) { From 4db876ba2703bb340c480e762dd557afbee04be4 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 06:16:53 -0500 Subject: [PATCH 10/28] Add context menus (#23808) --- WordPress/Classes/Utility/WPTableViewHandler.h | 1 + WordPress/Classes/Utility/WPTableViewHandler.m | 7 +++++++ .../ReaderDiscoverViewController.swift | 7 +++++++ .../ReaderStreamViewController.swift | 18 ++++++++++++++++++ 4 files changed, 33 insertions(+) diff --git a/WordPress/Classes/Utility/WPTableViewHandler.h b/WordPress/Classes/Utility/WPTableViewHandler.h index 252062620d2e..ad83e84b3f01 100644 --- a/WordPress/Classes/Utility/WPTableViewHandler.h +++ b/WordPress/Classes/Utility/WPTableViewHandler.h @@ -53,6 +53,7 @@ - (nullable UISwipeActionsConfiguration *)tableView:(nonnull UITableView *)tableView leadingSwipeActionsConfigurationForRowAtIndexPath:(nonnull NSIndexPath *)indexPath; - (nullable UISwipeActionsConfiguration *)tableView:(nonnull UITableView *)tableView trailingSwipeActionsConfigurationForRowAtIndexPath:(nonnull NSIndexPath *)indexPath; +- (nullable UIContextMenuConfiguration *)tableView:(nonnull UITableView *)tableView contextMenuConfigurationForRowAtIndexPath:(nonnull NSIndexPath *)indexPath point:(CGPoint)point; #pragma mark - Tracking the removal of views diff --git a/WordPress/Classes/Utility/WPTableViewHandler.m b/WordPress/Classes/Utility/WPTableViewHandler.m index 0a38306f90a8..abf0f16fc2e2 100644 --- a/WordPress/Classes/Utility/WPTableViewHandler.m +++ b/WordPress/Classes/Utility/WPTableViewHandler.m @@ -314,6 +314,13 @@ - (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath [self.delegate tableView:tableView didSelectRowAtIndexPath:indexPath]; } +- (UIContextMenuConfiguration *)tableView:(UITableView *)tableView contextMenuConfigurationForRowAtIndexPath:(NSIndexPath *)indexPath point:(CGPoint)point { + if ([self.delegate respondsToSelector:@selector(tableView:contextMenuConfigurationForRowAtIndexPath:point:)]) { + return [self.delegate tableView:tableView contextMenuConfigurationForRowAtIndexPath:indexPath point:point]; + } + return nil; +} + #pragma mark - Optional Delegate Methods diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift index 4bbdd1f5b3e0..69a461bb2d10 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift @@ -363,6 +363,13 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController { super.syncIfAppropriate(forceSync: true) tableView.reloadRows(at: [indexPath], with: UITableView.RowAnimation.fade) } + + override func getPost(at indexPath: IndexPath) -> ReaderPost? { + guard let card: ReaderCard = content.object(at: indexPath) else { + return nil + } + return card.post + } } // MARK: - ReaderRecommendedSitesCellDelegate diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift index 83a0d9c10e5c..ebb63c6dd294 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift @@ -1125,6 +1125,10 @@ import AutomatticTracks completion?(false) }) } + + func getPost(at indexPath: IndexPath) -> ReaderPost? { + content.object(at: indexPath) + } } // MARK: - ReaderStreamHeaderDelegate @@ -1433,6 +1437,20 @@ extension ReaderStreamViewController: WPTableViewHandlerDelegate { // Do nothing } + func tableView(_ tableView: UITableView, contextMenuConfigurationForRowAt indexPath: IndexPath, point: CGPoint) -> UIContextMenuConfiguration? { + guard let post = getPost(at: indexPath) else { + return nil + } + return UIContextMenuConfiguration(identifier: nil, previewProvider: nil) { [weak self] _ in + guard let self else { return nil } + return UIMenu(children: ReaderPostMenu( + post: post, + topic: readerTopic, + anchor: self.tableView.cellForRow(at: indexPath) ?? self.view, + viewController: self + ).makeMenu()) + } + } } // MARK: - SearchableActivity Conformance From 5dafb3d4248d056dba096642dd015b968a4800cd Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 07:28:49 -0500 Subject: [PATCH 11/28] Reader: Fix clipped recommended tags (#23806) * Add ReaderRecommendedTagsCell * Remove ReaderTopicsCardCell * Update design --- .../Cards/ReaderRecommendedSitesCell.swift | 6 +- .../Cards/ReaderRecommendedTagsCell.swift | 85 +++++ .../ReaderDiscoverViewController.swift | 19 +- .../Controllers/ReaderTopicsCardCell.swift | 329 ------------------ .../Controllers/ReaderTopicsCardCell.xib | 82 ----- .../ReaderInterestsStyleGuide.swift | 4 - 6 files changed, 97 insertions(+), 428 deletions(-) create mode 100644 WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedTagsCell.swift delete mode 100644 WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift delete mode 100644 WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.xib diff --git a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift index e448c6447791..4b8a21fd0ecb 100644 --- a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift +++ b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedSitesCell.swift @@ -3,7 +3,7 @@ import UIKit import WordPressUI import Combine -protocol ReaderRecommendedSitesCellDelegate: AnyObject { +protocol ReaderRecommendationsCellDelegate: AnyObject { func didSelect(topic: ReaderAbstractTopic) } @@ -53,7 +53,7 @@ final class ReaderRecommendedSitesCell: UITableViewCell { }()) } - func configure(with sites: [ReaderSiteTopic], delegate: ReaderRecommendedSitesCellDelegate) { + func configure(with sites: [ReaderSiteTopic], delegate: ReaderRecommendationsCellDelegate) { for site in sites { let siteView = ReaderRecommendedSitesCellView() siteView.configure(with: site) @@ -77,7 +77,7 @@ private final class ReaderRecommendedSitesCellView: UIView { return configuration }()) - weak var delegate: ReaderRecommendedSitesCellDelegate? + weak var delegate: ReaderRecommendationsCellDelegate? private let iconSize: SiteIconViewModel.Size = .regular private var site: ReaderSiteTopic? diff --git a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedTagsCell.swift b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedTagsCell.swift new file mode 100644 index 000000000000..c782bdd28c60 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderRecommendedTagsCell.swift @@ -0,0 +1,85 @@ +import SwiftUI +import UIKit +import WordPressUI + +final class ReaderRecommendedTagsCell: UITableViewCell { + private let scrollView = UIScrollView() + private let tagsStackView = UIStackView(axis: .horizontal, spacing: 8, insets: UIEdgeInsets(.vertical, 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 tagsStackView.subviews { + view.removeFromSuperview() + } + } + + private func setupView() { + selectionStyle = .none + + scrollView.showsHorizontalScrollIndicator = false + scrollView.clipsToBounds = false + + let backgroundView = UIView() + backgroundView.backgroundColor = .secondarySystemBackground + backgroundView.layer.cornerRadius = 8 + backgroundView.clipsToBounds = true + + contentView.addSubview(backgroundView) + backgroundView.pinEdges(insets: UIEdgeInsets(.all, 16)) + + let titleLabel = UILabel() + titleLabel.font = .preferredFont(forTextStyle: .subheadline) + titleLabel.textColor = .secondaryLabel + titleLabel.text = Strings.title + + let stackView = UIStackView(axis: .vertical, [titleLabel, scrollView]) + + scrollView.addSubview(tagsStackView) + tagsStackView.pinEdges() + scrollView.heightAnchor.constraint(equalTo: tagsStackView.heightAnchor).isActive = true + + backgroundView.addSubview(stackView) + stackView.pinEdges(insets: { + var insets = UIEdgeInsets(.all, 16) + insets.bottom = 6 // Covered by the scroll view + return insets + }()) + } + + func configure(with topics: [ReaderTagTopic], delegate: ReaderRecommendationsCellDelegate) { + for topic in topics { + var configuration = UIButton.Configuration.borderedTinted() + configuration.title = topic.title + configuration.cornerStyle = .capsule + configuration.baseForegroundColor = .label + configuration.titleTextAttributesTransformer = .init { + var container = $0 + container.font = UIFont.preferredFont(forTextStyle: .subheadline).withWeight(.medium) + return container + } + configuration.baseBackgroundColor = .secondaryLabel + + let button = UIButton(configuration: configuration) + button.addAction(.init(handler: { [weak delegate] _ in + delegate?.didSelect(topic: topic) + }), for: .primaryActionTriggered) + + tagsStackView.addArrangedSubview(button) + } + } +} + +private enum Strings { + static let title = NSLocalizedString("reader.suggested.tags.title", value: "You might like", comment: "A suggestion of topics (tags) the user might like") +} diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift index 69a461bb2d10..5729b75351fd 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderDiscoverViewController.swift @@ -195,7 +195,7 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController { // the superclass might trigger `layoutIfNeeded` from its `viewDidLoad`, and we want to make sure that // 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(ReaderRecommendedTagsCell.self, forCellReuseIdentifier: readerCardTopicsIdentifier) tableView.register(ReaderRecommendedSitesCell.self, forCellReuseIdentifier: readerCardSitesIdentifier) } @@ -232,9 +232,9 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController { return cell(for: post, at: indexPath, showsSeparator: shouldShowSeparator) case .topics: - return cell(for: card.topicsArray) + return makeRecommendedTagsCell(for: card.topicsArray) case .sites: - return cell(for: card.sitesArray) + return makeRecommendedSitesCell(for: card.sitesArray) case .unknown: return UITableViewCell() } @@ -254,15 +254,14 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController { } } - func cell(for interests: [ReaderTagTopic]) -> UITableViewCell { - let cell = tableView.dequeueReusableCell(withIdentifier: readerCardTopicsIdentifier) as! ReaderTopicsCardCell - cell.configure(interests) - cell.delegate = self + private func makeRecommendedTagsCell(for interests: [ReaderTagTopic]) -> UITableViewCell { + let cell = tableView.dequeueReusableCell(withIdentifier: readerCardTopicsIdentifier) as! ReaderRecommendedTagsCell + cell.configure(with: interests, delegate: self) hideSeparator(for: cell) return cell } - func cell(for sites: [ReaderSiteTopic]) -> UITableViewCell { + private func makeRecommendedSitesCell(for sites: [ReaderSiteTopic]) -> UITableViewCell { let cell = tableView.dequeueReusableCell(withIdentifier: readerCardSitesIdentifier) as! ReaderRecommendedSitesCell cell.configure(with: sites, delegate: self) hideSeparator(for: cell) @@ -372,9 +371,9 @@ private class ReaderDiscoverStreamViewController: ReaderStreamViewController { } } -// MARK: - ReaderRecommendedSitesCellDelegate +// MARK: - ReaderRecommendationsCellDelegate -extension ReaderDiscoverStreamViewController: ReaderRecommendedSitesCellDelegate { +extension ReaderDiscoverStreamViewController: ReaderRecommendationsCellDelegate { func didSelect(topic: ReaderAbstractTopic) { if topic as? ReaderTagTopic != nil { WPAnalytics.trackReader(.readerDiscoverTopicTapped) diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift deleted file mode 100644 index 5bfde0e5a421..000000000000 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.swift +++ /dev/null @@ -1,329 +0,0 @@ -import UIKit - -/// A cell that displays topics the user might like -/// -class ReaderTopicsCardCell: UITableViewCell, NibLoadable { - // Views - @IBOutlet weak var containerView: UIView! - @IBOutlet weak var headerLabel: UILabel! - @IBOutlet weak var collectionView: UICollectionView! - - private let layout = ReaderTagsCollectionViewLayout() - - private(set) var data: [ReaderAbstractTopic] = [] { - didSet { - guard oldValue != data else { - return - } - refreshData() - } - } - - weak var delegate: ReaderRecommendedSitesCellDelegate? - - static var defaultNibName: String { "ReaderTopicsCardCell" } - - func configure(_ data: [ReaderAbstractTopic]) { - self.data = data - } - - override func awakeFromNib() { - super.awakeFromNib() - - applyStyles() - - // Configure header - headerLabel.text = Constants.title - - // Configure collection view - collectionView.showsHorizontalScrollIndicator = false - collectionView.register(ReaderInterestsCollectionViewCell.defaultNib, - forCellWithReuseIdentifier: ReaderInterestsCollectionViewCell.defaultReuseID) - collectionView.register(ReaderTopicCardCollectionViewCell.self, - forCellWithReuseIdentifier: ReaderTopicCardCollectionViewCell.cellReuseIdentifier()) - - configureForNewDesign() - } - - private func applyStyles() { - headerLabel.font = WPStyleGuide.fontForTextStyle(.footnote) - - containerView.backgroundColor = .secondarySystemBackground - headerLabel.backgroundColor = .secondarySystemBackground - collectionView.backgroundColor = .secondarySystemBackground - - backgroundColor = .clear - contentView.backgroundColor = .systemBackground - } - - /// Configures the cell and the collection view for the new design. - private func configureForNewDesign() { - // set up custom collection view flow layout - layout.interitemSpacing = 8.0 - layout.lineSpacing = 8.0 - layout.delegate = self - collectionView.collectionViewLayout = layout - - // header title color - headerLabel.textColor = .secondaryLabel - - // corner radius - containerView.layer.cornerRadius = 10.0 - - backgroundColor = .systemBackground - contentView.backgroundColor = .systemBackground - } - - private func refreshData() { - collectionView.reloadData() - } - - private struct Constants { - static let title = NSLocalizedString("You might like", comment: "A suggestion of topics the user might like") - - static let reuseIdentifier = ReaderInterestsCollectionViewCell.defaultReuseID - } -} - -// MARK: - Collection View: Datasource & Delegate -extension ReaderTopicsCardCell: UICollectionViewDelegate, UICollectionViewDataSource { - func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { - guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: ReaderTopicCardCollectionViewCell.cellReuseIdentifier(), for: indexPath) as? ReaderTopicCardCollectionViewCell else { - return UICollectionViewCell() - } - - let title = data[indexPath.row].title - cell.titleLabel.text = title - - cell.accessibilityLabel = title - cell.accessibilityIdentifier = .topicsCardCellIdentifier - - return cell - } - - override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { - super.traitCollectionDidChange(previousTraitCollection) - - if traitCollection.hasDifferentColorAppearance(comparedTo: previousTraitCollection) { - refreshData() - } - } - - func numberOfSections(in collectionView: UICollectionView) -> Int { - return 1 - } - - func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int { - return data.count - } - - func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) { - let topic = data[indexPath.row] - - delegate?.didSelect(topic: topic) - } -} - -// MARK: - Collection View: Flow Layout Delegate - -extension ReaderTopicsCardCell: UICollectionViewDelegateFlowLayout { - func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize { - return sizeForCell(title: data[indexPath.row].title) - } - - // Calculates the dynamic size of the collection view cell based on the provided title - private func sizeForCell(title: String) -> CGSize { - let attributes: [NSAttributedString.Key: Any] = [ - .font: ReaderSuggestedTopicsStyleGuide.topicFont - ] - - let title: NSString = title as NSString - - var size = title.size(withAttributes: attributes) - size.height += (CellConstants.marginY * 2) + 2 - - // Prevent 1 token from being too long - let maxWidth = collectionView.bounds.width * CellConstants.maxWidthMultiplier - let width = min(size.width, maxWidth) - size.width = width + (CellConstants.marginX * 2) + 2 - - return size - } - - private struct CellConstants { - static let maxWidthMultiplier: CGFloat = 0.8 - static let marginX: CGFloat = 16 - static let marginY: CGFloat = 8 - } -} - -private extension String { - // MARK: Accessibility Identifiers Constants - static let topicsCardCellIdentifier = "topics-card-cell-button" -} - -// MARK: - New Collection View Cell - -class ReaderTopicCardCollectionViewCell: UICollectionViewCell, ReusableCell { - - lazy var titleLabel: UILabel = { - $0.translatesAutoresizingMaskIntoConstraints = false - $0.adjustsFontForContentSizeCategory = true - $0.font = WPStyleGuide.fontForTextStyle(.footnote) - $0.textColor = .label - $0.numberOfLines = 1 - $0.setContentCompressionResistancePriority(.defaultHigh, for: .horizontal) - return $0 - }(UILabel()) - - override init(frame: CGRect) { - super.init(frame: frame) - - isAccessibilityElement = true - - contentView.addSubview(titleLabel) - contentView.pinSubviewToAllEdges(titleLabel, insets: .init(top: 8.0, left: 16.0, bottom: 8.0, right: 16.0)) - - contentView.layer.cornerRadius = 5.0 - contentView.layer.borderWidth = 1.0 - updateColors() - } - - override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { - super.traitCollectionDidChange(previousTraitCollection) - - if let previousTraitCollection, - traitCollection.hasDifferentColorAppearance(comparedTo: previousTraitCollection) { - updateColors() - } - } - - required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") - } - - private func updateColors() { - contentView.backgroundColor = .clear - contentView.layer.borderColor = UIColor(light: .separator, dark: Constants.darkModeSeparatorColor).cgColor - } - - private struct Constants { - // This is a customized `.separator` color with the alpha updated to 1.0. - // With the default color on top of the gray background, the border appears almost invisible on certain devices. - // More context: p1697541472738849-slack-C05N140C8H5 - static let darkModeSeparatorColor = UIColor(red: 0.33, green: 0.33, blue: 0.35, alpha: 1.0) - } -} - -// MARK: - New Collection View Layout - -/// This tries to achieve a horizontal `UIStackView` layout that can automatically wrap into a new line. -/// -/// Note that this depends on the collection view cells having the same height. -/// Different cell heights are not supported yet by this layout. -class ReaderTagsCollectionViewLayout: UICollectionViewLayout { - - // MARK: Properties - - var interitemSpacing: CGFloat = .zero - - var lineSpacing: CGFloat = .zero - - private var isRightToLeft: Bool { - let layoutDirection = collectionView?.traitCollection.layoutDirection ?? .leftToRight - return layoutDirection == .rightToLeft - } - - weak var delegate: UICollectionViewDelegateFlowLayout? - - private var itemAttributes = [UICollectionViewLayoutAttributes]() - - private var contentHeight: CGFloat = .zero - - private var maxContentWidth: CGFloat { - guard let collectionView else { - return .zero - } - return collectionView.bounds.width - (collectionView.contentInset.left + collectionView.contentInset.right) - } - - override func layoutAttributesForItem(at indexPath: IndexPath) -> UICollectionViewLayoutAttributes? { - return itemAttributes[safe: indexPath.row] - } - - override func layoutAttributesForElements(in rect: CGRect) -> [UICollectionViewLayoutAttributes]? { - itemAttributes.filter { $0.frame.intersects(rect) } - } - - override var collectionViewContentSize: CGSize { - CGSize(width: maxContentWidth, height: contentHeight) - } - - override func shouldInvalidateLayout(forBoundsChange newBounds: CGRect) -> Bool { - guard let collectionView else { - return false - } - return collectionView.bounds.size != newBounds.size - } - - override func invalidateLayout() { - contentHeight = 0.0 - itemAttributes.removeAll() - super.invalidateLayout() - } - - override func prepare() { - super.prepare() - - // reset any stored attributes since we're doing a recalculation. - itemAttributes.removeAll() - - guard let collectionView else { - return - } - - let numberOfItems = collectionView.numberOfItems(inSection: .zero) - let insets = collectionView.contentInset - - var rowCount: CGFloat = 0 - var itemHeight: CGFloat = 0.0 - var availableLineWidth = maxContentWidth - - for index in 0.. availableLineWidth { - rowCount += 1 - availableLineWidth = maxContentWidth - } - - // at this point, we know that the item's width + line spacing fits into the current line. - frame.origin.y = insets.top + rowCount * (size.height + lineSpacing) - frame.origin.x = { - if isRightToLeft { - return availableLineWidth - size.width - insets.right - } - return insets.left + maxContentWidth - availableLineWidth - }() - - // add a layout attribute with the current item's frame. - let attribute = UICollectionViewLayoutAttributes(forCellWith: indexPath) - attribute.frame = frame - itemAttributes.append(attribute) - - // update the available line width for the next item. - availableLineWidth -= (interitemSpacing + size.width) - } - - // update total content height. - contentHeight = insets.top + ((rowCount + 1) * itemHeight) + (rowCount * lineSpacing) + insets.bottom - } - -} diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.xib b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.xib deleted file mode 100644 index 4c0ceb6ce8a6..000000000000 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderTopicsCardCell.xib +++ /dev/null @@ -1,82 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/WordPress/Classes/ViewRelated/Reader/Select Interests/ReaderInterestsStyleGuide.swift b/WordPress/Classes/ViewRelated/Reader/Select Interests/ReaderInterestsStyleGuide.swift index 063c0dbb39e9..0b4541a2e643 100644 --- a/WordPress/Classes/ViewRelated/Reader/Select Interests/ReaderInterestsStyleGuide.swift +++ b/WordPress/Classes/ViewRelated/Reader/Select Interests/ReaderInterestsStyleGuide.swift @@ -84,7 +84,3 @@ class ReaderInterestsStyleGuide { indicator.color = UIColor(light: .black, dark: .white) } } - -class ReaderSuggestedTopicsStyleGuide { - public static var topicFont: UIFont = WPStyleGuide.fontForTextStyle(.footnote) -} From e768dc4776695c3e9dc728384736efd22c1374d2 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 07:28:58 -0500 Subject: [PATCH 12/28] Refactor: Remove flipInsetsForRightToLeftLayoutDirection (#23811) * Remove one use of flipInsetsForRightToLeftLayoutDirection * Remove more uses of flipInsetsForRightToLeftLayoutDirection --- .../Classes/Extensions/UIEdgeInsets.swift | 23 ------------------- .../Detail/CommentContentTableViewCell.swift | 2 -- .../Branding/Button/JetpackButton.swift | 1 - .../Detail/Views/ReaderDetailToolbar.swift | 13 ----------- 4 files changed, 39 deletions(-) diff --git a/WordPress/Classes/Extensions/UIEdgeInsets.swift b/WordPress/Classes/Extensions/UIEdgeInsets.swift index ba676201db43..14fdff59b180 100644 --- a/WordPress/Classes/Extensions/UIEdgeInsets.swift +++ b/WordPress/Classes/Extensions/UIEdgeInsets.swift @@ -5,7 +5,6 @@ extension UIEdgeInsets { guard UIApplication.shared.userInterfaceLayoutDirection == .rightToLeft else { return self } - return flippedForRightToLeftLayoutDirection() } @@ -13,25 +12,3 @@ extension UIEdgeInsets { return UIEdgeInsets(top: top, left: right, bottom: bottom, right: left) } } - -extension UIButton { - - @objc func flipInsetsForRightToLeftLayoutDirection() { - guard userInterfaceLayoutDirection() == .rightToLeft else { - return - } - contentEdgeInsets = contentEdgeInsets.flippedForRightToLeftLayoutDirection() - imageEdgeInsets = imageEdgeInsets.flippedForRightToLeftLayoutDirection() - titleEdgeInsets = titleEdgeInsets.flippedForRightToLeftLayoutDirection() - } -} - -// Hack: Since UIEdgeInsets is a struct in ObjC, you can't have methods on it. -// You can't also export top level functions from Swift. -// 🙄 -@objc(InsetsHelper) -class _InsetsHelper: NSObject { - @objc static func flipForRightToLeftLayoutDirection(_ insets: UIEdgeInsets) -> UIEdgeInsets { - return insets.flippedForRightToLeftLayoutDirection() - } -} diff --git a/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift b/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift index 295490db7a9e..7d6730d9f6d0 100644 --- a/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift +++ b/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift @@ -385,7 +385,6 @@ private extension CommentContentTableViewCell { replyButton?.setTitleColor(Style.reactionButtonTextColor, for: .normal) replyButton?.setImage(Style.replyIconImage, for: .normal) replyButton?.addTarget(self, action: #selector(replyButtonTapped), for: .touchUpInside) - replyButton?.flipInsetsForRightToLeftLayoutDirection() replyButton?.adjustsImageSizeForAccessibilityContentSizeCategory = true adjustImageAndTitleEdgeInsets(for: replyButton) replyButton?.sizeToFit() @@ -397,7 +396,6 @@ private extension CommentContentTableViewCell { likeButton?.titleLabel?.adjustsFontForContentSizeCategory = true likeButton?.setTitleColor(Style.reactionButtonTextColor, for: .normal) likeButton?.addTarget(self, action: #selector(likeButtonTapped), for: .touchUpInside) - likeButton?.flipInsetsForRightToLeftLayoutDirection() likeButton?.adjustsImageSizeForAccessibilityContentSizeCategory = true adjustImageAndTitleEdgeInsets(for: likeButton) updateLikeButton(liked: false, numberOfLikes: 0) diff --git a/WordPress/Classes/ViewRelated/Jetpack/Branding/Button/JetpackButton.swift b/WordPress/Classes/ViewRelated/Jetpack/Branding/Button/JetpackButton.swift index ed99af3020c8..13bb85af02dd 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Branding/Button/JetpackButton.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Branding/Button/JetpackButton.swift @@ -78,7 +78,6 @@ class JetpackButton: CircularImageButton { imageEdgeInsets = Appearance.iconInsets contentEdgeInsets = Appearance.contentInsets imageView?.contentMode = .scaleAspectFit - flipInsetsForRightToLeftLayoutDirection() setImageBackgroundColor(imageBackgroundColor) } diff --git a/WordPress/Classes/ViewRelated/Reader/Detail/Views/ReaderDetailToolbar.swift b/WordPress/Classes/ViewRelated/Reader/Detail/Views/ReaderDetailToolbar.swift index 1371ff20a10d..8a472c977706 100644 --- a/WordPress/Classes/ViewRelated/Reader/Detail/Views/ReaderDetailToolbar.swift +++ b/WordPress/Classes/ViewRelated/Reader/Detail/Views/ReaderDetailToolbar.swift @@ -53,8 +53,6 @@ class ReaderDetailToolbar: UIView, NibLoadable { applyStyles() - adjustInsetsForTextDirection() - prepareActionButtonsForVoiceOver() } @@ -376,17 +374,6 @@ class ReaderDetailToolbar: UIView, NibLoadable { configureActionButtonStyle(saveForLaterButton) } - private func adjustInsetsForTextDirection() { - let buttonsToAdjust: [UIButton] = [ - likeButton, - commentButton, - saveForLaterButton, - reblogButton] - for button in buttonsToAdjust { - button.flipInsetsForRightToLeftLayoutDirection() - } - } - fileprivate func configureButtonTitles() { let commentTitle = Constants.commentButtonTitle From ba5f4afb2f484d3f83d38fb6d5d4cc1d51bcb764 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Fri, 15 Nov 2024 07:47:46 -0500 Subject: [PATCH 13/28] Remove a couple more FFs (#23805) * Remove .autoSafeDrafts FF * Remove .readingPreferencesFeedback --- .../Utility/Analytics/WPAnalyticsEvent.swift | 3 - .../BuildInformation/FeatureFlag.swift | 4 -- .../BuildInformation/RemoteFeatureFlag.swift | 7 --- .../ViewRelated/Post/PostEditor+Publish.swift | 12 ++-- .../Classes/ViewRelated/Post/PostEditor.swift | 6 +- .../ReaderDisplaySettingViewController.swift | 57 ------------------- 6 files changed, 5 insertions(+), 84 deletions(-) diff --git a/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift b/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift index 8dedc2b19b6c..e7208515fa58 100644 --- a/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift +++ b/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift @@ -598,7 +598,6 @@ import Foundation // Reading preferences case readingPreferencesOpened - case readingPreferencesFeedbackTapped case readingPreferencesItemTapped case readingPreferencesSaved case readingPreferencesClosed @@ -1648,8 +1647,6 @@ import Foundation // Reading Preferences case .readingPreferencesOpened: return "reader_reading_preferences_opened" - case .readingPreferencesFeedbackTapped: - return "reader_reading_preferences_feedback_tapped" case .readingPreferencesItemTapped: return "reader_reading_preferences_item_tapped" case .readingPreferencesSaved: diff --git a/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift b/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift index 0daeebeda050..953f89506fd3 100644 --- a/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift +++ b/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift @@ -9,7 +9,6 @@ enum FeatureFlag: Int, CaseIterable { case commentModerationUpdate case compliancePopover case googleDomainsCard - case autoSaveDrafts case voiceToContent case authenticateUsingApplicationPassword case tipKit @@ -39,8 +38,6 @@ enum FeatureFlag: Int, CaseIterable { return true case .googleDomainsCard: return false - case .autoSaveDrafts: - return false case .voiceToContent: return AppConfiguration.isJetpack && BuildConfiguration.current ~= [.localDeveloper, .a8cBranchTest] case .authenticateUsingApplicationPassword: @@ -84,7 +81,6 @@ extension FeatureFlag { case .commentModerationUpdate: "Comments Moderation Update" case .compliancePopover: "Compliance Popover" case .googleDomainsCard: "Google Domains Promotional Card" - case .autoSaveDrafts: "Autosave Drafts" case .voiceToContent: "Voice to Content" case .authenticateUsingApplicationPassword: "Application Passwords for self-hosted sites" case .tipKit: "TipKit" diff --git a/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift b/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift index d38ac65db635..b618b603550a 100644 --- a/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift +++ b/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift @@ -26,7 +26,6 @@ enum RemoteFeatureFlag: Int, CaseIterable { case wordPressSotWCard case inAppRating case siteMonitoring - case readingPreferencesFeedback case inAppUpdates case dotComWebLogin @@ -80,8 +79,6 @@ enum RemoteFeatureFlag: Int, CaseIterable { return false case .siteMonitoring: return false - case .readingPreferencesFeedback: - return true case .inAppUpdates: return false case .dotComWebLogin: @@ -140,8 +137,6 @@ enum RemoteFeatureFlag: Int, CaseIterable { return "in_app_rating_and_feedback" case .siteMonitoring: return "site_monitoring" - case .readingPreferencesFeedback: - return "reading_preferences_feedback" case .inAppUpdates: return "in_app_updates" case .dotComWebLogin: @@ -199,8 +194,6 @@ enum RemoteFeatureFlag: Int, CaseIterable { return "In-App Rating and Feedback" case .siteMonitoring: return "Site Monitoring" - case .readingPreferencesFeedback: - return "Reading Preferences Feedback" case .inAppUpdates: return "In-App Updates" case .dotComWebLogin: diff --git a/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift b/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift index 5c6752d09867..2984433caf4c 100644 --- a/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift +++ b/WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift @@ -189,14 +189,10 @@ extension PublishingEditor { } if post.original().isStatus(in: [.draft, .pending]) { - if FeatureFlag.autoSaveDrafts.enabled { - performSaveDraftAction() - } else { - // The "Discard Changes" behavior is problematic due to the way - // the editor and `PostCoordinator` often update the content - // in the background without the user interaction. - showCloseDraftConfirmationAlert() - } + // The "Discard Changes" behavior is problematic due to the way + // the editor and `PostCoordinator` often update the content + // in the background without the user interaction. + showCloseDraftConfirmationAlert() } else { showClosePublishedPostConfirmationAlert() } diff --git a/WordPress/Classes/ViewRelated/Post/PostEditor.swift b/WordPress/Classes/ViewRelated/Post/PostEditor.swift index 6d20e904d22a..bbe24267d949 100644 --- a/WordPress/Classes/ViewRelated/Post/PostEditor.swift +++ b/WordPress/Classes/ViewRelated/Post/PostEditor.swift @@ -163,11 +163,7 @@ extension PostEditor where Self: UIViewController { if !editorHasChanges { AbstractPost.deleteLatestRevision(post, in: context) } else { - if FeatureFlag.autoSaveDrafts.enabled, PostCoordinator.shared.isSyncAllowed(for: post) { - PostCoordinator.shared.setNeedsSync(for: post) - } else { - EditPostViewController.encode(post: post) - } + EditPostViewController.encode(post: post) } if context.hasChanges { ContextManager.sharedInstance().saveContextAndWait(context) diff --git a/WordPress/Classes/ViewRelated/Reader/Theme/ReaderDisplaySettingViewController.swift b/WordPress/Classes/ViewRelated/Reader/Theme/ReaderDisplaySettingViewController.swift index 08c83154836f..afe9a012e9ac 100644 --- a/WordPress/Classes/ViewRelated/Reader/Theme/ReaderDisplaySettingViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Theme/ReaderDisplaySettingViewController.swift @@ -226,18 +226,6 @@ extension ReaderDisplaySettingSelectionView { .font(Font(viewModel.displaySetting.font(with: .callout))) .foregroundStyle(viewModel.foregroundColor) - if let feedbackText { - feedbackText - .font(Font(viewModel.displaySetting.font(with: .callout))) - .foregroundStyle(viewModel.foregroundColor) - .tint(Color(linkTintColor)) - .accessibilityAddTraits(.isLink) - .environment(\.openURL, OpenURLAction { url in - WPAnalytics.track(.readingPreferencesFeedbackTapped) - return .systemAction - }) - } - tagsView Spacer() @@ -260,30 +248,6 @@ extension ReaderDisplaySettingSelectionView { .animation(.easeInOut, value: viewModel.displaySetting) } - var feedbackText: Text? { - guard AppConfiguration.isJetpack, - RemoteFeatureFlag.readingPreferencesFeedback.enabled() else { - return nil - } - - var linkString = "[\(Strings.feedbackLinkCTA)](\(Constants.feedbackLinkString))" - if viewModel.displaySetting.color != .system { - // for color themes other than the default, we'll mark it bold. - linkString = "**\(linkString)**" - } - - let string = String(format: Strings.feedbackLineFormat, linkString) - guard var attributedString = try? AttributedString(markdown: string) else { - return nil - } - - if let rangeOfLink = attributedString.range(of: Strings.feedbackLinkCTA) { - attributedString[rangeOfLink].underlineStyle = .single - } - - return Text(attributedString) - } - var linkTintColor: UIColor { viewModel.displaySetting.color == .system ? UIAppColor.blue : viewModel.displaySetting.color.foreground } @@ -310,7 +274,6 @@ extension ReaderDisplaySettingSelectionView { private struct Constants { static let gradientMaskHeight = 32.0 - static let feedbackLinkString = "https://automattic.survey.fm/reader-customization-survey" } private struct Strings { @@ -326,26 +289,6 @@ extension ReaderDisplaySettingSelectionView { comment: "Description text for the preview section of Reader Preferences" ) - static let feedbackLineFormat = NSLocalizedString( - "reader.preferences.preview.body.feedback.format", - value: "This is a new feature still in development. To help us improve it %1$@.", - comment: """ - Text format for the feedback line text, to be displayed in the preview section. - %1$@ is a placeholder for a call-to-action that completes the line, which will be filled programmatically. - Example: 'This is a new feature still in development. To help us improve it send your feedback.' - """ - ) - - static let feedbackLinkCTA = NSLocalizedString( - "reader.preferences.preview.body.feedback.link", - value: "send your feedback", - comment: """ - A call-to-action text fragment to ask the user provide feedback for the Reading Preferences feature. - Note that the lowercase format is intended, as this will be injected to form a full paragraph. - Refer to: `reader.preferences.preview.body.feedback.format` - """ - ) - static let tags = [ NSLocalizedString("reader.preferences.preview.tags.reading", value: "reading", comment: "Example tag for preview"), NSLocalizedString("reader.preferences.preview.tags.colors", value: "colors", comment: "Example tag for preview"), From 41b8062cc57f45ce511c57ac9ca56738d90fe1e9 Mon Sep 17 00:00:00 2001 From: Pinar Olguc Date: Fri, 15 Nov 2024 16:25:30 +0300 Subject: [PATCH 14/28] Integrate Gravatar Quick Editor (#23729) * Add remote FF gravatar_quick_editor * Add forceRefresh option for image download methods * Force refresh on gravatar update notification * Add `GravatarQuickEditorPresenter` * Display QuickEditor instead of the avatar upload menu * Move listenForGravatarChanges to upper level Add forceRefresh option * Add one more FF check * Adjust the obj-c call * Listen to changes on the MeHeaderView * Await the image download to update the cached image * Listen to gravatar changes on the signup epilogue * Add one more FF check * Separate the notification name for the QE Add email check when handling the notification `GravatarQEAvatarUpdateNotification` * Add a release note * Revert whitespace change * Use overrideImageCache` * Fix the old avatar issue * Update unit test * Move `addObserver` to viewDidLoad * Mode addObserver to init` * Add unit tests for `appendingGravatarCacheBusterParam` and handle the canonical URL as well * Fix "unused var" warning * Update release notes * Revert "Update release notes" This reverts commit f0b3eb971980ad3f918b2c28bfdf8cebfefb518b. --- .../Utility/Notification+Gravatar.swift | 20 ++++++++ RELEASE-NOTES.txt | 1 + .../Classes/Extensions/URL+Helpers.swift | 11 +++++ .../BuildInformation/RemoteFeatureFlag.swift | 7 +++ .../Media/ImageDownloader+Gravatar.swift | 11 +++-- .../Utility/Media/ImageDownloader.swift | 9 ++++ .../Utility/Media/ImageViewController.swift | 2 +- .../Classes/Utility/Media/MemoryCache.swift | 6 +++ .../BlogDetailsViewController+Me.swift | 12 ++++- .../Blog Details/BlogDetailsViewController.m | 2 +- .../Gravatar/UIImageView+Gravatar.swift | 33 ++++++++++--- .../GravatarQuickEditorPresenter.swift | 46 ++++++++++++++++++ .../Me/My Profile/MyProfileHeaderView.swift | 33 +++++++++++-- .../My Profile/MyProfileViewController.swift | 22 +++++---- .../ViewRelated/Me/Views/MeHeaderView.swift | 18 ++++++- .../Epilogues/EpilogueUserInfoCell.swift | 48 ++++++++++++++----- .../System/WPTabBarController+MeTab.swift | 13 ++++- WordPress/WordPress.xcodeproj/project.pbxproj | 4 ++ .../Extensions/URLHelpersTests.swift | 23 +++++++++ .../WordPressTest/ImageDownloaderTests.swift | 4 ++ .../LoginLinkRequestViewController.swift | 15 +++++- .../Signin/UIImageView+Additions.swift | 11 +++-- .../GravatarEmailTableViewCell.swift | 26 +++++++++- 23 files changed, 329 insertions(+), 48 deletions(-) create mode 100644 Modules/Sources/WordPressShared/Utility/Notification+Gravatar.swift create mode 100644 WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/GravatarQuickEditorPresenter.swift create mode 100644 WordPress/WordPressTest/Extensions/URLHelpersTests.swift diff --git a/Modules/Sources/WordPressShared/Utility/Notification+Gravatar.swift b/Modules/Sources/WordPressShared/Utility/Notification+Gravatar.swift new file mode 100644 index 000000000000..b59717aa7e1b --- /dev/null +++ b/Modules/Sources/WordPressShared/Utility/Notification+Gravatar.swift @@ -0,0 +1,20 @@ +import Foundation + +public enum GravatarQEAvatarUpdateNotificationKeys: String { + case email +} + +public extension NSNotification.Name { + /// Gravatar Quick Editor updated the avatar + static let GravatarQEAvatarUpdateNotification = NSNotification.Name(rawValue: "GravatarQEAvatarUpdateNotification") +} + +extension Foundation.Notification { + public func userInfoHasEmail(_ email: String) -> Bool { + guard let userInfo = userInfo, + let notificationEmail = userInfo[GravatarQEAvatarUpdateNotificationKeys.email.rawValue] as? String else { + return false + } + return email == notificationEmail + } +} diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index eb36b72b51af..a58a20e37bb4 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,6 +1,7 @@ 25.6 ----- * [*] [internal] Update Gravatar SDK to 3.0.0 [#23701] +* [*] Use the Gravatar Quick Editor to update the avatar [#23729] * [*] (Hidden under a feature flag) User Management for self-hosted sites. [#23768] 25.5 diff --git a/WordPress/Classes/Extensions/URL+Helpers.swift b/WordPress/Classes/Extensions/URL+Helpers.swift index 22aff7d8bd81..0eb0102a8e9a 100644 --- a/WordPress/Classes/Extensions/URL+Helpers.swift +++ b/WordPress/Classes/Extensions/URL+Helpers.swift @@ -151,4 +151,15 @@ extension URL { components?.queryItems = queryItems return components?.url ?? self } + + /// Gravatar doesn't support "Cache-Control: none" header. So we add a random query parameter to + /// bypass the backend cache and get the latest image. + public func appendingGravatarCacheBusterParam() -> URL { + var urlComponents = URLComponents(url: self, resolvingAgainstBaseURL: false) + if urlComponents?.queryItems == nil { + urlComponents?.queryItems = [] + } + urlComponents?.queryItems?.append(.init(name: "_", value: "\(NSDate().timeIntervalSince1970)")) + return urlComponents?.url ?? self + } } diff --git a/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift b/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift index b618b603550a..1fe3dbbe8e9d 100644 --- a/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift +++ b/WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift @@ -27,6 +27,7 @@ enum RemoteFeatureFlag: Int, CaseIterable { case inAppRating case siteMonitoring case inAppUpdates + case gravatarQuickEditor case dotComWebLogin var defaultValue: Bool { @@ -81,6 +82,8 @@ enum RemoteFeatureFlag: Int, CaseIterable { return false case .inAppUpdates: return false + case .gravatarQuickEditor: + return BuildConfiguration.current ~= [.localDeveloper, .a8cBranchTest, .a8cPrereleaseTesting] case .dotComWebLogin: return false } @@ -139,6 +142,8 @@ enum RemoteFeatureFlag: Int, CaseIterable { return "site_monitoring" case .inAppUpdates: return "in_app_updates" + case .gravatarQuickEditor: + return "gravatar_quick_editor" case .dotComWebLogin: return "jp_wpcom_web_login" } @@ -196,6 +201,8 @@ enum RemoteFeatureFlag: Int, CaseIterable { return "Site Monitoring" case .inAppUpdates: return "In-App Updates" + case .gravatarQuickEditor: + return "Gravatar Quick Editor" case .dotComWebLogin: return "Log in to WordPress.com from web browser" } diff --git a/WordPress/Classes/Utility/Media/ImageDownloader+Gravatar.swift b/WordPress/Classes/Utility/Media/ImageDownloader+Gravatar.swift index a4eb3ef71596..e90a8b8fc8df 100644 --- a/WordPress/Classes/Utility/Media/ImageDownloader+Gravatar.swift +++ b/WordPress/Classes/Utility/Media/ImageDownloader+Gravatar.swift @@ -4,19 +4,22 @@ import Gravatar extension ImageDownloader { - nonisolated func downloadGravatarImage(with email: String, completion: @escaping (UIImage?) -> Void) { + nonisolated func downloadGravatarImage(with email: String, forceRefresh: Bool = false, completion: @escaping (UIImage?) -> Void) { guard let url = AvatarURL.url(for: email) else { completion(nil) return } - if let cachedImage = ImageCache.shared.getImage(forKey: url.absoluteString) { + if !forceRefresh, let cachedImage = ImageCache.shared.getImage(forKey: url.absoluteString) { completion(cachedImage) return } - - downloadImage(at: url) { image, _ in + var urlToDownload = url + if forceRefresh { + urlToDownload = url.appendingGravatarCacheBusterParam() + } + downloadImage(at: urlToDownload) { image, _ in DispatchQueue.main.async { guard let image else { diff --git a/WordPress/Classes/Utility/Media/ImageDownloader.swift b/WordPress/Classes/Utility/Media/ImageDownloader.swift index 58fce0d42621..f81deb20962d 100644 --- a/WordPress/Classes/Utility/Media/ImageDownloader.swift +++ b/WordPress/Classes/Utility/Media/ImageDownloader.swift @@ -102,6 +102,15 @@ actor ImageDownloader { return imageURL.absoluteString + (size.map { "?size=\($0)" } ?? "") } + func clearURLSessionCache() { + urlSessionWithCache.configuration.urlCache?.removeAllCachedResponses() + urlSession.configuration.urlCache?.removeAllCachedResponses() + } + + func clearMemoryCache() { + self.cache.removeAllObjects() + } + // MARK: - Networking private func data(for request: URLRequest, options: ImageRequestOptions) async throws -> Data { diff --git a/WordPress/Classes/Utility/Media/ImageViewController.swift b/WordPress/Classes/Utility/Media/ImageViewController.swift index a749f1e6dba1..583c3335d92c 100644 --- a/WordPress/Classes/Utility/Media/ImageViewController.swift +++ b/WordPress/Classes/Utility/Media/ImageViewController.swift @@ -6,7 +6,7 @@ final class ImageViewController { var downloader: ImageDownloader = .shared var onStateChanged: (State) -> Void = { _ in } - private var task: Task? + private(set) var task: Task? enum State { case loading diff --git a/WordPress/Classes/Utility/Media/MemoryCache.swift b/WordPress/Classes/Utility/Media/MemoryCache.swift index f7824ad98d25..4eda923d339e 100644 --- a/WordPress/Classes/Utility/Media/MemoryCache.swift +++ b/WordPress/Classes/Utility/Media/MemoryCache.swift @@ -4,10 +4,12 @@ import WordPressUI protocol MemoryCacheProtocol: AnyObject { subscript(key: String) -> UIImage? { get set } + func removeAllObjects() } /// - note: The type is thread-safe because it uses thread-safe `NSCache`. final class MemoryCache: MemoryCacheProtocol, @unchecked Sendable { + /// A shared image cache used by the entire system. static let shared = MemoryCache() @@ -23,6 +25,10 @@ final class MemoryCache: MemoryCacheProtocol, @unchecked Sendable { cache.removeAllObjects() } + func removeAllObjects() { + cache.removeAllObjects() + } + // MARK: - UIImage subscript(key: String) -> UIImage? { diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+Me.swift b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+Me.swift index 40b98d2cc756..24b18203ca53 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+Me.swift +++ b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+Me.swift @@ -4,12 +4,12 @@ import Gravatar extension BlogDetailsViewController { - @objc func downloadGravatarImage(for row: BlogDetailsRow) { + @objc func downloadGravatarImage(for row: BlogDetailsRow, forceRefresh: Bool = false) { guard let email = blog.account?.email else { return } - ImageDownloader.shared.downloadGravatarImage(with: email) { [weak self] image in + ImageDownloader.shared.downloadGravatarImage(with: email, forceRefresh: forceRefresh) { [weak self] image in guard let image, let gravatarIcon = image.gravatarIcon(size: Metrics.iconSize) else { return @@ -21,9 +21,17 @@ extension BlogDetailsViewController { } @objc func observeGravatarImageUpdate() { + NotificationCenter.default.addObserver(self, selector: #selector(refreshAvatar(_:)), name: .GravatarQEAvatarUpdateNotification, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(updateGravatarImage(_:)), name: .GravatarImageUpdateNotification, object: nil) } + @objc private func refreshAvatar(_ notification: Foundation.Notification) { + guard let meRow, + let email = blog.account?.email, + notification.userInfoHasEmail(email) else { return } + downloadGravatarImage(for: meRow, forceRefresh: true) + } + @objc private func updateGravatarImage(_ notification: Foundation.Notification) { guard let userInfo = notification.userInfo, let email = userInfo["email"] as? String, diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m index bb5c5f49151f..ae5084002b65 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m +++ b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m @@ -1350,7 +1350,7 @@ - (BlogDetailsSection *)configurationSectionViewModel callback:^{ [weakSelf showMe]; }]; - [self downloadGravatarImageFor:row]; + [self downloadGravatarImageFor:row forceRefresh: NO]; self.meRow = row; [rows addObject:row]; } diff --git a/WordPress/Classes/ViewRelated/Gravatar/UIImageView+Gravatar.swift b/WordPress/Classes/ViewRelated/Gravatar/UIImageView+Gravatar.swift index 47c9cad67b5a..43065c180f51 100644 --- a/WordPress/Classes/ViewRelated/Gravatar/UIImageView+Gravatar.swift +++ b/WordPress/Classes/ViewRelated/Gravatar/UIImageView+Gravatar.swift @@ -34,16 +34,18 @@ extension UIImageView { /// - email: The user's email /// - gravatarRating: Expected image rating /// - placeholderImage: Image to be used as Placeholder + /// - forceRefresh: Skip the cache and fetch the latest version of the avatar. public func downloadGravatar( for email: String, gravatarRating: Rating = .general, - placeholderImage: UIImage = .gravatarPlaceholderImage + placeholderImage: UIImage = .gravatarPlaceholderImage, + forceRefresh: Bool = false ) { let avatarURL = AvatarURL.url(for: email, preferredSize: .pixels(gravatarDefaultSize()), gravatarRating: gravatarRating) - downloadGravatar(fullURL: avatarURL, placeholder: placeholderImage, animate: false) + downloadGravatar(fullURL: avatarURL, placeholder: placeholderImage, animate: false, forceRefresh: forceRefresh) } - public func downloadGravatar(_ gravatar: AvatarURL?, placeholder: UIImage, animate: Bool) { + public func downloadGravatar(_ gravatar: AvatarURL?, placeholder: UIImage, animate: Bool, forceRefresh: Bool = false) { guard let gravatar = gravatar else { self.image = placeholder return @@ -56,14 +58,31 @@ extension UIImageView { layoutIfNeeded() let size = Int(ceil(frame.width * min(2, UIScreen.main.scale))) - let url = gravatar.replacing(options: .init(preferredSize: .pixels(size)))?.url - downloadGravatar(fullURL: url, placeholder: placeholder, animate: animate) + guard let url = gravatar.replacing(options: .init(preferredSize: .pixels(size)))?.url else { return } + downloadGravatar(fullURL: url, placeholder: placeholder, animate: animate, forceRefresh: forceRefresh) } - private func downloadGravatar(fullURL: URL?, placeholder: UIImage, animate: Bool) { + private func downloadGravatar(fullURL: URL?, placeholder: UIImage, animate: Bool, forceRefresh: Bool = false) { wp.prepareForReuse() if let fullURL { - wp.setImage(with: fullURL) + var urlToDownload = fullURL + if forceRefresh { + urlToDownload = fullURL.appendingGravatarCacheBusterParam() + } + + wp.setImage(with: urlToDownload) + + if forceRefresh { + // If this is a `forceRefresh`, the cache for the original URL should be updated too. + // Because the cache buster parameter modifies the download URL. + Task { + await wp.controller.task?.value // Wait until setting the new image is done. + if let image { + ImageCache.shared.setImage(image, forKey: fullURL.absoluteString) // Update the cache for the original URL + } + } + } + if image == nil { // If image wasn't found synchronously in memory cache image = placeholder } diff --git a/WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/GravatarQuickEditorPresenter.swift b/WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/GravatarQuickEditorPresenter.swift new file mode 100644 index 000000000000..519806648a35 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/GravatarQuickEditorPresenter.swift @@ -0,0 +1,46 @@ +import Foundation +import GravatarUI +import WordPressShared +import WordPressAuthenticator + +@MainActor +struct GravatarQuickEditorPresenter { + let email: String + let authToken: String + + init?(email: String) { + let context = ContextManager.sharedInstance().mainContext + guard let account = try? WPAccount.lookupDefaultWordPressComAccount(in: context) else { + return nil + } + self.email = email + self.authToken = account.authToken + } + + func presentQuickEditor(on presentingViewController: UIViewController) { + let presenter = QuickEditorPresenter( + email: Email(email), + scope: .avatarPicker(AvatarPickerConfiguration(contentLayout: .horizontal())), + configuration: .init( + interfaceStyle: presentingViewController.traitCollection.userInterfaceStyle + ), + token: authToken + ) + presenter.present( + in: presentingViewController, + onAvatarUpdated: { + AuthenticatorAnalyticsTracker.shared.track(click: .selectAvatar) + Task { + // Purge the cache otherwise the old avatars remain around. + await ImageDownloader.shared.clearURLSessionCache() + await ImageDownloader.shared.clearMemoryCache() + NotificationCenter.default.post(name: .GravatarQEAvatarUpdateNotification, + object: self, + userInfo: [GravatarQEAvatarUpdateNotificationKeys.email.rawValue: email]) + } + }, onDismiss: { + // No op. + } + ) + } +} diff --git a/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileHeaderView.swift b/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileHeaderView.swift index 249d9283bab0..a0f662e89062 100644 --- a/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileHeaderView.swift +++ b/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileHeaderView.swift @@ -5,7 +5,7 @@ class MyProfileHeaderView: UITableViewHeaderFooterView { // MARK: - Public Properties and Outlets @IBOutlet var gravatarImageView: CircularImageView! @IBOutlet var gravatarButton: UIButton! - + weak var presentingViewController: UIViewController? // A fake button displayed on top of gravatarImageView. let imageViewButton = UIButton(type: .system) @@ -24,8 +24,8 @@ class MyProfileHeaderView: UITableViewHeaderFooterView { } var gravatarEmail: String? = nil { didSet { - if let email = gravatarEmail { - gravatarImageView.downloadGravatar(for: email, gravatarRating: .x) + if gravatarEmail != nil { + downloadAvatar() } } } @@ -51,10 +51,23 @@ class MyProfileHeaderView: UITableViewHeaderFooterView { configureGravatarButton() } + private func downloadAvatar(forceRefresh: Bool = false) { + if let email = gravatarEmail { + gravatarImageView.downloadGravatar(for: email, gravatarRating: .x, forceRefresh: forceRefresh) + } + } + + @objc private func refreshAvatar(_ notification: Foundation.Notification) { + guard let email = gravatarEmail, + notification.userInfoHasEmail(email) else { return } + downloadAvatar(forceRefresh: true) + } + /// Overrides the current Gravatar Image (set via Email) with a given image reference. /// Plus, the internal image cache is updated, to prevent undesired glitches upon refresh. /// func overrideGravatarImage(_ image: UIImage) { + guard !RemoteFeatureFlag.gravatarQuickEditor.enabled() else { return } gravatarImageView.image = image // Note: @@ -81,9 +94,23 @@ class MyProfileHeaderView: UITableViewHeaderFooterView { gravatarImageView.addSubview(imageViewButton) imageViewButton.translatesAutoresizingMaskIntoConstraints = false imageViewButton.pinSubviewToAllEdges(gravatarImageView) + if RemoteFeatureFlag.gravatarQuickEditor.enabled() { + imageViewButton.addTarget(self, action: #selector(gravatarButtonTapped), for: .touchUpInside) + NotificationCenter.default.addObserver(self, selector: #selector(refreshAvatar), name: .GravatarQEAvatarUpdateNotification, object: nil) + } } private func configureGravatarButton() { gravatarButton.tintColor = UIAppColor.primary + if RemoteFeatureFlag.gravatarQuickEditor.enabled() { + gravatarButton.addTarget(self, action: #selector(gravatarButtonTapped), for: .touchUpInside) + } + } + + @objc private func gravatarButtonTapped() { + guard let email = gravatarEmail, + let presenter = GravatarQuickEditorPresenter(email: email), + let presentingViewController else { return } + presenter.presentQuickEditor(on: presentingViewController) } } diff --git a/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileViewController.swift b/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileViewController.swift index 8b50453ad97e..8c010ce5d3a4 100644 --- a/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileViewController.swift +++ b/WordPress/Classes/ViewRelated/Me/My Profile/MyProfileViewController.swift @@ -15,17 +15,19 @@ func MyProfileViewController(account: WPAccount, service: AccountSettingsService let controller = MyProfileController(account: account, service: service, headerView: headerView) let viewController = ImmuTableViewController(controller: controller, style: .insetGrouped) controller.tableView = viewController.tableView + headerView.presentingViewController = viewController + if !RemoteFeatureFlag.gravatarQuickEditor.enabled() { + let menuController = AvatarMenuController(viewController: viewController) + menuController.onAvatarSelected = { [weak controller, weak viewController] image in + guard let controller, let viewController else { return } + controller.uploadGravatarImage(image, presenter: viewController) + } + objc_setAssociatedObject(viewController, &associateObjectKey, menuController, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) - let menuController = AvatarMenuController(viewController: viewController) - menuController.onAvatarSelected = { [weak controller, weak viewController] image in - guard let controller, let viewController else { return } - controller.uploadGravatarImage(image, presenter: viewController) - } - objc_setAssociatedObject(viewController, &associateObjectKey, menuController, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) - - for button in [headerView.imageViewButton, headerView.gravatarButton] as [UIButton] { - button.menu = menuController.makeMenu() - button.showsMenuAsPrimaryAction = true + for button in [headerView.imageViewButton, headerView.gravatarButton] as [UIButton] { + button.menu = menuController.makeMenu() + button.showsMenuAsPrimaryAction = true + } } viewController.tableView.tableHeaderView = headerView return viewController diff --git a/WordPress/Classes/ViewRelated/Me/Views/MeHeaderView.swift b/WordPress/Classes/ViewRelated/Me/Views/MeHeaderView.swift index 17579ff0cfaa..714a1276a7ab 100644 --- a/WordPress/Classes/ViewRelated/Me/Views/MeHeaderView.swift +++ b/WordPress/Classes/ViewRelated/Me/Views/MeHeaderView.swift @@ -43,6 +43,7 @@ final class MeHeaderView: UIView { // tableView.headerView inevitably has to break something $0.priority = UILayoutPriority(999) } + NotificationCenter.default.addObserver(self, selector: #selector(refreshAvatar), name: .GravatarQEAvatarUpdateNotification, object: nil) } required init?(coder: NSCoder) { @@ -63,14 +64,27 @@ final class MeHeaderView: UIView { titleLabel.text = viewModel.displayName detailsLabel.text = viewModel.username - if let gravatarEmail = viewModel.gravatarEmail { - iconView.downloadGravatar(for: gravatarEmail, gravatarRating: .x) + if viewModel.gravatarEmail != nil { + downloadAvatar() } else { iconView.image = nil } } + private func downloadAvatar(forceRefresh: Bool = false) { + if let email = viewModel?.gravatarEmail { + iconView.downloadGravatar(for: email, gravatarRating: .x, forceRefresh: forceRefresh) + } + } + + @objc private func refreshAvatar(_ notification: Foundation.Notification) { + guard let email = viewModel?.gravatarEmail, + notification.userInfoHasEmail(email) else { return } + downloadAvatar(forceRefresh: true) + } + func overrideGravatarImage(_ image: UIImage) { + guard !RemoteFeatureFlag.gravatarQuickEditor.enabled() else { return } iconView.image = image // Note: diff --git a/WordPress/Classes/ViewRelated/NUX/Views/Epilogues/EpilogueUserInfoCell.swift b/WordPress/Classes/ViewRelated/NUX/Views/Epilogues/EpilogueUserInfoCell.swift index e5f69122fd23..23e39c0810e9 100644 --- a/WordPress/Classes/ViewRelated/NUX/Views/Epilogues/EpilogueUserInfoCell.swift +++ b/WordPress/Classes/ViewRelated/NUX/Views/Epilogues/EpilogueUserInfoCell.swift @@ -16,16 +16,19 @@ class EpilogueUserInfoCell: UITableViewCell { private var gravatarStatus: GravatarUploaderStatus = .idle private var email: String? private var avatarMenuController: AnyObject? + private var allowGravatarUploads: Bool = false override func awakeFromNib() { super.awakeFromNib() configureImages() configureColors() + NotificationCenter.default.addObserver(self, selector: #selector(refreshAvatar), name: .GravatarQEAvatarUpdateNotification, object: nil) } /// Configures the cell so that the LoginEpilogueUserInfo's payload is displayed /// func configure(userInfo: LoginEpilogueUserInfo, showEmail: Bool = false, allowGravatarUploads: Bool = false, viewController: UIViewController) { + self.allowGravatarUploads = allowGravatarUploads email = userInfo.email self.viewController = viewController @@ -59,23 +62,46 @@ class EpilogueUserInfoCell: UITableViewCell { if let gravatarUrl = userInfo.gravatarUrl, let url = URL(string: gravatarUrl) { gravatarView.downloadImage(from: url) } else { - let placeholder: UIImage = allowGravatarUploads ? .gravatarUploadablePlaceholderImage : .gravatarPlaceholderImage - gravatarView.downloadGravatar(for: userInfo.email, gravatarRating: .x, placeholderImage: placeholder) + downloadGravatar() } } } private func setupGravatarButton(viewController: UIViewController) { - let menuController = AvatarMenuController(viewController: viewController) - menuController.onAvatarSelected = { [weak self] in - self?.uploadGravatarImage($0) + if RemoteFeatureFlag.gravatarQuickEditor.enabled() { + gravatarButton.addTarget(self, action: #selector(gravatarButtonTapped), for: .touchUpInside) } - self.avatarMenuController = menuController // Just retaining it - gravatarButton.menu = menuController.makeMenu() - gravatarButton.showsMenuAsPrimaryAction = true - gravatarButton.addAction(UIAction { _ in - AuthenticatorAnalyticsTracker.shared.track(click: .selectAvatar) - }, for: .menuActionTriggered) + else { + let menuController = AvatarMenuController(viewController: viewController) + menuController.onAvatarSelected = { [weak self] in + self?.uploadGravatarImage($0) + } + self.avatarMenuController = menuController // Just retaining it + gravatarButton.menu = menuController.makeMenu() + gravatarButton.showsMenuAsPrimaryAction = true + gravatarButton.addAction(UIAction { _ in + AuthenticatorAnalyticsTracker.shared.track(click: .selectAvatar) + }, for: .menuActionTriggered) + } + } + + @objc private func gravatarButtonTapped() { + guard let email, + let presenter = GravatarQuickEditorPresenter(email: email), + let viewController else { return } + presenter.presentQuickEditor(on: viewController) + } + + private func downloadGravatar(forceRefresh: Bool = false) { + let placeholder: UIImage = allowGravatarUploads ? .gravatarUploadablePlaceholderImage : .gravatarPlaceholderImage + if let email { + gravatarView.downloadGravatar(for: email, gravatarRating: .x, placeholderImage: placeholder, forceRefresh: forceRefresh) + } + } + + @objc private func refreshAvatar(_ notification: Foundation.Notification) { + guard let email, notification.userInfoHasEmail(email) else { return } + downloadGravatar(forceRefresh: true) } /// Starts the Activity Indicator Animation, and hides the Username + Fullname labels. diff --git a/WordPress/Classes/ViewRelated/System/WPTabBarController+MeTab.swift b/WordPress/Classes/ViewRelated/System/WPTabBarController+MeTab.swift index ab2b8e2761f5..6303daff9eaa 100644 --- a/WordPress/Classes/ViewRelated/System/WPTabBarController+MeTab.swift +++ b/WordPress/Classes/ViewRelated/System/WPTabBarController+MeTab.swift @@ -9,6 +9,8 @@ extension WPTabBarController { } @objc func observeGravatarImageUpdate() { + NotificationCenter.default.addObserver(self, selector: #selector(refreshAvatar(_:)), name: .GravatarQEAvatarUpdateNotification, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(updateGravatarImage(_:)), name: .GravatarImageUpdateNotification, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(accountDidChange), name: .WPAccountDefaultWordPressComAccountChanged, object: nil) @@ -18,13 +20,16 @@ extension WPTabBarController { @objc func configureMeTabImage(placeholderImage: UIImage?) { meNavigationController.tabBarItem.image = placeholderImage + downloadImage() + } + func downloadImage(forceRefresh: Bool = false) { guard let account = defaultAccount(), let email = account.email else { return } - ImageDownloader.shared.downloadGravatarImage(with: email) { [weak self] image in + ImageDownloader.shared.downloadGravatarImage(with: email, forceRefresh: forceRefresh) { [weak self] image in guard let image else { return } @@ -33,6 +38,12 @@ extension WPTabBarController { } } + @objc private func refreshAvatar(_ notification: Foundation.Notification) { + guard let email = defaultAccount()?.email, + notification.userInfoHasEmail(email) else { return } + downloadImage(forceRefresh: true) + } + @objc private func updateGravatarImage(_ notification: Foundation.Notification) { guard let userInfo = notification.userInfo, let email = userInfo["email"] as? String, diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index 4f5c29b75ea8..ad89d935a845 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -997,6 +997,7 @@ 8BEE845A27B1DC9D0001A93C /* dashboard-200-with-drafts-and-scheduled.json in Resources */ = {isa = PBXBuildFile; fileRef = 8BEE845927B1DC9D0001A93C /* dashboard-200-with-drafts-and-scheduled.json */; }; 8BFE36FF230F1C850061EBA8 /* AbstractPost+fixLocalMediaURLsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8BFE36FE230F1C850061EBA8 /* AbstractPost+fixLocalMediaURLsTests.swift */; }; 91BE834E2C48FF0F00BB5B3B /* UIImageView+Additions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91BE834D2C48FF0F00BB5B3B /* UIImageView+Additions.swift */; }; + 91CFB9552CE21196005CD369 /* URLHelpersTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91CFB9542CE21196005CD369 /* URLHelpersTests.swift */; }; 931215E1267DE1C0008C3B69 /* StatsTotalRowDataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 931215E0267DE1C0008C3B69 /* StatsTotalRowDataTests.swift */; }; 931D26F619ED7F7000114F17 /* BlogServiceTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 930FD0A519882742000CC81D /* BlogServiceTest.m */; }; 931D26F719ED7F7500114F17 /* ReaderPostServiceTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 5DE8A0401912D95B00B2FF59 /* ReaderPostServiceTest.m */; }; @@ -2802,6 +2803,7 @@ 8DE205D2AC15F16289E7D21A /* Pods-WordPressDraftActionExtension.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-WordPressDraftActionExtension.release.xcconfig"; path = "../Pods/Target Support Files/Pods-WordPressDraftActionExtension/Pods-WordPressDraftActionExtension.release.xcconfig"; sourceTree = ""; }; 9149D34BF5182F360C84EDB9 /* Pods-JetpackDraftActionExtension.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-JetpackDraftActionExtension.debug.xcconfig"; path = "../Pods/Target Support Files/Pods-JetpackDraftActionExtension/Pods-JetpackDraftActionExtension.debug.xcconfig"; sourceTree = ""; }; 91BE834D2C48FF0F00BB5B3B /* UIImageView+Additions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIImageView+Additions.swift"; sourceTree = ""; }; + 91CFB9542CE21196005CD369 /* URLHelpersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLHelpersTests.swift; sourceTree = ""; }; 92B40A77F0765C1E93B11727 /* Pods_WordPressDraftActionExtension.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_WordPressDraftActionExtension.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 930C6374182BD86400976C21 /* WordPress-Internal-Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = "WordPress-Internal-Info.plist"; sourceTree = ""; }; 930FD0A519882742000CC81D /* BlogServiceTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = BlogServiceTest.m; sourceTree = ""; }; @@ -7070,6 +7072,7 @@ 0CD6299A2B9AAA9A00325EA4 /* Foundation+Extensions.swift */, 8BFE36FE230F1C850061EBA8 /* AbstractPost+fixLocalMediaURLsTests.swift */, AE3047A9270B66D300FE9266 /* Scanner+QuotedTextTests.swift */, + 91CFB9542CE21196005CD369 /* URLHelpersTests.swift */, ); path = Extensions; sourceTree = ""; @@ -10137,6 +10140,7 @@ B532ACCF1DC3AB8E00FFFA57 /* NotificationSyncMediatorTests.swift in Sources */, 7E4A772F20F7FDF8001C706D /* ActivityLogTestData.swift in Sources */, 3F759FBC2A2DB2CF0039A845 /* TestError.swift in Sources */, + 91CFB9552CE21196005CD369 /* URLHelpersTests.swift in Sources */, B59D40A61DB522DF003D2D79 /* NSAttributedStringTests.swift in Sources */, F565190323CF6D1D003FACAF /* WKCookieJarTests.swift in Sources */, C738CB0D28623F07001BE107 /* QRLoginURLParserTests.swift in Sources */, diff --git a/WordPress/WordPressTest/Extensions/URLHelpersTests.swift b/WordPress/WordPressTest/Extensions/URLHelpersTests.swift new file mode 100644 index 000000000000..e702e74e510c --- /dev/null +++ b/WordPress/WordPressTest/Extensions/URLHelpersTests.swift @@ -0,0 +1,23 @@ +import XCTest +import WordPress + +class URLHelpersTests: XCTestCase { + + func testAddCacheBusterToExistingQueryParameters() async throws { + try await doTest("https://gravatar.com/avatar/1234?s=80") + } + + func testAddCacheBusterToCanonicalURL() async throws { + try await doTest("https://gravatar.com") + } + + func doTest(_ urlString: String) async throws { + let url = try XCTUnwrap(URL(string: urlString)) + let newURL = url.appendingGravatarCacheBusterParam() + XCTAssertNotEqual(url.absoluteString, newURL.absoluteString) + + let components = URLComponents(url: newURL, resolvingAgainstBaseURL: false) + let cacheBusterQueryItem = components?.queryItems?.filter { $0.name == "_" }.first + XCTAssertNotNil(cacheBusterQueryItem?.value) + } +} diff --git a/WordPress/WordPressTest/ImageDownloaderTests.swift b/WordPress/WordPressTest/ImageDownloaderTests.swift index 1f7911b590fa..12786eeb8ec5 100644 --- a/WordPress/WordPressTest/ImageDownloaderTests.swift +++ b/WordPress/WordPressTest/ImageDownloaderTests.swift @@ -158,4 +158,8 @@ private final class MockMemoryCache: MemoryCacheProtocol { get { cache[key] } set { cache[key] = newValue } } + + func removeAllObjects() { + cache = [:] + } } diff --git a/WordPressAuthenticator/Sources/Signin/LoginLinkRequestViewController.swift b/WordPressAuthenticator/Sources/Signin/LoginLinkRequestViewController.swift index df6bc5f6f30d..2fc94a0f35e4 100644 --- a/WordPressAuthenticator/Sources/Signin/LoginLinkRequestViewController.swift +++ b/WordPressAuthenticator/Sources/Signin/LoginLinkRequestViewController.swift @@ -30,6 +30,7 @@ class LoginLinkRequestViewController: LoginViewController { if !email.isValidEmail() { assert(email.isValidEmail(), "The value of loginFields.username was not a valid email address.") } + NotificationCenter.default.addObserver(self, selector: #selector(refreshAvatar), name: .GravatarQEAvatarUpdateNotification, object: nil) } override func viewWillAppear(_ animated: Bool) { @@ -38,7 +39,7 @@ class LoginLinkRequestViewController: LoginViewController { let email = loginFields.username if email.isValidEmail() { Task { - try await gravatarView?.setGravatarImage(with: email, rating: .x) + try await downloadAvatar() } } else { gravatarView?.isHidden = true @@ -50,6 +51,18 @@ class LoginLinkRequestViewController: LoginViewController { WordPressAuthenticator.track(.loginMagicLinkRequestFormViewed) } + private func downloadAvatar(forceRefresh: Bool = false) async throws { + let email = loginFields.username + try await gravatarView?.setGravatarImage(with: email, rating: .x, forceRefresh: forceRefresh) + } + + @objc private func refreshAvatar(_ notification: Foundation.Notification) { + guard notification.userInfoHasEmail(loginFields.username) else { return } + Task { + try await downloadAvatar(forceRefresh: true) + } + } + // MARK: - Configuration /// Assigns localized strings to various UIControl defined in the storyboard. diff --git a/WordPressAuthenticator/Sources/Signin/UIImageView+Additions.swift b/WordPressAuthenticator/Sources/Signin/UIImageView+Additions.swift index 807740611632..02c99799b714 100644 --- a/WordPressAuthenticator/Sources/Signin/UIImageView+Additions.swift +++ b/WordPressAuthenticator/Sources/Signin/UIImageView+Additions.swift @@ -3,12 +3,17 @@ import WordPressUI import GravatarUI extension UIImageView { - func setGravatarImage(with email: String, placeholder: UIImage = .gravatarPlaceholderImage, rating: Rating = .general, preferredSize: CGSize? = nil) async throws { + func setGravatarImage(with email: String, placeholder: UIImage = .gravatarPlaceholderImage, rating: Rating = .general, preferredSize: CGSize? = nil, forceRefresh: Bool = false) async throws { listenForGravatarChanges(forEmail: email) + var options: [ImageSettingOption] = [] + if forceRefresh { + options.append(.forceRefresh) + } try await gravatar.setImage(avatarID: .email(email), placeholder: placeholder, - rating: .x, + rating: rating, preferredSize: preferredSize, - defaultAvatarOption: .status404) + defaultAvatarOption: .status404, + options: options) } } diff --git a/WordPressAuthenticator/Sources/Unified Auth/View Related/Reusable Views/GravatarEmailTableViewCell.swift b/WordPressAuthenticator/Sources/Unified Auth/View Related/Reusable Views/GravatarEmailTableViewCell.swift index 179ce951b07f..1b7c0ca94136 100644 --- a/WordPressAuthenticator/Sources/Unified Auth/View Related/Reusable Views/GravatarEmailTableViewCell.swift +++ b/WordPressAuthenticator/Sources/Unified Auth/View Related/Reusable Views/GravatarEmailTableViewCell.swift @@ -20,10 +20,19 @@ class GravatarEmailTableViewCell: UITableViewCell { /// public static let reuseIdentifier = "GravatarEmailTableViewCell" public var onChangeSelectionHandler: ((_ sender: UITextField) -> Void)? + private var gravatarPlaceholderImage: UIImage? = nil + private var gravatarPreferredSize: CGSize = .zero + private var email: String? + + required init?(coder: NSCoder) { + super.init(coder: coder) + NotificationCenter.default.addObserver(self, selector: #selector(refreshAvatar), name: .GravatarQEAvatarUpdateNotification, object: nil) + } /// Public Methods /// public func configure(withEmail email: String?, andPlaceholder placeholderImage: UIImage? = nil, hasBorders: Bool = false) { + self.email = email gravatarImageView?.tintColor = WordPressAuthenticator.shared.unifiedStyle?.borderColor ?? WordPressAuthenticator.shared.style.primaryNormalBorderColor emailLabel?.textColor = WordPressAuthenticator.shared.unifiedStyle?.gravatarEmailTextColor ?? WordPressAuthenticator.shared.unifiedStyle?.textSubtleColor ?? WordPressAuthenticator.shared.style.subheadlineColor emailLabel?.font = UIFont.preferredFont(forTextStyle: .body) @@ -36,9 +45,10 @@ class GravatarEmailTableViewCell: UITableViewCell { gravatarImageView?.image = gridicon return } - + self.gravatarPlaceholderImage = placeholderImage ?? gridicon + self.gravatarPreferredSize = gridicon.size Task { - try await gravatarImageView?.setGravatarImage(with: email, placeholder: placeholderImage ?? gridicon, preferredSize: gridicon.size) + try await downloadAvatar() } gravatarImageViewSizeConstraints.forEach { constraint in @@ -55,6 +65,18 @@ class GravatarEmailTableViewCell: UITableViewCell { containerView.layer.borderColor = hasBorders ? UIColor.systemGray3.cgColor : UIColor.clear.cgColor } + @objc private func refreshAvatar(_ notification: Foundation.Notification) { + guard let email, notification.userInfoHasEmail(email) else { return } + Task { + try await downloadAvatar(forceRefresh: true) + } + } + + private func downloadAvatar(forceRefresh: Bool = false) async throws { + guard let email, let gravatarPlaceholderImage else { return } + try await gravatarImageView?.setGravatarImage(with: email, placeholder: gravatarPlaceholderImage, preferredSize: gravatarPreferredSize, forceRefresh: forceRefresh) + } + func updateEmailAddress(_ email: String?) { emailLabel?.text = email } From c9850601222239a28b7d37b6522013ead3358fe0 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Sun, 17 Nov 2024 14:32:17 +1300 Subject: [PATCH 15/28] Move the Application Passwords entry to the current user details (#23814) * Move users code from WordPressUI to the app target * Change package level access to internal level access * Show application passwords list for current user * Remove the 'Application Passwords' entry from the blog details --- .../ApplicationTokenListView.swift | 2 +- WordPress/Classes/Services/UserService.swift | 24 +++++++----- .../Users/Components/UserListItem.swift | 14 +++---- .../Users/Components/UserProfileImage.swift | 0 .../Classes}/Users/UserProvider.swift | 22 +++++++---- .../Users/ViewModel/DisplayUser.swift | 2 +- .../Users/ViewModel/UserDeleteViewModel.swift | 0 .../Users/ViewModel/UserDetailViewModel.swift | 11 +----- .../Users/ViewModel/UserListViewModel.swift | 0 .../Users/Views/UserDetailsView.swift | 19 ++++++++- .../Classes}/Users/Views/UserListView.swift | 13 ++++--- ...DetailsViewController+SectionHelpers.swift | 39 ++----------------- .../Blog Details/BlogDetailsViewController.m | 17 -------- 13 files changed, 64 insertions(+), 99 deletions(-) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/Components/UserListItem.swift (72%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/Components/UserProfileImage.swift (100%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/UserProvider.swift (71%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/ViewModel/DisplayUser.swift (97%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/ViewModel/UserDeleteViewModel.swift (100%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/ViewModel/UserDetailViewModel.swift (66%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/ViewModel/UserListViewModel.swift (100%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/Views/UserDetailsView.swift (94%) rename {Modules/Sources/WordPressUI/Views => WordPress/Classes}/Users/Views/UserListView.swift (79%) diff --git a/WordPress/Classes/ApplicationToken/ApplicationTokenListView.swift b/WordPress/Classes/ApplicationToken/ApplicationTokenListView.swift index 6558a8c831ba..acde770fc324 100644 --- a/WordPress/Classes/ApplicationToken/ApplicationTokenListView.swift +++ b/WordPress/Classes/ApplicationToken/ApplicationTokenListView.swift @@ -113,7 +113,7 @@ extension ApplicationTokenListView { // MARK: - SwiftUI Preview -private class StaticTokenProvider: ApplicationTokenListDataProvider { +class StaticTokenProvider: ApplicationTokenListDataProvider { private let result: Result<[ApplicationTokenItem], Error> diff --git a/WordPress/Classes/Services/UserService.swift b/WordPress/Classes/Services/UserService.swift index f684dc20b93f..066cfba3c1aa 100644 --- a/WordPress/Classes/Services/UserService.swift +++ b/WordPress/Classes/Services/UserService.swift @@ -20,7 +20,15 @@ actor UserService: UserServiceProtocol { nonisolated let usersUpdates: AsyncStream<[DisplayUser]> private nonisolated let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation - private var currentUser: UserWithEditContext? + private var _currentUser: UserWithEditContext? + private var currentUser: UserWithEditContext? { + get async { + if _currentUser == nil { + _currentUser = try? await self.client.api.users.retrieveMeWithEditContext() + } + return _currentUser + } + } init(client: WordPressClient) { self.client = client @@ -53,16 +61,12 @@ actor UserService: UserServiceProtocol { return task } - func isCurrentUserCapableOf(_ capability: String) async throws -> Bool { - let currentUser: UserWithEditContext - if let cached = self.currentUser { - currentUser = cached - } else { - currentUser = try await self.client.api.users.retrieveMeWithEditContext() - self.currentUser = currentUser - } + func isCurrentUserCapableOf(_ capability: String) async -> Bool { + await currentUser?.capabilities.keys.contains(capability) == true + } - return currentUser.capabilities.keys.contains(capability) + func isCurrentUser(_ user: DisplayUser) async -> Bool { + await currentUser?.id == user.id } func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { diff --git a/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift b/WordPress/Classes/Users/Components/UserListItem.swift similarity index 72% rename from Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift rename to WordPress/Classes/Users/Components/UserListItem.swift index 93c21253aa7a..7c8a7f735637 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift +++ b/WordPress/Classes/Users/Components/UserListItem.swift @@ -8,17 +8,13 @@ struct UserListItem: View { @Environment(\.dynamicTypeSize) var dynamicTypeSize - private let user: DisplayUser - private let userService: UserServiceProtocol - - init(user: DisplayUser, userService: UserServiceProtocol) { - self.user = user - self.userService = userService - } + let user: DisplayUser + let userService: UserServiceProtocol + let applicationTokenListDataProvider: ApplicationTokenListDataProvider var body: some View { NavigationLink { - UserDetailsView(user: user, userService: userService) + UserDetailsView(user: user, userService: userService, applicationTokenListDataProvider: applicationTokenListDataProvider) } label: { HStack(alignment: .top) { if !dynamicTypeSize.isAccessibilitySize { @@ -34,5 +30,5 @@ struct UserListItem: View { } #Preview { - UserListItem(user: DisplayUser.MockUser, userService: MockUserProvider()) + UserListItem(user: DisplayUser.MockUser, userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens))) } diff --git a/Modules/Sources/WordPressUI/Views/Users/Components/UserProfileImage.swift b/WordPress/Classes/Users/Components/UserProfileImage.swift similarity index 100% rename from Modules/Sources/WordPressUI/Views/Users/Components/UserProfileImage.swift rename to WordPress/Classes/Users/Components/UserProfileImage.swift diff --git a/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift b/WordPress/Classes/Users/UserProvider.swift similarity index 71% rename from Modules/Sources/WordPressUI/Views/Users/UserProvider.swift rename to WordPress/Classes/Users/UserProvider.swift index 59b1cda47221..386b22db53ac 100644 --- a/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift +++ b/WordPress/Classes/Users/UserProvider.swift @@ -7,14 +7,16 @@ public protocol UserServiceProtocol: Actor { func fetchUsers() async throws -> [DisplayUser] - func isCurrentUserCapableOf(_ capability: String) async throws -> Bool + func isCurrentUser(_ user: DisplayUser) async -> Bool + + func isCurrentUserCapableOf(_ capability: String) async -> Bool func setNewPassword(id: Int32, newPassword: String) async throws func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws } -package actor MockUserProvider: UserServiceProtocol { +actor MockUserProvider: UserServiceProtocol { enum Scenario { case infinitLoading @@ -24,10 +26,10 @@ package actor MockUserProvider: UserServiceProtocol { var scenario: Scenario - package nonisolated let usersUpdates: AsyncStream<[DisplayUser]> + nonisolated let usersUpdates: AsyncStream<[DisplayUser]> private let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation - package private(set) var users: [DisplayUser]? { + private(set) var users: [DisplayUser]? { didSet { if let users { usersUpdatesContinuation.yield(users) @@ -40,7 +42,7 @@ package actor MockUserProvider: UserServiceProtocol { (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream() } - package func fetchUsers() async throws -> [DisplayUser] { + func fetchUsers() async throws -> [DisplayUser] { switch scenario { case .infinitLoading: // Do nothing @@ -57,15 +59,19 @@ package actor MockUserProvider: UserServiceProtocol { } } - package func isCurrentUserCapableOf(_ capability: String) async throws -> Bool { + func isCurrentUser(_ user: DisplayUser) async -> Bool { + true + } + + func isCurrentUserCapableOf(_ capability: String) async -> Bool { true } - package func setNewPassword(id: Int32, newPassword: String) async throws { + func setNewPassword(id: Int32, newPassword: String) async throws { // Not used in Preview } - package func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { + func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { // Not used in Preview } } diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/DisplayUser.swift b/WordPress/Classes/Users/ViewModel/DisplayUser.swift similarity index 97% rename from Modules/Sources/WordPressUI/Views/Users/ViewModel/DisplayUser.swift rename to WordPress/Classes/Users/ViewModel/DisplayUser.swift index 34c892c63354..23fc80a143c5 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/DisplayUser.swift +++ b/WordPress/Classes/Users/ViewModel/DisplayUser.swift @@ -42,7 +42,7 @@ public struct DisplayUser: Identifiable, Codable, Hashable { self.biography = biography } - static package let MockUser = DisplayUser( + static let MockUser = DisplayUser( id: 16, handle: "@person", username: "example", diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift b/WordPress/Classes/Users/ViewModel/UserDeleteViewModel.swift similarity index 100% rename from Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift rename to WordPress/Classes/Users/ViewModel/UserDeleteViewModel.swift diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift b/WordPress/Classes/Users/ViewModel/UserDetailViewModel.swift similarity index 66% rename from Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift rename to WordPress/Classes/Users/ViewModel/UserDetailViewModel.swift index 87179642d603..064404230afb 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDetailViewModel.swift +++ b/WordPress/Classes/Users/ViewModel/UserDetailViewModel.swift @@ -10,23 +10,14 @@ class UserDetailViewModel: ObservableObject { @Published private(set) var isLoadingCurrentUser: Bool = false - @Published - private(set) var error: Error? = nil - init(userService: UserServiceProtocol) { self.userService = userService } func loadCurrentUserRole() async { - error = nil - isLoadingCurrentUser = true defer { isLoadingCurrentUser = false} - do { - currentUserCanModifyUsers = try await userService.isCurrentUserCapableOf("edit_users") - } catch { - self.error = error - } + currentUserCanModifyUsers = await userService.isCurrentUserCapableOf("edit_users") } } diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift b/WordPress/Classes/Users/ViewModel/UserListViewModel.swift similarity index 100% rename from Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift rename to WordPress/Classes/Users/ViewModel/UserListViewModel.swift diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift b/WordPress/Classes/Users/Views/UserDetailsView.swift similarity index 94% rename from Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift rename to WordPress/Classes/Users/Views/UserDetailsView.swift index 39d8e0457f47..6c0e7bf92314 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift +++ b/WordPress/Classes/Users/Views/UserDetailsView.swift @@ -21,15 +21,18 @@ struct UserDetailsView: View { @StateObject fileprivate var viewModel: UserDetailViewModel @StateObject + fileprivate var applicationTokenListViewModel: ApplicationTokenListViewModel + @StateObject fileprivate var deleteUserViewModel: UserDeleteViewModel @Environment(\.dismiss) var dismissAction: DismissAction - init(user: DisplayUser, userService: UserServiceProtocol) { + init(user: DisplayUser, userService: UserServiceProtocol, applicationTokenListDataProvider: ApplicationTokenListDataProvider) { self.user = user self.userService = userService _viewModel = StateObject(wrappedValue: UserDetailViewModel(userService: userService)) + _applicationTokenListViewModel = StateObject(wrappedValue: ApplicationTokenListViewModel(dataProvider: applicationTokenListDataProvider)) _deleteUserViewModel = StateObject(wrappedValue: UserDeleteViewModel(user: user, userService: userService)) } @@ -58,6 +61,14 @@ struct UserDetailsView: View { } } + if !applicationTokenListViewModel.applicationTokens.isEmpty { + Section(ApplicationTokenListView.title) { + ForEach(applicationTokenListViewModel.applicationTokens) { token in + ApplicationTokenListItemView(item: token) + } + } + } + if viewModel.currentUserCanModifyUsers { Section(Strings.accountManagementSectionTitle) { Button(Strings.setNewPasswordActionTitle) { @@ -104,6 +115,10 @@ struct UserDetailsView: View { Task { await viewModel.loadCurrentUserRole() await deleteUserViewModel.fetchOtherUsers() + + if await userService.isCurrentUser(user) { + await applicationTokenListViewModel.fetchTokens() + } } } } @@ -382,6 +397,6 @@ private extension String { #Preview { NavigationStack { - UserDetailsView(user: DisplayUser.MockUser, userService: MockUserProvider()) + UserDetailsView(user: DisplayUser.MockUser, userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens))) } } diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift b/WordPress/Classes/Users/Views/UserListView.swift similarity index 79% rename from Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift rename to WordPress/Classes/Users/Views/UserListView.swift index 31cb09e7775a..bd9b43585224 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift +++ b/WordPress/Classes/Users/Views/UserListView.swift @@ -1,13 +1,16 @@ import SwiftUI +import WordPressUI public struct UserListView: View { @StateObject private var viewModel: UserListViewModel private let userService: UserServiceProtocol + private let applicationTokenListDataProvider: ApplicationTokenListDataProvider - public init(userService: UserServiceProtocol) { + public init(userService: UserServiceProtocol, applicationTokenListDataProvider: ApplicationTokenListDataProvider) { self.userService = userService + self.applicationTokenListDataProvider = applicationTokenListDataProvider _viewModel = StateObject(wrappedValue: UserListViewModel(userService: userService)) } @@ -31,7 +34,7 @@ public struct UserListView: View { .listRowBackground(Color.clear) } else { ForEach(section.users) { user in - UserListItem(user: user, userService: userService) + UserListItem(user: user, userService: userService, applicationTokenListDataProvider: applicationTokenListDataProvider) } } } @@ -70,18 +73,18 @@ public struct UserListView: View { #Preview("Loading") { NavigationView { - UserListView(userService: MockUserProvider()) + UserListView(userService: MockUserProvider(), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens))) } } #Preview("Error") { NavigationView { - UserListView(userService: MockUserProvider(scenario: .error)) + UserListView(userService: MockUserProvider(scenario: .error), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens))) } } #Preview("List") { NavigationView { - UserListView(userService: MockUserProvider(scenario: .dummyData)) + UserListView(userService: MockUserProvider(scenario: .dummyData), applicationTokenListDataProvider: StaticTokenProvider(tokens: .success(.testTokens))) } } diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift index 65fe07c64dd6..cf820db1427c 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift +++ b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift @@ -124,49 +124,16 @@ extension BlogDetailsViewController { return AppConfiguration.allowsDomainRegistration && blog.supports(.domains) } - @objc func shouldShowApplicationPasswordRow() -> Bool { - // Only available for application-password authenticated self-hosted sites. - return self.blog.account == nil && self.blog.userID != nil - } - - private func createApplicationPasswordService() -> ApplicationPasswordService? { - guard let userId = self.blog.userID?.intValue else { - return nil - } - - do { - let site = try WordPressSite(blog: self.blog) - let client = WordPressClient(site: site) - return ApplicationPasswordService(api: client, currentUserId: userId) - } catch { - DDLogError("Failed to create WordPressClient: \(error)") - return nil - } - } - - @objc func showApplicationPasswordManagement() { - guard let presentationDelegate, let userId = self.blog.userID?.intValue else { - return - } - - let feature = NSLocalizedString("applicationPasswordRequired.feature.plugins", value: "Application Passwords Management", comment: "Feature name for managing Application Passwords in the app") - let rootView = ApplicationPasswordRequiredView(blog: self.blog, localizedFeatureName: feature) { client in - let service = ApplicationPasswordService(api: client, currentUserId: userId) - let viewModel = ApplicationTokenListViewModel(dataProvider: service) - return ApplicationTokenListView(viewModel: viewModel) - } - presentationDelegate.presentBlogDetailsViewController(UIHostingController(rootView: rootView)) - } - @objc func showUsers() { - guard let presentationDelegate else { + guard let presentationDelegate, let userId = self.blog.userID?.intValue else { return } let feature = NSLocalizedString("applicationPasswordRequired.feature.users", value: "User Management", comment: "Feature name for managing users in the app") let rootView = ApplicationPasswordRequiredView(blog: self.blog, localizedFeatureName: feature) { client in let service = UserService(client: client) - return UserListView(userService: service) + let applicationPasswordService = ApplicationPasswordService(api: client, currentUserId: userId) + return UserListView(userService: service, applicationTokenListDataProvider: applicationPasswordService) } presentationDelegate.presentBlogDetailsViewController(UIHostingController(rootView: rootView)) } diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m index ae5084002b65..ab41fbca01b8 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m +++ b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController.m @@ -927,16 +927,6 @@ - (BlogDetailsRow *)adminRow return row; } -- (BlogDetailsRow *)applicationPasswordManagementRow -{ - __weak __typeof(self) weakSelf = self; - NSString *title = NSLocalizedString(@"Application Passwords", @"Noun. Title. Links to the application password management feature."); - BlogDetailsRow *row = [[BlogDetailsRow alloc] initWithTitle:title - image:[UIImage systemImageNamed:@"lock"] - callback:^{ [weakSelf showApplicationPasswordManagement]; }]; - return row; -} - #pragma mark - Data Model setup - (void)reloadTableViewPreservingSelection @@ -1139,9 +1129,6 @@ - (BlogDetailsSection *)trafficSectionViewModel if ([self shouldAddDomainRegistrationRow]) { [secondSectionRows addObject:[self domainsRow]]; } - if ([self shouldShowApplicationPasswordRow]) { - [secondSectionRows addObject:[self applicationPasswordManagementRow]]; - } [secondSectionRows addObject:[self siteSettingsRow]]; // Third section @@ -1385,10 +1372,6 @@ - (BlogDetailsSection *)configurationSectionViewModel }]]; } - if ([self shouldShowApplicationPasswordRow]) { - [rows addObject:[self applicationPasswordManagementRow]]; - } - BlogDetailsRow *row = [[BlogDetailsRow alloc] initWithTitle:NSLocalizedString(@"Site Settings", @"Noun. Title. Links to the blog's Settings screen.") identifier:BlogDetailsSettingsCellIdentifier accessibilityIdentifier:@"Settings Row" From eafc917ec77e34a8c5524690c8cd3f59b47e9a27 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 19 Nov 2024 09:29:58 +1300 Subject: [PATCH 16/28] Enable .com web based login on test builds (#23801) --- WordPress/Classes/System/WordPressAppDelegate.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/WordPress/Classes/System/WordPressAppDelegate.swift b/WordPress/Classes/System/WordPressAppDelegate.swift index f102c8728a65..d786c1304d69 100644 --- a/WordPress/Classes/System/WordPressAppDelegate.swift +++ b/WordPress/Classes/System/WordPressAppDelegate.swift @@ -540,6 +540,11 @@ extension WordPressAppDelegate { /// Updates the remote feature flags using an authenticated remote if a token is provided or an account exists /// Otherwise an anonymous remote will be used func updateFeatureFlags(authToken: String? = nil, completion: (() -> Void)? = nil) { + // Enable certain feature flags on test builds. + if BuildConfiguration.current ~= [.a8cPrereleaseTesting, .a8cBranchTest, .localDeveloper] { + FeatureFlagOverrideStore().override(RemoteFeatureFlag.dotComWebLogin, withValue: true) + } + var api: WordPressComRestApi if let authToken { api = WordPressComRestApi.defaultV2Api(authToken: authToken) From 7b5e060b2f3b9fe51f7d4325f9e8e4990da32acb Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Mon, 18 Nov 2024 17:22:29 -0500 Subject: [PATCH 17/28] Fix issue with nav items jumping (#23816) --- .../Reader/Detail/ReaderDetailViewController.swift | 7 ------- 1 file changed, 7 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift index 5e4564e6fc04..7b2da6cce258 100644 --- a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift @@ -226,13 +226,6 @@ class ReaderDetailViewController: UIViewController, ReaderDetailView { }) } - override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { - super.traitCollectionDidChange(previousTraitCollection) - - // Bar items may change if we're moving single pane to split view - self.configureNavigationBar() - } - override func accessibilityPerformEscape() -> Bool { navigationController?.popViewController(animated: true) return true From 703d15a32a3aefe80f686a7e18be45777eaf2bbf Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Nov 2024 18:02:10 -0800 Subject: [PATCH 18/28] Bump fastlane-plugin-wpmreleasetoolkit from 12.3.0 to 12.3.2 (#23820) Bumps [fastlane-plugin-wpmreleasetoolkit](https://github.com/wordpress-mobile/release-toolkit) from 12.3.0 to 12.3.2. - [Release notes](https://github.com/wordpress-mobile/release-toolkit/releases) - [Changelog](https://github.com/wordpress-mobile/release-toolkit/blob/trunk/CHANGELOG.md) - [Commits](https://github.com/wordpress-mobile/release-toolkit/compare/12.3.0...12.3.2) --- updated-dependencies: - dependency-name: fastlane-plugin-wpmreleasetoolkit dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 12259bbf6133..496d6e7090cf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,16 +26,16 @@ GEM ast (2.4.2) atomos (0.1.3) aws-eventstream (1.3.0) - aws-partitions (1.1001.0) - aws-sdk-core (3.212.0) + aws-partitions (1.1009.0) + aws-sdk-core (3.213.0) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.992.0) aws-sigv4 (~> 1.9) jmespath (~> 1, >= 1.6.1) - aws-sdk-kms (1.95.0) + aws-sdk-kms (1.96.0) aws-sdk-core (~> 3, >= 3.210.0) aws-sigv4 (~> 1.5) - aws-sdk-s3 (1.170.0) + aws-sdk-s3 (1.172.0) aws-sdk-core (~> 3, >= 3.210.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.5) @@ -43,7 +43,7 @@ GEM aws-eventstream (~> 1, >= 1.0.2) babosa (1.0.4) base64 (0.2.0) - benchmark (0.3.0) + benchmark (0.4.0) bigdecimal (3.1.8) buildkit (1.6.1) sawyer (>= 0.6) @@ -210,7 +210,7 @@ GEM fastlane-plugin-appcenter (2.1.2) fastlane-plugin-sentry (1.25.1) os (~> 1.1, >= 1.1.4) - fastlane-plugin-wpmreleasetoolkit (12.3.0) + fastlane-plugin-wpmreleasetoolkit (12.3.2) activesupport (>= 6.1.7.1) buildkit (~> 1.5) chroma (= 0.2.0) @@ -280,7 +280,7 @@ GEM concurrent-ruby (~> 1.0) java-properties (0.3.0) jmespath (1.6.2) - json (2.8.1) + json (2.8.2) jwt (2.9.3) base64 kramdown (2.4.0) @@ -291,7 +291,7 @@ GEM logger (1.6.1) mini_magick (4.13.2) mini_mime (1.1.5) - mini_portile2 (2.8.7) + mini_portile2 (2.8.8) minitest (5.25.1) molinillo (0.8.0) multi_json (1.15.0) @@ -309,7 +309,7 @@ GEM sawyer (~> 0.9) open4 (1.3.4) options (2.3.2) - optparse (0.5.0) + optparse (0.6.0) os (1.1.4) parallel (1.26.3) parser (3.3.6.0) @@ -359,7 +359,7 @@ GEM sawyer (0.9.2) addressable (>= 2.3.5) faraday (>= 0.17.3, < 3) - securerandom (0.3.1) + securerandom (0.3.2) security (0.1.5) signet (0.19.0) addressable (~> 2.8) From 7ab94fdc44950962fbbcf2c9fbeef559f46aa229 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 07:31:48 -0500 Subject: [PATCH 19/28] Reader: Fix RTL and more (#23821) * Fix RTL support in ReaderSidebarSection * Fix RTL support in ReaderSidebarViewController * Fix accessibility issue in the Reader menu * Update like and reply button to use new UIButton.Configuration * Reimplement animation * Throw error UINotificationFeedbackTypeError --- .../Style/WPStyleGuide+CommentDetail.swift | 19 --- .../Detail/CommentContentTableViewCell.swift | 137 +++++---------- .../Detail/CommentContentTableViewCell.xib | 37 +--- .../Comments/ReaderCommentsViewController.m | 1 + .../Sidebar/ReaderSidebarViewController.swift | 9 +- .../Views/WPRichText/WPRichContentView.swift | 14 +- .../Contents.json | 12 -- .../arrowshape.turn.up.backward.svg | 161 ------------------ 8 files changed, 66 insertions(+), 324 deletions(-) delete mode 100644 WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/Contents.json delete mode 100644 WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/arrowshape.turn.up.backward.svg diff --git a/WordPress/Classes/ViewRelated/Comments/Style/WPStyleGuide+CommentDetail.swift b/WordPress/Classes/ViewRelated/Comments/Style/WPStyleGuide+CommentDetail.swift index 32bf759a0832..6f83a097e08e 100644 --- a/WordPress/Classes/ViewRelated/Comments/Style/WPStyleGuide+CommentDetail.swift +++ b/WordPress/Classes/ViewRelated/Comments/Style/WPStyleGuide+CommentDetail.swift @@ -36,38 +36,19 @@ extension WPStyleGuide { static let dateFont = CommentDetail.tertiaryTextFont static let dateTextColor = CommentDetail.secondaryTextColor - static let reactionButtonFont = WPStyleGuide.fontForTextStyle(.caption1) - static let reactionButtonTextColor = UIColor.label - // highlighted state static let highlightedBackgroundColor = UIColor( light: UIAppColor.blue(.shade0), dark: UIAppColor.blue(.shade100) ).withAlphaComponent(0.5) static let highlightedBarBackgroundColor = UIAppColor.blue(.shade40) - static let highlightedReplyButtonTintColor = UIAppColor.primary static let placeholderImage = UIImage.gravatarPlaceholderImage - private static let reactionIconConfiguration = UIImage.SymbolConfiguration(font: reactionButtonFont, scale: .large) - static let unlikedIconImage = UIImage(systemName: "star", withConfiguration: reactionIconConfiguration) - static let likedIconImage = UIImage(systemName: "star.fill", withConfiguration: reactionIconConfiguration) - static let accessoryIconConfiguration = UIImage.SymbolConfiguration(font: CommentDetail.tertiaryTextFont, scale: .medium) static let shareIconImageName = "square.and.arrow.up" static let ellipsisIconImageName = "ellipsis.circle" static let infoIconImageName = "info.circle" - - static var replyIconImage: UIImage? { - // this symbol is only available in iOS 14 and above. For iOS 13, we need to use the backported image in our assets. - let name = "arrowshape.turn.up.backward" - let image = UIImage(systemName: name) ?? UIImage(named: name) - return image?.withConfiguration(reactionIconConfiguration).imageFlippedForRightToLeftLayoutDirection() - } - - static let highlightedReplyIconImage = UIImage(systemName: "arrowshape.turn.up.left.fill", withConfiguration: reactionIconConfiguration)? - .withTintColor(highlightedReplyButtonTintColor, renderingMode: .alwaysTemplate) - .imageFlippedForRightToLeftLayoutDirection() } public struct ReplyIndicator { diff --git a/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift b/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift index 7d6730d9f6d0..511873b3ed03 100644 --- a/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift +++ b/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.swift @@ -81,9 +81,8 @@ class CommentContentTableViewCell: UITableViewCell, NibReusable { @objc var isReplyHighlighted: Bool = false { didSet { - replyButton?.tintColor = isReplyHighlighted ? Style.highlightedReplyButtonTintColor : Style.reactionButtonTextColor - replyButton?.setTitleColor(isReplyHighlighted ? Style.highlightedReplyButtonTintColor : Style.reactionButtonTextColor, for: .normal) - replyButton?.setImage(isReplyHighlighted ? Style.highlightedReplyIconImage : Style.replyIconImage, for: .normal) + replyButton.tintColor = isReplyHighlighted ? UIAppColor.brand : .label + replyButton.configuration?.image = UIImage(systemName: isReplyHighlighted ? "arrowshape.turn.up.left.fill" : "arrowshape.turn.up.left") } } @@ -130,8 +129,6 @@ class CommentContentTableViewCell: UITableViewCell, NibReusable { private var likeCount: Int = 0 - private var isLikeButtonAnimating: Bool = false - /// Styling configuration based on `ReaderDisplaySetting`. The parameter is optional so that the styling approach /// can be scoped by using the "legacy" style when the passed parameter is nil. private var style: CellStyle = .init(displaySetting: nil) @@ -377,36 +374,40 @@ private extension CommentContentTableViewCell { accessoryButton?.setImage(accessoryButtonImage, for: .normal) accessoryButton?.addTarget(self, action: #selector(accessoryButtonTapped), for: .touchUpInside) - replyButton?.tintColor = Style.reactionButtonTextColor - replyButton?.titleLabel?.font = Style.reactionButtonFont - replyButton?.titleLabel?.adjustsFontSizeToFitWidth = true - replyButton?.titleLabel?.adjustsFontForContentSizeCategory = true - replyButton?.setTitle(.reply, for: .normal) - replyButton?.setTitleColor(Style.reactionButtonTextColor, for: .normal) - replyButton?.setImage(Style.replyIconImage, for: .normal) - replyButton?.addTarget(self, action: #selector(replyButtonTapped), for: .touchUpInside) - replyButton?.adjustsImageSizeForAccessibilityContentSizeCategory = true - adjustImageAndTitleEdgeInsets(for: replyButton) - replyButton?.sizeToFit() - replyButton?.accessibilityIdentifier = .replyButtonAccessibilityId - - likeButton?.tintColor = Style.reactionButtonTextColor - likeButton?.titleLabel?.font = Style.reactionButtonFont - likeButton?.titleLabel?.adjustsFontSizeToFitWidth = true - likeButton?.titleLabel?.adjustsFontForContentSizeCategory = true - likeButton?.setTitleColor(Style.reactionButtonTextColor, for: .normal) - likeButton?.addTarget(self, action: #selector(likeButtonTapped), for: .touchUpInside) - likeButton?.adjustsImageSizeForAccessibilityContentSizeCategory = true - adjustImageAndTitleEdgeInsets(for: likeButton) + replyButton.configuration = makeReactionButtonConfiguration(systemImage: "arrowshape.turn.up.left") + replyButton.tintColor = .label + replyButton.setTitle(.reply, for: .normal) + replyButton.addTarget(self, action: #selector(replyButtonTapped), for: .touchUpInside) + replyButton.maximumContentSizeCategory = .accessibilityMedium + replyButton.accessibilityIdentifier = .replyButtonAccessibilityId + + likeButton.configuration = makeReactionButtonConfiguration(systemImage: "star") + likeButton.tintColor = .label + + likeButton.addTarget(self, action: #selector(likeButtonTapped), for: .touchUpInside) + likeButton.maximumContentSizeCategory = .accessibilityMedium updateLikeButton(liked: false, numberOfLikes: 0) - likeButton?.sizeToFit() - likeButton?.accessibilityIdentifier = .likeButtonAccessibilityId + likeButton.accessibilityIdentifier = .likeButtonAccessibilityId separatorView.layoutMargins = .init(top: 0, left: 20, bottom: 0, right: 0).flippedForRightToLeft applyStyles() } + private func makeReactionButtonConfiguration(systemImage: String) -> UIButton.Configuration { + var configuration = UIButton.Configuration.plain() + configuration.image = UIImage(systemName: systemImage) + configuration.imagePlacement = .top + configuration.imagePadding = 5 + configuration.titleTextAttributesTransformer = UIConfigurationTextAttributesTransformer { + var attributes = $0 + attributes.font = UIFont.preferredFont(forTextStyle: .footnote) + return attributes + } + configuration.preferredSymbolConfigurationForImage = UIImage.SymbolConfiguration(font: UIFont.preferredFont(forTextStyle: .caption1)) + return configuration + } + /// Applies the `ReaderDisplaySetting` styles private func applyStyles() { nameLabel?.font = style.nameFont @@ -416,16 +417,6 @@ private extension CommentContentTableViewCell { dateLabel?.textColor = style.dateTextColor } - private func adjustImageAndTitleEdgeInsets(for button: UIButton) { - guard let imageSize = button.imageView?.frame.size, let titleSize = button.titleLabel?.frame.size else { - return - } - - let spacing: CGFloat = 3 - button.titleEdgeInsets = .init(top: 0, left: -titleSize.width, bottom: -(imageSize.height + spacing), right: 0) - button.imageEdgeInsets = .init(top: -(titleSize.height + spacing), left: imageSize.width/2, bottom: 0, right: 0) - } - /// Configures the avatar image view with the provided URL. /// If the URL does not contain any image, the default placeholder image will be displayed. /// - Parameter url: The URL containing the image. @@ -460,61 +451,20 @@ private extension CommentContentTableViewCell { /// - numberOfLikes: The number of likes to be displayed. /// - animated: Whether the Like button state change should be animated or not. Defaults to false. /// - completion: Completion block called once the animation is completed. Defaults to nil. - func updateLikeButton(liked: Bool, numberOfLikes: Int, animated: Bool = false, completion: (() -> Void)? = nil) { - guard !isLikeButtonAnimating else { - return - } - + func updateLikeButton(liked: Bool, numberOfLikes: Int, animated: Bool = false) { isLiked = liked likeCount = numberOfLikes - - let onAnimationComplete = { [weak self] in - guard let self = self else { - return - } - - self.likeButton.tintColor = liked ? Style.likedTintColor : Style.reactionButtonTextColor - self.likeButton.setImage(liked ? Style.likedIconImage : Style.unlikedIconImage, for: .normal) - self.likeButton.setTitle(self.likeButtonTitle, for: .normal) - self.adjustImageAndTitleEdgeInsets(for: self.likeButton) - self.likeButton.setTitleColor(liked ? Style.likedTintColor : Style.reactionButtonTextColor, for: .normal) - self.likeButton.accessibilityLabel = liked ? String(numberOfLikes) + .commentIsLiked : String(numberOfLikes) + .commentIsNotLiked - completion?() - } - - guard animated else { - onAnimationComplete() - return - } - - isLikeButtonAnimating = true - - if isLiked { - UINotificationFeedbackGenerator().notificationOccurred(.success) - } - - animateLikeButton { - onAnimationComplete() - self.isLikeButtonAnimating = false + likeButton.tintColor = liked ? Style.likedTintColor : .label + if var configuration = likeButton.configuration { + configuration.image = UIImage(systemName: liked ? "star.fill" : "star") + configuration.title = likeButtonTitle + likeButton.configuration = configuration + } else { + wpAssertionFailure("missing configuration") } - } - - /// Animates the Like button state change. - func animateLikeButton(completion: @escaping () -> Void) { - guard let buttonImageView = likeButton.imageView, - let overlayImage = Style.likedIconImage?.withTintColor(Style.likedTintColor) else { - completion() - return - } - - let overlayImageView = UIImageView(image: overlayImage) - overlayImageView.frame = likeButton.convert(buttonImageView.bounds, from: buttonImageView) - likeButton.addSubview(overlayImageView) - - let animation = isLiked ? overlayImageView.fadeInWithRotationAnimation : overlayImageView.fadeOutWithRotationAnimation - animation { _ in - overlayImageView.removeFromSuperview() - completion() + likeButton.accessibilityLabel = liked ? String(numberOfLikes) + .commentIsLiked : String(numberOfLikes) + .commentIsNotLiked + if liked && animated { + likeButton.imageView?.fadeInWithRotationAnimation() } } @@ -579,11 +529,8 @@ private extension CommentContentTableViewCell { } @objc func likeButtonTapped() { - ReachabilityUtils.onAvailableInternetConnectionDo { - updateLikeButton(liked: !isLiked, numberOfLikes: isLiked ? likeCount - 1 : likeCount + 1, animated: true) { - self.likeButtonAction?() - } - } + updateLikeButton(liked: !isLiked, numberOfLikes: isLiked ? likeCount - 1 : likeCount + 1, animated: true) + likeButtonAction?() } } diff --git a/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.xib b/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.xib index 6a028d4df9ff..82a7af5568fe 100644 --- a/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.xib +++ b/WordPress/Classes/ViewRelated/Comments/Views/Detail/CommentContentTableViewCell.xib @@ -1,9 +1,9 @@ - + - + @@ -130,32 +130,15 @@ @@ -207,10 +190,8 @@ - - - - + + @@ -218,13 +199,13 @@ - + - + diff --git a/WordPress/Classes/ViewRelated/Reader/Comments/ReaderCommentsViewController.m b/WordPress/Classes/ViewRelated/Reader/Comments/ReaderCommentsViewController.m index f20c62a244dc..6535c5065d41 100644 --- a/WordPress/Classes/ViewRelated/Reader/Comments/ReaderCommentsViewController.m +++ b/WordPress/Classes/ViewRelated/Reader/Comments/ReaderCommentsViewController.m @@ -984,6 +984,7 @@ - (void)didTapLikeForComment:(Comment *)comment atIndexPath:(NSIndexPath *)index } failure:^(NSError * __unused error) { // in case of failure, revert the cell's like state. [weakSelf.tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationAutomatic]; + [[UINotificationFeedbackGenerator new] notificationOccurred:UINotificationFeedbackTypeError]; }]; } diff --git a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift index c24a611e661c..0b729928f70a 100644 --- a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift @@ -57,6 +57,7 @@ private struct ReaderSidebarView: View { @FetchRequest(sortDescriptors: [SortDescriptor(\.title, order: .forward)]) private var teams: FetchedResults + @Environment(\.layoutDirection) var layoutDirection @Environment(\.editMode) var editMode var isEditing: Bool { editMode?.wrappedValue.isEditing == true } @@ -129,11 +130,13 @@ private struct ReaderSidebarView: View { .lineLimit(1) if viewModel.isCompact { Spacer() - Image(systemName: "chevron.right") + Image(systemName: layoutDirection == .rightToLeft ? "chevron.left" : "chevron.right") .font(.system(size: 14).weight(.medium)) .foregroundStyle(.secondary.opacity(0.8)) } } + .accessibilityElement() + .accessibilityLabel(title) } private var preferredTintColor: Color { @@ -159,6 +162,8 @@ private struct ReaderSidebarSection: View { var isCompact: Bool @ViewBuilder var content: () -> Content + @Environment(\.layoutDirection) var layoutDirection + var body: some View { if isCompact { Button { @@ -169,7 +174,7 @@ private struct ReaderSidebarSection: View { .font(.subheadline.weight(.semibold)) .foregroundStyle(.secondary) Spacer() - Image(systemName: isExpanded ? "chevron.down" : "chevron.right") + Image(systemName: isExpanded ? "chevron.down" : (layoutDirection == .rightToLeft ? "chevron.left" : "chevron.right")) .font(.system(size: 14).weight(.semibold)) .foregroundStyle(AppColor.brand) .frame(width: 14) diff --git a/WordPress/Classes/ViewRelated/Views/WPRichText/WPRichContentView.swift b/WordPress/Classes/ViewRelated/Views/WPRichText/WPRichContentView.swift index 80d4faa7b422..467ed7df025c 100644 --- a/WordPress/Classes/ViewRelated/Views/WPRichText/WPRichContentView.swift +++ b/WordPress/Classes/ViewRelated/Views/WPRichText/WPRichContentView.swift @@ -33,13 +33,13 @@ class WPRichContentView: UITextView { } @objc lazy var linkTapGestureRecognizer: UITapGestureRecognizer = { [unowned self] in - let gestureRecognizer = UITapGestureRecognizer(target: self, action: #selector(tapRecognized)) - gestureRecognizer.cancelsTouchesInView = true - gestureRecognizer.delaysTouchesBegan = true - gestureRecognizer.delaysTouchesEnded = true - gestureRecognizer.delegate = self - return gestureRecognizer - }() + let gestureRecognizer = UITapGestureRecognizer(target: self, action: #selector(tapRecognized)) + gestureRecognizer.cancelsTouchesInView = true + gestureRecognizer.delaysTouchesBegan = true + gestureRecognizer.delaysTouchesEnded = true + gestureRecognizer.delegate = self + return gestureRecognizer + }() override var textContainerInset: UIEdgeInsets { didSet { diff --git a/WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/Contents.json b/WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/Contents.json deleted file mode 100644 index 3f189dca8974..000000000000 --- a/WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/Contents.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "info" : { - "author" : "xcode", - "version" : 1 - }, - "symbols" : [ - { - "filename" : "arrowshape.turn.up.backward.svg", - "idiom" : "universal" - } - ] -} diff --git a/WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/arrowshape.turn.up.backward.svg b/WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/arrowshape.turn.up.backward.svg deleted file mode 100644 index 9354f96b00d5..000000000000 --- a/WordPress/Resources/AppImages.xcassets/arrowshape.turn.up.backward.symbolset/arrowshape.turn.up.backward.svg +++ /dev/null @@ -1,161 +0,0 @@ - - - - - - - - - Weight/Scale Variations - Ultralight - Thin - Light - Regular - Medium - Semibold - Bold - Heavy - Black - - - - - - - - - - - Design Variations - Symbols are supported in up to nine weights and three scales. - For optimal layout with text and other symbols, vertically align - symbols with the adjacent text. - - - - - - Margins - Leading and trailing margins on the left and right side of each symbol - can be adjusted by modifying the x-location of the margin guidelines. - Modifications are automatically applied proportionally to all - scales and weights. - - - - Exporting - Symbols should be outlined when exporting to ensure the - design is preserved when submitting to Xcode. - Template v.3.0 - Requires Xcode 13 or greater - Generated from arrowshape.turn.up.backward - Typeset at 100 points - Small - Medium - Large - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From d61e178dbd5a6b17f5145365cf8ea2eaa9e1d73e Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 09:16:33 -0500 Subject: [PATCH 20/28] Reader: Add Favorites (#23815) * Add support for adding favorites * Add analytics * Save in background * Fix --- .../Classes/Models/ReaderAbstractTopic.swift | 1 + .../Classes/Services/ReaderTopicService.m | 1 - .../Utility/Analytics/WPAnalyticsEvent.swift | 3 ++ .../ReaderSidebarSubscriptionsSection.swift | 54 ++++++++++++++----- .../Sidebar/ReaderSidebarViewController.swift | 16 +++++- 5 files changed, 60 insertions(+), 15 deletions(-) diff --git a/WordPress/Classes/Models/ReaderAbstractTopic.swift b/WordPress/Classes/Models/ReaderAbstractTopic.swift index e4d3fb896d33..3ef6cace96f9 100644 --- a/WordPress/Classes/Models/ReaderAbstractTopic.swift +++ b/WordPress/Classes/Models/ReaderAbstractTopic.swift @@ -11,6 +11,7 @@ import CoreData @NSManaged open var following: Bool @NSManaged open var lastSynced: Date? @NSManaged open var path: String + /// Repurposed for "isFavorite". @NSManaged open var showInMenu: Bool @NSManaged open var title: String @NSManaged open var type: String diff --git a/WordPress/Classes/Services/ReaderTopicService.m b/WordPress/Classes/Services/ReaderTopicService.m index b69d1f04b059..52e188cf58d9 100644 --- a/WordPress/Classes/Services/ReaderTopicService.m +++ b/WordPress/Classes/Services/ReaderTopicService.m @@ -735,7 +735,6 @@ - (ReaderSiteTopic *)siteTopicForRemoteSiteInfo:(RemoteReaderSiteInfo *)siteInfo topic.organizationID = [siteInfo.organizationID integerValue]; topic.path = siteInfo.postsEndpoint; topic.postCount = siteInfo.postCount; - topic.showInMenu = NO; topic.siteBlavatar = siteInfo.siteBlavatar; topic.siteDescription = siteInfo.siteDescription; topic.siteID = siteInfo.siteID; diff --git a/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift b/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift index e7208515fa58..0a16a27d96c0 100644 --- a/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift +++ b/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift @@ -116,6 +116,7 @@ import Foundation case readerCommentTextHighlighted case readerCommentTextCopied case readerPostContextMenuButtonTapped + case readerAddSiteToFavoritesTapped // Stats - Empty Stats nudges case statsPublicizeNudgeShown @@ -816,6 +817,8 @@ import Foundation return "reader_comment_text_copied" case .readerPostContextMenuButtonTapped: return "reader_post_context_menu_button_tapped" + case .readerAddSiteToFavoritesTapped: + return "reader_add_site_to_favorites_tapped" // Stats - Empty Stats nudges case .statsPublicizeNudgeShown: diff --git a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarSubscriptionsSection.swift b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarSubscriptionsSection.swift index a1d3160f6587..40a6100cd2ac 100644 --- a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarSubscriptionsSection.swift +++ b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarSubscriptionsSection.swift @@ -13,19 +13,8 @@ struct ReaderSidebarSubscriptionsSection: View { private var subscriptions: FetchedResults var body: some View { - ForEach(subscriptions, id: \.self) { site in - Label { - Text(site.title) - } icon: { - ReaderSiteIconView(site: site, size: .small) - } - .lineLimit(1) - .tag(ReaderSidebarItem.subscription(TaggedManagedObjectID(site))) - .swipeActions(edge: .trailing) { - Button(SharedStrings.Reader.unfollow, role: .destructive) { - ReaderSubscriptionHelper().unfollow(site) - }.tint(.red) - } + ForEach(subscriptions, id: \.self) { + ReaderSidebarSubscriptionCell(site: $0) } .onDelete(perform: delete) } @@ -37,3 +26,42 @@ struct ReaderSidebarSubscriptionsSection: View { } } } + +struct ReaderSidebarSubscriptionCell: View { + @ObservedObject var site: ReaderSiteTopic + @Environment(\.editMode) var editMode + + var body: some View { + HStack { + Label { + Text(site.title) + } icon: { + ReaderSiteIconView(site: site, size: .small) + } + if editMode?.wrappedValue.isEditing == true { + Spacer() + Button { + if !site.showInMenu { + WPAnalytics.track(.readerAddSiteToFavoritesTapped) + } + + let siteObjectID = TaggedManagedObjectID(site) + ContextManager.shared.performAndSave({ managedObjectContext in + let site = try managedObjectContext.existingObject(with: siteObjectID) + site.showInMenu.toggle() + }, completion: nil, on: DispatchQueue.main) + } label: { + Image(systemName: site.showInMenu ? "star.fill" : "star") + .foregroundStyle(site.showInMenu ? .pink : .secondary) + }.buttonStyle(.plain) + } + } + .lineLimit(1) + .tag(ReaderSidebarItem.subscription(TaggedManagedObjectID(site))) + .swipeActions(edge: .trailing) { + Button(SharedStrings.Reader.unfollow, role: .destructive) { + ReaderSubscriptionHelper().unfollow(site) + }.tint(.red) + } + } +} diff --git a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift index 0b729928f70a..2bb662fdd1b4 100644 --- a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift @@ -50,6 +50,7 @@ private struct ReaderSidebarView: View { @ObservedObject var viewModel: ReaderSidebarViewModel @AppStorage("reader_sidebar_organization_expanded") var isSectionOrganizationExpanded = true + @AppStorage("reader_sidebar_favorites_expanded") var isSectionFavoritesExpanded = true @AppStorage("reader_sidebar_subscriptions_expanded") var isSectionSubscriptionsExpanded = true @AppStorage("reader_sidebar_lists_expanded") var isSectionListsExpanded = true @AppStorage("reader_sidebar_tags_expanded") var isSectionTagsExpanded = true @@ -57,6 +58,12 @@ private struct ReaderSidebarView: View { @FetchRequest(sortDescriptors: [SortDescriptor(\.title, order: .forward)]) private var teams: FetchedResults + @FetchRequest( + sortDescriptors: [SortDescriptor(\.title, order: .forward)], + predicate: NSPredicate(format: "following = YES AND showInMenu = YES") + ) + private var favorites: FetchedResults + @Environment(\.layoutDirection) var layoutDirection @Environment(\.editMode) var editMode @@ -101,7 +108,13 @@ private struct ReaderSidebarView: View { .withDisabledSelection(isEditing) } } - + if !favorites.isEmpty || isEditing { + makeSection(Strings.favorites, isExpanded: $isSectionFavoritesExpanded) { + ForEach(favorites, id: \.self) { + ReaderSidebarSubscriptionCell(site: $0) + } + } + } if !teams.isEmpty { makeSection(Strings.organization, isExpanded: $isSectionOrganizationExpanded) { ReaderSidebarOrganizationSection(viewModel: viewModel, teams: teams) @@ -213,6 +226,7 @@ private extension View { private struct Strings { static let reader = SharedStrings.Reader.title static let subscriptions = NSLocalizedString("reader.sidebar.section.subscriptions.title", value: "Subscriptions", comment: "Reader sidebar section title") + static let favorites = NSLocalizedString("reader.sidebar.section.favorites.title", value: "Favorites", comment: "Reader sidebar section title") static let lists = NSLocalizedString("reader.sidebar.section.lists.title", value: "Lists", comment: "Reader sidebar section title") static let tags = NSLocalizedString("reader.sidebar.section.tags.title", value: "Tags", comment: "Reader sidebar section title") static let organization = NSLocalizedString("reader.sidebar.section.organization.title", value: "Organization", comment: "Reader sidebar section title") From d29cf660c56f083fa93bf4c194aba0ad3dc94595 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 19 Nov 2024 15:05:52 -0500 Subject: [PATCH 21/28] Fix searchBarCancelButtonClicked --- .../Reader/Controllers/ReaderSearchViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSearchViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSearchViewController.swift index 50ced18e797a..b7e8c3567df0 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSearchViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSearchViewController.swift @@ -231,7 +231,7 @@ extension ReaderSearchViewController: UISearchBarDelegate { public func searchBarCancelButtonClicked(_ searchBar: UISearchBar) { searchBar.resignFirstResponder() - suggestionsViewModel.searchText == "" + suggestionsViewModel.searchText = "" showSearchSuggestions() } } From f11e3857d0eaea4beba7e991e3485c3e592575a5 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 17:26:47 -0500 Subject: [PATCH 22/28] More menu minor updates (#23824) --- .../Me/Me Main/MeViewController.swift | 54 +++++++++---------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Me/Me Main/MeViewController.swift b/WordPress/Classes/ViewRelated/Me/Me Main/MeViewController.swift index 4fdeb9f5640c..d9627d7811a7 100644 --- a/WordPress/Classes/ViewRelated/Me/Me Main/MeViewController.swift +++ b/WordPress/Classes/ViewRelated/Me/Me Main/MeViewController.swift @@ -174,36 +174,12 @@ class MeViewController: UITableViewController { rows = loggedInRows + rows } - return rows + [helpAndSupportIndicator] - }()), - // middle section - ImmuTableSection(rows: { - var rows: [ImmuTableRow] = [] - - rows.append(ButtonRow( - title: Strings.submitFeedback, - textAlignment: .left, - action: showFeedbackView()) - ) - - rows.append(ButtonRow( - title: ShareAppContentPresenter.RowConstants.buttonTitle, - textAlignment: .left, - isLoading: sharePresenter.isLoading, - action: displayShareFlow()) - ) - - rows.append(ButtonRow( - title: RowTitles.about, - textAlignment: .left, - action: pushAbout(), - accessibilityIdentifier: "About") - ) return rows - }()) + }()), + ImmuTableSection(rows: [helpAndSupportIndicator]), ] - #if IS_JETPACK +#if IS_JETPACK if RemoteFeatureFlag.domainManagement.enabled() && loggedIn && !isSidebarModeEnabled { sections.append(.init(rows: [ NavigationItemRow( @@ -220,7 +196,29 @@ class MeViewController: UITableViewController { ]) ) } - #endif +#endif + + sections.append( + ImmuTableSection(rows: [ + ButtonRow( + title: ShareAppContentPresenter.RowConstants.buttonTitle, + textAlignment: .left, + isLoading: sharePresenter.isLoading, + action: displayShareFlow() + ), + ButtonRow( + title: Strings.submitFeedback, + textAlignment: .left, + action: showFeedbackView() + ), + ButtonRow( + title: RowTitles.about, + textAlignment: .left, + action: pushAbout(), + accessibilityIdentifier: "About" + ) + ]) + ) // last section sections.append( From bf48abf8c46931edca8fe0479bee9e10a51c7380 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 17:53:57 -0500 Subject: [PATCH 23/28] Fix site header background in dark mode (#23825) --- .../ViewRelated/Reader/Headers/ReaderSiteHeaderView.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Headers/ReaderSiteHeaderView.swift b/WordPress/Classes/ViewRelated/Reader/Headers/ReaderSiteHeaderView.swift index b61f22acdd46..0918a2a04bad 100644 --- a/WordPress/Classes/ViewRelated/Reader/Headers/ReaderSiteHeaderView.swift +++ b/WordPress/Classes/ViewRelated/Reader/Headers/ReaderSiteHeaderView.swift @@ -82,7 +82,6 @@ private struct ReaderSiteHeader: View { } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) .padding(EdgeInsets(top: 8, leading: 0, bottom: 16, trailing: 0)) - .background(Color(UIColor.secondarySystemGroupedBackground)) } private var countsDisplay: some View { From 34cf20aee5101aebace2acb90b67b9949adc38c8 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 17:54:25 -0500 Subject: [PATCH 24/28] Fix scroll-to-top in Reader (#23826) --- .../Reader/Sidebar/ReaderSidebarViewController.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift index 2bb662fdd1b4..51da321f3c22 100644 --- a/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Sidebar/ReaderSidebarViewController.swift @@ -46,6 +46,14 @@ final class ReaderSidebarViewController: UIHostingController { } } +extension ReaderSidebarViewController: ScrollableViewController { + func scrollViewToTop() { + if let scrollView = contentScrollView(for: .top) { + scrollView.scrollToTop(animated: true) + } + } +} + private struct ReaderSidebarView: View { @ObservedObject var viewModel: ReaderSidebarViewModel From 13c021a8279cf3bfaf62224bce0f7e5c45c5a251 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 17:55:05 -0500 Subject: [PATCH 25/28] Remove UIKitConstants (#23829) --- .../WordPressUI/Constants/UIKitConstants.swift | 9 --------- .../Extensions/UIView+Animations.swift | 18 +++++++++--------- .../FancyAlertPresentationController.swift | 10 +++++----- .../FancyAlert/FancyAlertViewController.swift | 2 +- 4 files changed, 15 insertions(+), 24 deletions(-) delete mode 100644 Modules/Sources/WordPressUI/Constants/UIKitConstants.swift diff --git a/Modules/Sources/WordPressUI/Constants/UIKitConstants.swift b/Modules/Sources/WordPressUI/Constants/UIKitConstants.swift deleted file mode 100644 index 461ad2bcb725..000000000000 --- a/Modules/Sources/WordPressUI/Constants/UIKitConstants.swift +++ /dev/null @@ -1,9 +0,0 @@ -import Foundation - -// MARK: UIKit Constants -// -public class UIKitConstants { - public static let alphaMid: CGFloat = 0.5 - public static let alphaZero: CGFloat = 0 - public static let alphaFull: CGFloat = 1 -} diff --git a/Modules/Sources/WordPressUI/Extensions/UIView+Animations.swift b/Modules/Sources/WordPressUI/Extensions/UIView+Animations.swift index ce556c3887c8..202cc0138d37 100644 --- a/Modules/Sources/WordPressUI/Extensions/UIView+Animations.swift +++ b/Modules/Sources/WordPressUI/Extensions/UIView+Animations.swift @@ -70,10 +70,10 @@ extension UIView { /// Applies a fade in animation /// public func fadeInAnimation(_ completion: ((Bool) -> Void)? = nil) { - alpha = UIKitConstants.alphaMid + alpha = 0.5 UIView.animate(withDuration: Animations.duration, animations: { [weak self] in - self?.alpha = UIKitConstants.alphaFull + self?.alpha = 1 }, completion: { success in completion?(success) }) @@ -83,11 +83,11 @@ extension UIView { /// public func fadeInWithRotationAnimation(_ completion: ((Bool) -> Void)? = nil) { transform = CGAffineTransform.makeRotation(-270, scale: 3) - alpha = UIKitConstants.alphaZero + alpha = 0 UIView.animate(withDuration: Animations.duration, animations: { self.transform = CGAffineTransform.makeRotation(0, scale: 0.75) - self.alpha = UIKitConstants.alphaFull + self.alpha = 1 }, completion: { _ in UIView.animate(withDuration: Animations.duration, animations: { self.transform = CGAffineTransform(scaleX: 1.0, y: 1.0) @@ -102,7 +102,7 @@ extension UIView { public func fadeOutWithRotationAnimation(_ completion: ((Bool) -> Void)? = nil) { UIView.animate(withDuration: Animations.duration, animations: { self.transform = CGAffineTransform.makeRotation(120, scale: 3) - self.alpha = UIKitConstants.alphaZero + self.alpha = 0 }, completion: { success in completion?(success) }) @@ -113,7 +113,7 @@ extension UIView { public func explodeAnimation(_ completion: ((Bool) -> Void)? = nil) { UIView.animate(withDuration: Animations.duration, animations: { self.transform = CGAffineTransform(scaleX: 3.0, y: 3.0) - self.alpha = UIKitConstants.alphaZero + self.alpha = 0 }, completion: { success in completion?(success) }) @@ -123,10 +123,10 @@ extension UIView { /// public func implodeAnimation(_ completion: ((Bool) -> Void)? = nil) { transform = CGAffineTransform(scaleX: 3.0, y: 3.0) - alpha = UIKitConstants.alphaZero + alpha = 0 UIView.animate(withDuration: Animations.duration, animations: { - self.alpha = UIKitConstants.alphaFull + self.alpha = 1 self.transform = CGAffineTransform(scaleX: 1.0, y: 1.0) }, completion: { success in completion?(success) @@ -143,7 +143,7 @@ extension UIView { } self.isHidden = false - let alpha: CGFloat = isHidden ? UIKitConstants.alphaZero : UIKitConstants.alphaFull + let alpha: CGFloat = isHidden ? 0 : 1 UIView.animate(withDuration: Animations.duration, delay: 0, options: .transitionCrossDissolve, animations: { self.alpha = alpha }) { success in diff --git a/Modules/Sources/WordPressUI/FancyAlert/FancyAlertPresentationController.swift b/Modules/Sources/WordPressUI/FancyAlert/FancyAlertPresentationController.swift index cf4d30b15172..c14593b01392 100644 --- a/Modules/Sources/WordPressUI/FancyAlert/FancyAlertPresentationController.swift +++ b/Modules/Sources/WordPressUI/FancyAlert/FancyAlertPresentationController.swift @@ -11,7 +11,7 @@ open class FancyAlertPresentationController: UIPresentationController, UIViewCon private let dimmingView: UIView = { $0.translatesAutoresizingMaskIntoConstraints = false $0.backgroundColor = UIColor(white: 0.0, alpha: Constants.dimmingViewAlpha) - $0.alpha = UIKitConstants.alphaZero + $0.alpha = 0 return $0 }(UIView()) @@ -22,12 +22,12 @@ open class FancyAlertPresentationController: UIPresentationController, UIViewCon containerView.pinSubviewToAllEdges(dimmingView) guard let transitionCoordinator = presentingViewController.transitionCoordinator else { - dimmingView.alpha = UIKitConstants.alphaFull + dimmingView.alpha = 1 return } transitionCoordinator.animate(alongsideTransition: { _ in - self.dimmingView.alpha = UIKitConstants.alphaFull + self.dimmingView.alpha = 1 }) } @@ -39,13 +39,13 @@ open class FancyAlertPresentationController: UIPresentationController, UIViewCon override open func dismissalTransitionWillBegin() { guard let coordinator = presentedViewController.transitionCoordinator else { - dimmingView.alpha = UIKitConstants.alphaZero + dimmingView.alpha = 0 return } coordinator.animate(alongsideTransition: { _ in - self.dimmingView.alpha = UIKitConstants.alphaZero + self.dimmingView.alpha = 0 }) } diff --git a/Modules/Sources/WordPressUI/FancyAlert/FancyAlertViewController.swift b/Modules/Sources/WordPressUI/FancyAlert/FancyAlertViewController.swift index 10b82a242b1f..1203f38a1af3 100644 --- a/Modules/Sources/WordPressUI/FancyAlert/FancyAlertViewController.swift +++ b/Modules/Sources/WordPressUI/FancyAlert/FancyAlertViewController.swift @@ -409,7 +409,7 @@ open class FancyAlertViewController: UIViewController { @objc func fadeAllViews(visible: Bool, alongside animation: ((FancyAlertViewController) -> Void)? = nil, completion: ((Bool) -> Void)? = nil) { UIView.animate(withDuration: Constants.fadeAnimationDuration, animations: { - self.alertView.contentViews.forEach { $0.alpha = (visible) ? UIKitConstants.alphaFull : UIKitConstants.alphaZero } + self.alertView.contentViews.forEach { $0.alpha = (visible) ? 1 : 0 } animation?(self) }, completion: completion) } From fa6caed56ebbecba218b8309b45864b13c29ad98 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 18:00:29 -0500 Subject: [PATCH 26/28] Fix footer height (#23828) --- .../Reader/Controllers/ReaderStreamViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift index ebb63c6dd294..aaaf00f7d9d6 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift @@ -71,7 +71,7 @@ import AutomatticTracks private let refreshInterval = 300 private var cleanupAndRefreshAfterScrolling = false private let recentlyBlockedSitePostObjectIDs = NSMutableArray() - private let heightForFooterView = CGFloat(34.0) + private let heightForFooterView = CGFloat(44) private let estimatedHeightsCache = NSCache() private var isFeed = false private var syncIsFillingGap = false From 8949d19b4e8781ba74eea90a8743dcb81497574b Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 18:30:15 -0500 Subject: [PATCH 27/28] Add like animation in post list (#23827) --- .../Reader/Cards/ReaderPostCell.swift | 18 ++++++++++++++++-- .../Reader/Cards/ReaderPostCellViewModel.swift | 6 +++--- .../ReaderPostActions/ReaderLikeAction.swift | 6 ++++-- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCell.swift b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCell.swift index 1384fa110338..b05350872134 100644 --- a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCell.swift +++ b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCell.swift @@ -245,7 +245,21 @@ private final class ReaderPostCellView: UIView { } @objc private func buttonLikeTapped() { - viewModel?.toggleLike() + guard let viewModel else { + return wpAssertionFailure("missing ViewModel") + } + if !viewModel.toolbar.isLiked { + var toolbar = viewModel.toolbar + toolbar.isLiked = true + toolbar.likeCount += 1 + configureToolbar(with: toolbar) + UINotificationFeedbackGenerator().notificationOccurred(.success) + buttons.like.imageView?.fadeInWithRotationAnimation { _ in + viewModel.toggleLike() + } + } else { + viewModel.toggleLike() + } } private func makeMoreMenu() -> [UIMenuElement] { @@ -365,7 +379,7 @@ private func makeAuthorButton() -> UIButton { private func makeButton(systemImage: String, font: UIFont = UIFont.preferredFont(forTextStyle: .footnote)) -> UIButton { var configuration = UIButton.Configuration.plain() configuration.image = UIImage(systemName: systemImage) - configuration.imagePadding = 8 + configuration.imagePadding = 6 configuration.preferredSymbolConfigurationForImage = UIImage.SymbolConfiguration(font: font) configuration.baseForegroundColor = .secondaryLabel configuration.contentInsets = .init(top: 16, leading: 12, bottom: 14, trailing: 12) diff --git a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCellViewModel.swift b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCellViewModel.swift index 50d1518018d8..95c8e0995ff0 100644 --- a/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCellViewModel.swift +++ b/WordPress/Classes/ViewRelated/Reader/Cards/ReaderPostCellViewModel.swift @@ -98,7 +98,7 @@ final class ReaderPostCellViewModel { } func toggleLike() { - ReaderLikeAction().execute(with: post) + ReaderLikeAction().execute(with: post, isFeedbackNeeded: false) } } @@ -107,8 +107,8 @@ struct ReaderPostToolbarViewModel { let isCommentsEnabled: Bool let commentCount: Int let isLikesEnabled: Bool - let likeCount: Int - let isLiked: Bool + var likeCount: Int + var isLiked: Bool static func make(post: ReaderPost) -> ReaderPostToolbarViewModel { ReaderPostToolbarViewModel( diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderLikeAction.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderLikeAction.swift index 6d5cc1a2fd8d..ca056e003136 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderLikeAction.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderPostActions/ReaderLikeAction.swift @@ -1,12 +1,14 @@ /// Encapsulates a command to toggle a post's liked status final class ReaderLikeAction { - func execute(with post: ReaderPost, source: String? = nil, completion: (() -> Void)? = nil) { + func execute(with post: ReaderPost, source: String? = nil, isFeedbackNeeded: Bool = true, completion: (() -> Void)? = nil) { if !post.isLiked { // Consider a like from the list to be enough to push a page view. // Solves a long-standing question from folks who ask 'why do I // have more likes than page views?'. ReaderHelpers.bumpPageViewForPost(post) - UINotificationFeedbackGenerator().notificationOccurred(.success) + if isFeedbackNeeded { + UINotificationFeedbackGenerator().notificationOccurred(.success) + } } let service = ReaderPostService(coreDataStack: ContextManager.shared) service.toggleLiked(for: post, source: source, success: { From ea3a267dc3b341d3d462d7c94590aa34be913563 Mon Sep 17 00:00:00 2001 From: Alex Grebenyuk Date: Tue, 19 Nov 2024 18:33:03 -0500 Subject: [PATCH 28/28] Remove downloadSiteIcon (#23832) --- .../Extensions/UIImageView+SiteIcon.swift | 120 ------------------ .../Views/NoteBlockHeaderTableViewCell.swift | 3 +- 2 files changed, 1 insertion(+), 122 deletions(-) delete mode 100644 WordPress/Classes/Extensions/UIImageView+SiteIcon.swift diff --git a/WordPress/Classes/Extensions/UIImageView+SiteIcon.swift b/WordPress/Classes/Extensions/UIImageView+SiteIcon.swift deleted file mode 100644 index c0e8d8b0a4e5..000000000000 --- a/WordPress/Classes/Extensions/UIImageView+SiteIcon.swift +++ /dev/null @@ -1,120 +0,0 @@ -import AlamofireImage -import AutomatticTracks -import Foundation -import Gridicons - -/// UIImageView Helper Methods that allow us to download a SiteIcon, given a website's "Icon Path" -/// -extension UIImageView { - - /// Default Settings - /// - struct SiteIconDefaults { - /// Default SiteIcon's Image Size, in points. - /// - static let imageSize = CGSize(width: 40, height: 40) - } - - /// Downloads the SiteIcon Image, hosted at the specified path. This method will attempt to optimize the URL, so that - /// the download Image Size matches `imageSize`. - /// - /// TODO: This is a convenience method. Nuke me once we're all swifted. - /// - /// - Parameter path: SiteIcon's url (string encoded) to be downloaded. - /// - @objc - func downloadSiteIcon(at path: String) { - downloadSiteIcon(at: path, placeholderImage: .siteIconPlaceholder) - } - - /// Downloads the SiteIcon Image, hosted at the specified path. This method will attempt to optimize the URL, so that - /// the download Image Size matches `imageSize`. - /// - /// - Parameters: - /// - path: SiteIcon's url (string encoded) to be downloaded. - /// - imageSize: Request site icon in the specified image size. - /// - placeholderImage: Yes. It's the "place holder image", Sherlock. - /// - @objc - func downloadSiteIcon( - at path: String, - imageSize: CGSize = SiteIconDefaults.imageSize, - placeholderImage: UIImage? - ) { - guard let siteIconURL = SiteIconViewModel.optimizedURL(for: path, imageSize: imageSize) else { - image = placeholderImage - return - } - - logURLOptimization(from: path, to: siteIconURL) - - let request = URLRequest(url: siteIconURL) - downloadSiteIcon(with: request, imageSize: imageSize, placeholderImage: placeholderImage) - } - - /// Downloads a SiteIcon image, using a specified request. - /// - /// - Parameters: - /// - request: The request for the SiteIcon. - /// - imageSize: Request site icon in the specified image size. - /// - placeholderImage: Yes. It's the "place holder image". - /// - private func downloadSiteIcon( - with request: URLRequest, - imageSize expectedSize: CGSize = SiteIconDefaults.imageSize, - placeholderImage: UIImage? - ) { - af.setImage(withURLRequest: request, placeholderImage: placeholderImage, completion: { [weak self] dataResponse in - switch dataResponse.result { - case .success(let image): - guard let self = self else { - return - } - - // In `MediaRequesAuthenticator.authenticatedRequestForPrivateAtomicSiteThroughPhoton` we're - // having to replace photon URLs for Atomic Private Sites, with a call to the Atomic Media Proxy - // endpoint. The downside of calling that endpoint is that it doesn't always return images of - // the requested size. - // - // The following lines of code ensure that we resize the image to the default Site Icon size, to - // ensure there is no UI breakage due to having larger images set here. - // - if image.size != expectedSize { - self.image = image.resized(to: expectedSize, format: .scaleAspectFill) - } else { - self.image = image - } - - self.layer.borderColor = UIColor.clear.cgColor - case .failure(let error): - if case .requestCancelled = error { - // Do not log intentionally cancelled requests as errors. - } else { - DDLogError("\(error.localizedDescription)") - } - } - }) - } -} - -// MARK: - Logging Support - -/// This is just a temporary extension to try and narrow down the caused behind this issue: https://sentry.io/share/issue/3da4662c65224346bb3a731c131df13d/ -/// -private extension UIImageView { - - private func logURLOptimization(from original: String, to optimized: URL) { - DDLogInfo("URL optimized from \(original) to \(optimized.absoluteString)") - } - - private func logURLOptimization(from original: String, to optimized: URL, for blog: Blog) { - let blogInfo: String - if blog.isAccessibleThroughWPCom() { - blogInfo = "dot-com-accessible: \(blog.url ?? "unknown"), id: \(blog.dotComID ?? 0)" - } else { - blogInfo = "self-hosted with url: \(blog.url ?? "unknown")" - } - - DDLogInfo("URL optimized from \(original) to \(optimized.absoluteString) for blog \(blogInfo)") - } -} diff --git a/WordPress/Classes/ViewRelated/Notifications/Views/NoteBlockHeaderTableViewCell.swift b/WordPress/Classes/ViewRelated/Notifications/Views/NoteBlockHeaderTableViewCell.swift index 162aaab7efca..c05420513ddb 100644 --- a/WordPress/Classes/ViewRelated/Notifications/Views/NoteBlockHeaderTableViewCell.swift +++ b/WordPress/Classes/ViewRelated/Notifications/Views/NoteBlockHeaderTableViewCell.swift @@ -67,11 +67,10 @@ class NoteBlockHeaderTableViewCell: NoteBlockTableViewCell { authorAvatarImageView.image = .gravatarPlaceholderImage return } - if let gravatar = AvatarURL(url: url) { authorAvatarImageView.downloadGravatar(gravatar, placeholder: .gravatarPlaceholderImage, animate: true) } else { - authorAvatarImageView.downloadSiteIcon(at: url.absoluteString) + authorAvatarImageView.wp.setImage(with: url, size: SiteIconViewModel.Size.regular.size) } }