Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hiding dashboard cards: add Personalize Home Tab screen #20369

Merged
merged 9 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* [**] [internal] Refactor updating account related Core Data operations, which ususally happens during log in and out of the app. [#20394]
* [***] [internal] Refactor uploading photos (from the device photo, the Free Photo library, and other sources) to the WordPress Media Library. Affected areas are where you can choose a photo and upload, including the "Media" screen, adding images to a post, updating site icon, etc. [#20322]
* [**] [WordPress-only] Warns user about sites with only individual plugins not supporting core app features and offers the option to switch to the Jetpack app. [#20408]
* [**] Add a "Personalize Home Tab" button to the bottom of the Home tab that allows changing cards visibility. [#20369]

22.0
-----
Expand Down
5 changes: 5 additions & 0 deletions WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ enum FeatureFlag: Int, CaseIterable {
case jetpackIndividualPluginSupport
case siteCreationDomainPurchasing
case readerUserBlocking
case personalizeHomeTab

/// Returns a boolean indicating if the feature is enabled
var enabled: Bool {
Expand Down Expand Up @@ -125,6 +126,8 @@ enum FeatureFlag: Int, CaseIterable {
return false
case .readerUserBlocking:
return true
case .personalizeHomeTab:
return false
kean marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -221,6 +224,8 @@ extension FeatureFlag {
return "Site Creation Domain Purchasing"
case .readerUserBlocking:
return "Reader User Blocking"
case .personalizeHomeTab:
return "Personalize Home Tab"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import UIKit
import SwiftUI

final class BlogDashboardPersonalizeCardCell: DashboardCollectionViewCell {
private var blog: Blog?
private weak var presentingViewController: BlogDashboardViewController?

private let personalizeButton = UIButton(type: .system)

// MARK: - Initializers

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

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

// MARK: - View setup

private func setupView() {
let titleLabel = UILabel()
titleLabel.text = Strings.buttonTitle
titleLabel.font = WPStyleGuide.fontForTextStyle(.subheadline, fontWeight: .semibold)
titleLabel.adjustsFontForContentSizeCategory = true

let imageView = UIImageView(image: UIImage(named: "personalize")?.withRenderingMode(.alwaysTemplate))
imageView.tintColor = .label

let spacer = UIView()
spacer.translatesAutoresizingMaskIntoConstraints = false
spacer.widthAnchor.constraint(greaterThanOrEqualToConstant: 8).isActive = true

let contents = UIStackView(arrangedSubviews: [titleLabel, spacer, imageView])
contents.alignment = .center
contents.isUserInteractionEnabled = false

personalizeButton.accessibilityLabel = Strings.buttonTitle
personalizeButton.setBackgroundImage(.renderBackgroundImage(fill: .tertiarySystemFill), for: .normal)
personalizeButton.addTarget(self, action: #selector(buttonTapped), for: .touchUpInside)

let container = UIView()
container.layer.cornerRadius = 10
container.addSubview(personalizeButton)
container.addSubview(contents)

personalizeButton.translatesAutoresizingMaskIntoConstraints = false
container.pinSubviewToAllEdges(personalizeButton)

contents.translatesAutoresizingMaskIntoConstraints = false
container.pinSubviewToAllEdges(contents, insets: .init(allEdges: 16))

contentView.addSubview(container)
container.translatesAutoresizingMaskIntoConstraints = false
contentView.pinSubviewToAllEdges(container)
}
Comment on lines +23 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

not a blocker: Could a UIButton's text and images properties and insets simplify this by avoiding separate subviews? I'm curious about your thoughts here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making UIButton look like this isn't easy. By default, it displays the image on the left side, and there is no (good) way to set flexible spacing for the image. I wish it was as easy to use as the SwiftUI Button.

There is a new UIButton.Configuration that almost gets you there, but it's iOS 15+ only.

Screenshot 2023-03-24 at 9 54 47 PM

There is a way to do it with negative image/title insets, but then again, there is flexible spacing. I could use layoutSubviews to adjust the spacing dynamically based on the container and the title size. But it would've been over-complicated.

I considered using a simple gesture recognizer for handling taps on this card, but UIButton has a nice highlighted state and works better for accessibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for sharing! I agree about UIButton image/title insets being complicated.


override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
super.traitCollectionDidChange(previousTraitCollection)

personalizeButton.setBackgroundImage(.renderBackgroundImage(fill: .tertiarySystemFill), for: .normal)
}

// MARK: - BlogDashboardCardConfigurable

func configure(blog: Blog, viewController: BlogDashboardViewController?, apiResponse: BlogDashboardRemoteEntity?) {
self.blog = blog
self.presentingViewController = viewController
}

// MARK: - Actions

@objc private func buttonTapped() {
guard let blog = blog, let siteID = blog.dotComID?.intValue else {
return DDLogError("Failed to show dashboard personalization screen: siteID is missing")
}
WPAnalytics.track(.dashboardCardItemTapped, properties: ["type": DashboardCard.personalize.rawValue], blog: blog)
let viewController = UIHostingController(rootView: NavigationView {
BlogDashboardPersonalizationView(viewModel: .init(service: .init(siteID: siteID)))
}.navigationViewStyle(.stack)) // .stack is required for iPad
if UIDevice.isPad() {
viewController.modalPresentationStyle = .formSheet
}
presentingViewController?.present(viewController, animated: true)
}
}

private extension BlogDashboardPersonalizeCardCell {
struct Strings {
static let buttonTitle = NSLocalizedString("dasboard.personalizeHomeButtonTitle", value: "Personalize your home tab", comment: "Personialize home tab button title")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ enum DashboardCard: String, CaseIterable {
case nextPost = "create_next"
case createPost = "create_first"
case jetpackBadge

// Card placeholder for when loading data
/// Card placeholder for when loading data
case ghost
case failure
/// A "Personalize Home Tab" button
case personalize

var cell: DashboardCollectionViewCell.Type {
switch self {
Expand Down Expand Up @@ -52,6 +53,8 @@ enum DashboardCard: String, CaseIterable {
case .domainsDashboardCard:
/// TODO
return DashboardFailureCardCell.self
case .personalize:
return BlogDashboardPersonalizeCardCell.self
}
}

Expand Down Expand Up @@ -88,6 +91,8 @@ enum DashboardCard: String, CaseIterable {
return BlazeHelper.shouldShowCard(for: blog)
case .domainsDashboardCard:
return DomainsDashboardCardHelper.shouldShowCard(for: blog)
case .personalize:
return AppConfiguration.isJetpack && FeatureFlag.personalizeHomeTab.enabled
}
}

Expand All @@ -111,6 +116,15 @@ enum DashboardCard: String, CaseIterable {
}
}

/// A list of cards that can be shown/hidden on a "Personalize Home Tab" screen.
static let personalizableCards: [DashboardCard] = [
.todaysStats,
.draftPosts,
.scheduledPosts,
.blaze,
.prompts
]

Comment on lines +120 to +127
Copy link
Contributor

Choose a reason for hiding this comment

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

@kean Sorry for being late to the party here, but what do you think about doing something like this?

Suggested change
static let personalizableCards: [DashboardCard] = [
.todaysStats,
.draftPosts,
.scheduledPosts,
.blaze,
.prompts
]
// Boolean indicating whether the DashboardCard can be shown/hidden from the "Personalize Home Tab" screen.
var isPersonalizable: Bool {
switch self {
// add a case for each card
}
}
static var personalizableCards: [DashboardCard] {
DashboardCard.allCases.filter({ $0.isPersonalizable })
}

This will help us avoid not adding future cards to the personalize screen if applicable, especially for developers unaware of these new changes.

Copy link
Contributor Author

@kean kean Apr 3, 2023

Choose a reason for hiding this comment

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

I like that. The personalizableCards were initially defined as an array because the order was important – it didn't match the cards' order on the Dashboard. But I just confirmed late last week that it could and should match.

I'm planning to generate this array based on whether the BlogDashboardPersonalizationService defines a key for a card:

static var personalizableCards: [DashboardCard] {
    DashboardCard.allCases.filter {
        BlogDashboardPersonalizationService.makeKey(for: $0 != nil)
    }
}

The keys are also defined using a switch, so the compiler will surface it to any developers adding new cards.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will achieve the same thing I had in mind 👍
The only issue is we'll have to make BlogDashboardPersonalizationService.makeKey public for this reason alone. It might make more sense to move personalizableCards to BlogDashboardPersonalizationService at that point. This way, we'll have all the personalization logic in one place. Just something to think about 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

My personal recommendation would be to change makeKey() into a computed property DashboardCard.personalizeKey defined in a private extension in BlogDashboardPersonalizationService.swift. This will enhance the verbosity of the code.
And move personalizableCards to BlogDashboardPersonalizationService.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened a draft PR with this change: #20466.

It might make more sense to move personalizableCards to BlogDashboardPersonalizationService at that point.

Makes total sense, thanks!

/// Includes all cards that should be fetched from the backend
/// The `String` should match its identifier on the backend.
enum RemoteDashboardCard: String, CaseIterable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ private func makeKey(for card: DashboardCard) -> String? {
// avoid having to migrate data.
return "prompts-enabled-site-settings"
case .domainsDashboardCard:
return "domains-dashboard-enabled-site-settings"
case .quickStart, .jetpackBadge, .jetpackInstall, .nextPost, .createPost, .failure, .ghost:
return "domains-dashboard-card-enabled-site-settings"
case .quickStart, .jetpackBadge, .jetpackInstall, .nextPost, .createPost, .failure, .ghost, .personalize:
return nil
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import SwiftUI

struct BlogDashboardPersonalizationView: View {
@StateObject var viewModel: BlogDashboardPersonalizationViewModel

@SwiftUI.Environment(\.presentationMode) var presentationMode

var body: some View {
List {
Section(content: {
ForEach(viewModel.cards, content: BlogDashboardPersonalizationCardCell.init)
}, header: {
Text(Strings.sectionHeader)
}, footer: {
Text(Strings.sectionFooter)
})
}
.listStyle(.insetGrouped)
.navigationTitle(Strings.title)
.navigationBarTitleDisplayMode(.inline)
.toolbar {
ToolbarItem(placement: .navigationBarLeading) { closeButton }
}
}

private var closeButton: some View {
Button(action: { presentationMode.wrappedValue.dismiss() }) {
Image(systemName: "xmark")
.font(.body.weight(.medium))
.foregroundColor(Color.primary)
}
}
}

private struct BlogDashboardPersonalizationCardCell: View {
@ObservedObject var viewModel: BlogDashboardPersonalizationCardCellViewModel

var body: some View {
Toggle(viewModel.title, isOn: $viewModel.isOn)
}
}

private extension BlogDashboardPersonalizationView {
struct Strings {
static let title = NSLocalizedString("personalizeHome.title", value: "Personalize Home Tab", comment: "Page title")
static let sectionHeader = NSLocalizedString("personalizeHome.cardsSectionHeader", value: "Add or hide cards", comment: "Section header")
static let sectionFooter = NSLocalizedString("personalizeHome.cardsSectionFooter", value: "Cards may show different content depending on what's happening on your site. We're working on more cards and controls.", comment: "Section footer displayed below the list of toggles")
}
}

#if DEBUG
struct BlogDashboardPersonalizationView_Previews: PreviewProvider {
static var previews: some View {
NavigationView {
BlogDashboardPersonalizationView(viewModel: .init(service: .init(repository: UserDefaults.standard, siteID: 1)))
}
}
}
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import SwiftUI

final class BlogDashboardPersonalizationViewModel: ObservableObject {
let cards: [BlogDashboardPersonalizationCardCellViewModel]

init(service: BlogDashboardPersonalizationService) {
self.cards = DashboardCard.personalizableCards.map {
BlogDashboardPersonalizationCardCellViewModel(card: $0, service: service)
}
}
}

final class BlogDashboardPersonalizationCardCellViewModel: ObservableObject, Identifiable {
private let card: DashboardCard
private let service: BlogDashboardPersonalizationService

var id: DashboardCard { card }
var title: String { card.localizedTitle }

var isOn: Bool {
get { service.isEnabled(card) }
set {
objectWillChange.send()
service.setEnabled(newValue, for: card)
}
}

init(card: DashboardCard, service: BlogDashboardPersonalizationService) {
self.card = card
self.service = service
}
}

private extension DashboardCard {
var localizedTitle: String {
switch self {
case .prompts:
return NSLocalizedString("personalizeHome.dashboardCard.prompts", value: "Blogging prompts", comment: "Card title for the pesonalization menu")
case .blaze:
return NSLocalizedString("personalizeHome.dashboardCard.blaze", value: "Blaze", comment: "Card title for the pesonalization menu")
case .todaysStats:
return NSLocalizedString("personalizeHome.dashboardCard.todaysStats", value: "Today's stats", comment: "Card title for the pesonalization menu")
case .draftPosts:
return NSLocalizedString("personalizeHome.dashboardCard.draftPosts", value: "Draft posts", comment: "Card title for the pesonalization menu")
case .scheduledPosts:
return NSLocalizedString("personalizeHome.dashboardCard.scheduledPosts", value: "Scheduled posts", comment: "Card title for the pesonalization menu")
case .quickStart, .nextPost, .createPost, .ghost, .failure, .personalize, .jetpackBadge, .jetpackInstall, .domainsDashboardCard:
assertionFailure("\(self) card should not appear in the personalization menus")
return "" // These cards don't appear in the personalization menus
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"images" : [
{
"filename" : "personalize.pdf",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
}
}
Binary file not shown.
Loading