Hiding dashboard cards: add Personalize Home Tab screen #20369

kean · 2023-03-20T23:23:38Z

Related issue: #20296

Previous PRs (dependencies):

Hiding dashboard cards: generalize logic for storing preferences #20365

This PR introduces a new "Personalize Home Tab" button to the Home tab (Jetpack-only). It opens a new screen that allows toggling visibility for most of the cards on the dashboard.

Not every card is made personalizable yet (see DashboardCard.personalizableCards that controls the list). The remaining cards require special attention and will be covered separately.

Testing Instructions

Prerequisites

"Personalize Home Tab" feature flag is enabled

Test Case 1

Select site A and scroll to the bottom of the Home tab
Tap "Personalize Home Tab"
Use the switches to hide one of the cards
Verify that the card got hidden on the Home tab
Switch to site B and open the Home tab
Verify that the previously hidden card is still visible (preferences are saved on a per-site basis)

Testing Notes

Test on iOS 14 and other version to make sure the design is consistent across platforms and everything works as expected
Test on iPad
Try different font sizes (dynamic type), themes (light/dark), RTL languages, VoiceOver

Screenshots

Add button to Home tab	Add Personalize screen

Dynamic Type	Dynamic Type

Dark Mode	Dark Mode

RTL	RTL

iPad

Regression Notes

Potential unintended areas of impact: n/a (it's a new screen, so I don't expect it to affect any of the existing features)
What I did to test those areas of impact (or what existing automated tests I relied on): n/a
What automated tests I added (or what prevented me from doing so): I added a few unit tests for the new screen

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift

guarani · 2023-03-24T19:31:03Z

👋 @kean, is there a good way to see just the changes that this PR introduces without requiring #20365 to be merged first?

kean · 2023-03-24T19:36:15Z

👋 @kean, is there a good way to see just the changes that this PR introduces without requiring #20365 to be merged first?

Yes, @guarani, this PR has the following commits: https://github.com/wordpress-mobile/WordPress-iOS/pull/20369/files/2e08a5ec31734e3ea60a786aea748866c0b79638..1db57f64dac8911765444ab7de959ce79a429c19.

guarani

The new personalization is looking great!

I gave this a test and things worked smoothly except for a visual glitch when hiding all cards while the screen is scrolled all the way to the bottom:

scroll-glitch.mov

Trying this out, I can see how the personalization options for the "draft post" card and "scheduled post" dashboard cards can be unintuitive when there is no draft or scheduled post. Perhaps this needs to be made more intuitive before we release this. I don't think this is a blocker to this PR, but worth addressing before we ship this to users.

I'm approving this but I think we need @osullivanchris's input to give this. Some installable builds should pop up here, I've run CI now and it's building as of the time of writing.

guarani · 2023-03-24T19:54:26Z

RELEASE-NOTES.txt

@@ -1,6 +1,6 @@
 22.1
 -----
-
+* [*] Add a Personalize Home Tab button to the bottom of the Home tab that allows changing visibility for the cards on the Home tab [#20369]


Nice job on the release notes. You could change this to ** to give this a signal this is of higher importance when the final editorialized release notes are made. However, I know this isn't documented publically, so I've created a PR to address this: #20402.

I increased the priority (and also fixes the merge conflicts in the release notes).

WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift

WordPress/Classes/ViewRelated/Blog/BlogPersonalization/BlogDashboardPersonalizationView.swift

guarani · 2023-03-24T22:39:52Z

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/BlogDashboardPersonalizeCardCell.swift

+    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)
+    }


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.

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.

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.

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

guarani · 2023-03-24T23:00:05Z

Yes, @guarani, this PR has the following commits: https://github.com/wordpress-mobile/WordPress-iOS/pull/20369/files/2e08a5ec31734e3ea60a786aea748866c0b79638..1db57f64dac8911765444ab7de959ce79a429c19.

That works and is an option, I did that here now to review the PR. What do you think about targeting the previous PR's branch (the one this depends on) instead of trunk? A nice effect is that when the first PR is merged, GitHub should automatically update this PR to point to trunk, so it shouldn't add manual work. Just a suggestion, and not sure if you've already considered it.

kean · 2023-03-25T01:32:55Z

What do you think about targeting the previous PR's branch (the one this depends on) instead of trunk?

Great thinking, but It doesn't seem to be possible for branches in forks. The branch from the previous PR doesn't exist in the main repo and I can't target it.

I gave this a test and things worked smoothly except for a visual glitch when hiding all cards while the screen is scrolled all the way to the bottom:

Nice catch. I put it on the list of defects.

osullivanchris · 2023-03-27T09:08:20Z

Trying this out, I can see how the personalization options for the "draft post" card and "scheduled post" dashboard cards can be unintuitive when there is no draft or scheduled post. Perhaps this needs to be made more intuitive before we release this. I don't think this is a blocker to this PR, but worth addressing before we ship this to users.

I'm approving this but I think we need @osullivanchris's input to give this. Some installable builds should pop up here, I've run CI now and it's building as of the time of writing.

Oh super interesting, good catch @guarani

I think there's two problems: scroll position after personalising, and turning on cards with no content.

Re; scroll position. What do you both think about this. If no changes made, retain scroll position. If user taps some switches, scroll back to top when the modal presentation closes. Does that make sense?

Re; draft/scheduled posts. I have that line at the bottom of the list saying that some cards won't have content. But it could be easily ignored in this case. I can think of two things:

We could add subheadings to the list items, describing the card a little. E.g. title 'Draft Posts', subtitle 'Shows any recently created/edited drafts'.
We could add subheadings only to items that need some warning or description e.g. title: 'Draft posts', caption 'Only shows if you have recently created/edited draft posts'
We could show some empty state for the card on the dashboard. I don't really love this though.

Any opinions on those 3 approaches?

kean · 2023-03-27T19:23:02Z

Re; scroll position. What do you both think about this. If no changes made, retain scroll position. If user taps some switches, scroll back to top when the modal presentation closes. Does that make sense?

The behavior from the video is a defect (minor). I added it to my list and will address it in one of the upcoming PRs. The only way to access the Personalize button is by scrolling to the very bottom. I think it'll be best if, after you close the Personalize screen, you'll be back to where you were: at the bottom of the screen.

Re; draft/scheduled posts. I have that line at the bottom of the list saying that some cards won't have content. But it could be easily ignored in this case.

It depends on how you think about these widgets. If you take the iOS Today view, you, as a user, are expected to create your own dashboard by adding widgets. If you add a widget, and it doesn't show up, it would've been weird. But in the case of Wordpress, all widgets are already enabled by default.

I think of the Personalize screen in its current state as either a way to undo the "Hide this" action or as a way to filter out the widgets you don't want to see. The section footer text at the bottom already alludes to the fact that the cards are dynamic: "Cards may show different content depending on what's happening on your site."

osullivanchris · 2023-03-28T09:05:40Z

The behavior from #20369 (review) is a defect (minor). I added it to my list and will address it in one of the upcoming PRs. The only way to access the Personalize button is by scrolling to the very bottom. I think it'll be best if, after you close the Personalize screen, you'll be back to where you were: at the bottom of the screen.

Sounds fine to start with.

It depends on how you think about these widgets. If you take the iOS Today view, you, as a user, are expected to create your own dashboard by adding widgets. If you add a widget, and it doesn't show up, it would've been weird. But in the case of Wordpress, all widgets are already enabled by default.

I think of the Personalize screen in its current state as either a way to undo the "Hide this" action or as a way to filter out the widgets you don't want to see. The section footer text at the bottom already alludes to the fact that the cards are dynamic: "Cards may show different content depending on what's happening on your site."

You've made a good case there, thanks. Let's stick with what we have.

guarani · 2023-03-29T04:32:13Z

Great thinking, but It doesn't seem to be possible for branches in forks. The branch from the previous PR doesn't exist in the main repo and I can't target it.

Ah, that's a shame.

I re-triggered the CI because of a failure and updated it to the latest commit. Looks like there's another release note conflict to resolve though.

Co-authored-by: Paul Von Schrottky <[email protected]>

kean · 2023-03-29T12:24:22Z

@guarani, rebased with trunk. If merge is preferred, please let me know.

There is a new "Domains" card that was added to the dashboard in trunk. I updated the code to compile but haven't added it to the Personalize screen.

case .domainsDashboardCard:
    /// TODO
    return DashboardFailureCardCell.self

staskus · 2023-03-29T12:46:29Z

@guarani, rebased with trunk. If merge is preferred, please let me know.

There is a new "Domains" card that was added to the dashboard in trunk. I updated the code to compile but haven't added it to the Personalize screen.
case .domainsDashboardCard:
    /// TODO
    return DashboardFailureCardCell.self

@kean thanks for that. There's a new domain card that is in the process of being created and is hidden under the feature flag.

Feel free to proceed with merging this PR, we will make sure to include it in a new personalization view. As I understand, each card will need to be added there manually?

kean · 2023-03-29T13:14:44Z

Hey, @staskus! Thanks for the heads up.

Not every card is personalizable, though it looks like the "Domains" card needs to be. @osullivanchris, what do you think?

As I understand, each card will need to be added there manually?

There are three steps to add the full hide/unhide support for a card (but not everything is merged yet):

Add a user defaults key to BlogDashboardPersonalizationService(more details and rational here)
Add the card to DashboardCard.personalizableCards and provide a localized title in BlogDashboardPersonalizationCardCellViewModel (both added in this PR)
Add the "Hide this" action to the card's context menu. I'm adding a reusable makeHideCardAction methods to make it easy in Hiding dashboard cards: add context menus #20384

We could potentially remove one of these steps. The list of personalizable cards could be auto-generated based on whether the card has the associated key, but the order of the cards on the Personalize screen doesn't match the order on the Dashboard, so it needs to be set somewhere. @osullivanchris, can we make the order the same? It makes sense, especially if we eventually add the ability to re-order the cards, which is being discussed.

osullivanchris · 2023-03-29T13:53:48Z

Not every card is personalizable, though it looks like the "Domains" card needs to be. @osullivanchris, what do you think?

Yeah I think the user should be able to hide this if they want to.

We could potentially remove one of these steps. The list of personalizable cards could be auto-generated based on whether the card has the associated key, but the order of the cards on the Personalize screen doesn't match the order on the Dashboard, so it needs to be set somewhere. @osullivanchris, can we make the order the same? It makes sense, especially if we eventually add the ability to re-order the cards, which is being discussed.

If the order of the menu was automatically derived from the order that cards are displayed no the dashboard, I think that would be great. Whatever order I put in my mockup was arbitrary, just for showing how it would look.

staskus · 2023-03-29T14:00:41Z

Thanks for the explanation, @kean!

Not every card is personalizable, though it looks like the "Domains" card needs to be

Yes, it will probably be. Since we're still creating the card, we'll be happy to try using the flow you described and see if we notice anything that could be improved from the developers' perspective.

We could potentially remove one of these steps. The list of personalizable cards could be auto-generated based on whether the card has the associated key

That definitely makes sense to me. I don't want to be jumping into this project and stalling the PR. We could make such improvements later if needed.

guarani · 2023-03-30T05:37:42Z

@guarani, rebased with trunk. If merge is preferred, please let me know.

👋 Hey @kean, we normally merge when updating a PR from trunk.

I triggered CI on this branch again and enabled auto-merge 👍

hassaanelgarem · 2023-04-02T23:34:48Z

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/DashboardCard.swift

+    static let personalizableCards: [DashboardCard] = [
+        .todaysStats,
+        .draftPosts,
+        .scheduledPosts,
+        .blaze,
+        .prompts
+    ]
+


@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.

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.

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 👍

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.

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!

Notice that the entry that was removed did not have the "Jetpack-only" label but was still a Jetpack-only feature. I verified this by following the instructions in #20369 and not seeing the button to personalize the home screen. This is consistent with only Jetpack offering a home screen that is enhanced with cards.

There were conflicts on `Podfile` and `Podfile.lock` because `release/22.1` and `trunk` both changed the Gutenberg version. As usual, I resolved them with `git checkout --theirs`, keeping the value from `trunk` under the assumption that the latest version of Gutenberg, 1.93.0-alpha2, is what `trunk` needs to work and that if the 1.92.1 hotfix hasn't already been integrated in the latest release, it will be soon and the Gutenberg team will followup with a new version update on `trunk`. There was also a conflict on the `RELEASE-NOTES.txt` that GitHub picked up with its more refined conflicts engine and that auto-merge didn't address properly but that I manually fixed. That was due to `release/22.1` adding new release note entries – which will not make it into the App Store release notes – in the 22.1 section and `trunk` having #20128 editing that section, too. #20128 was meant to land on `release/22.1` but I forgot to update the base branch after starting the code freeze 🤦‍♂️😭 ``` <<<<<<< release/22.1 -- Incoming Change * [**] Add a "Personalize Home Tab" button to the bottom of the Home tab that allows changing cards visibility. [#20369] * [*] [Reader] Fix an issue that was causing the app to crash when tapping the More or Share buttons in Reader Detail screen. [#20490] * [*] Block editor: Avoid empty Gallery block error [WordPress/gutenberg#49557] ======= * [**] [internal] Refactored Google SignIn implementation to not use the Google SDK [#20128] >>>>>>> trunk -- Current Change ```

There were conflicts on `Podfile` and `Podfile.lock` because `release/22.1` and `trunk` both changed the Gutenberg version. As usual, I resolved them with `git checkout --theirs`, keeping the value from `trunk` under the assumption that the latest version of Gutenberg, 1.93.0-alpha2, is what `trunk` needs to work and that if the 1.92.1 hotfix hasn't already been integrated in the latest release, it will be soon and the Gutenberg team will followup with a new version update on `trunk`. There was also a conflict on the `RELEASE-NOTES.txt` that GitHub picked up with its more refined conflicts engine and that auto-merge didn't address properly but that I manually fixed. That was due to `release/22.1` adding new release note entries – which will not make it into the App Store release notes – in the 22.1 section and `trunk` having #20128 editing that section, too. #20128 was meant to land on `release/22.1` but I forgot to update the base branch after starting the code freeze 🤦‍♂️😭 Also, the entry about the personalize home tab had been removed on `trunk` via ec8a377. The release notes editor was notified about it, #20483 (review) ``` <<<<<<< release/22.1 -- Incoming Change * [**] Add a "Personalize Home Tab" button to the bottom of the Home tab that allows changing cards visibility. [#20369] * [*] [Reader] Fix an issue that was causing the app to crash when tapping the More or Share buttons in Reader Detail screen. [#20490] * [*] Block editor: Avoid empty Gallery block error [WordPress/gutenberg#49557] ======= * [**] [internal] Refactored Google SignIn implementation to not use the Google SDK [#20128] >>>>>>> trunk -- Current Change ```

kean marked this pull request as ready for review March 20, 2023 23:25

kean commented Mar 22, 2023

View reviewed changes

WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift Show resolved Hide resolved

kean marked this pull request as draft March 22, 2023 00:10

kean marked this pull request as ready for review March 22, 2023 21:43

This was referenced Mar 22, 2023

Hiding dashboard cards: add context menus #20384

Merged

Hiding dashboard cards: add empty state view #20385

Merged

Hiding dashboard cards: make quick start cards personalizable #20393

Merged

guarani mentioned this pull request Mar 24, 2023

Temporary PR to trigger build of fork PR #20403

Merged

guarani approved these changes Mar 24, 2023

View reviewed changes

kean and others added 9 commits March 29, 2023 08:12

Add Personalize Home Tab screen

00ee6ce

Fix BlogDashboardPersonalizationView design on iOS 14

a6eaa97

Add analyticts to new personalization flow on dashboard

99b7f22

Add BlogDashboardPersonalizationView iPad support

da4f699

Add BlogDashboardPersonalizationViewModel tests and docs

2b9a082

Update to support latest BlogDashboardPersonalizationService changes

d5b1676

Fix localizable string value

e23baec

Co-authored-by: Paul Von Schrottky <[email protected]>

Update personalize screen size on iPad

e7ef530

Update release notes

fb7d8d6

guarani enabled auto-merge March 30, 2023 05:38

guarani merged commit dd5e2a2 into wordpress-mobile:trunk Mar 30, 2023

This was referenced Mar 30, 2023

Hiding dashboard cards: remove prompts from site settings #20453

Merged

Hiding dashboard cards: fix scroll position #20454

Merged

staskus added [Type] Enhancement Part of a WIP Feature labels Mar 31, 2023

hassaanelgarem reviewed Apr 2, 2023

View reviewed changes

kean mentioned this pull request Apr 3, 2023

Hiding dashboard cards: register cards automatically #20466

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hiding dashboard cards: add Personalize Home Tab screen #20369

Hiding dashboard cards: add Personalize Home Tab screen #20369

kean commented Mar 20, 2023 •

edited

Loading

guarani commented Mar 24, 2023

kean commented Mar 24, 2023

guarani left a comment •

edited

Loading

guarani Mar 24, 2023

kean Mar 27, 2023

guarani Mar 24, 2023

kean Mar 25, 2023

guarani Mar 29, 2023

guarani commented Mar 24, 2023

kean commented Mar 25, 2023 •

edited

Loading

osullivanchris commented Mar 27, 2023

kean commented Mar 27, 2023

osullivanchris commented Mar 28, 2023

guarani commented Mar 29, 2023

kean commented Mar 29, 2023 •

edited

Loading

staskus commented Mar 29, 2023

kean commented Mar 29, 2023 •

edited

Loading

osullivanchris commented Mar 29, 2023

staskus commented Mar 29, 2023

guarani commented Mar 30, 2023

hassaanelgarem Apr 2, 2023

kean Apr 3, 2023 •

edited

Loading

hassaanelgarem Apr 3, 2023

hassaanelgarem Apr 3, 2023

kean Apr 3, 2023

-    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 })
+    }

Hiding dashboard cards: add Personalize Home Tab screen #20369

Hiding dashboard cards: add Personalize Home Tab screen #20369

Conversation

kean commented Mar 20, 2023 • edited Loading

Testing Instructions

Screenshots

Regression Notes

guarani commented Mar 24, 2023

kean commented Mar 24, 2023

guarani left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guarani commented Mar 24, 2023

kean commented Mar 25, 2023 • edited Loading

osullivanchris commented Mar 27, 2023

kean commented Mar 27, 2023

osullivanchris commented Mar 28, 2023

guarani commented Mar 29, 2023

kean commented Mar 29, 2023 • edited Loading

staskus commented Mar 29, 2023

kean commented Mar 29, 2023 • edited Loading

osullivanchris commented Mar 29, 2023

staskus commented Mar 29, 2023

guarani commented Mar 30, 2023

Choose a reason for hiding this comment

kean Apr 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kean commented Mar 20, 2023 •

edited

Loading

guarani left a comment •

edited

Loading

kean commented Mar 25, 2023 •

edited

Loading

kean commented Mar 29, 2023 •

edited

Loading

kean commented Mar 29, 2023 •

edited

Loading

kean Apr 3, 2023 •

edited

Loading