Empty Stats: standalone card #17142

momo-ozawa · 2021-09-10T15:36:49Z

Description

This PR adds the new Grow Audience standalone card to the Insights tab of the Stats screen.

Added: Grow Audience standalone card
- All-time view count is displayed in the card
- Tapping the Dismiss button permanently dismisses the card
Added: Grow Audience ghost loading card
Updated: Customize card presentation logic
- The Customized card will only be displayed once the Grow Audience card is dismissed

Design ref: pbArwn-2NH-p2

Before	After

Notes

Other nudges (i.e. Social, Reader) will be added in future PRs.
The Set up blogging reminders button is not hooked up yet. Tapping it will do nothing.
The Grow Audience card presentation logic isn't hooked up to the view count yet. In the future, the card will be hidden once the all-time view count reaches a certain threshold.

How to test

Delete WPiOS and do a fresh install
Log in to an account with multiple sites
Go to My Sites (Let's call the current site Site A)
Go to Stats
✅ The Grow Audience card should be displayed for Site A
Go back to My Sites and choose a different site (Let's call this site Site B)
Go to Stats
✅ The Grow Audience card should be displayed for Site B
Tap Dismiss on the Grow Audience card
✅ The Grow Audience card should be dismissed for Site B, and the Customize card should be displayed instead
Go back to Site A > Stats
✅ The Grow Audience card should be dismissed for Site A, and the Customize card should be displayed instead

Regression Notes

Potential unintended areas of impact
Loading / dismissing the Customize card in the Stats screen
What I did to test those areas of impact (or what existing automated tests I relied on)
Manually tested the loading / dismissal flow
What automated tests I added (or what prevented me from doing so)
N/A

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.

peril-wordpress-mobile · 2021-09-10T15:36:54Z

You can trigger an installable build for these changes by visiting CircleCI here.

peril-wordpress-mobile · 2021-09-10T15:59:24Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

nikola-milicevic

When I tested this on the newly created site I encountered an error:

After going back, switching to some other (older) site, checking out stats (which didn't show Grow Audience card), and coming back again the the newly created site I got this result:

So I guess it's worth checking it out.

nikola-milicevic · 2021-09-13T14:48:08Z

WordPress/Classes/ViewRelated/Stats/Shared Views/GrowAudienceCell.swift

+    @IBAction private func actionButtonTapped(_ sender: UIButton) {
+    }


I would just put a comment here with the explanation why the body is empty.

Fixed! 6bd4538

osullivanchris · 2021-09-16T14:12:10Z

Had a look at the build and its looking great, very nice job 👌

Its hack week so I'm just capturing these thoughts, but no rush to respond.

A couple of things

I need to make a dark mode asset for you. Will do after hack week.
Do you prefer having the card stuck to the top, like the 'Customise your insights' card, or have some padding above? @leandroalonso preferred the padding in the original design. I'm just trying to be consistent. But without the padding maybe it looks a tad more banner-like
Would you be able to send me an iPad screenshot? I have trouble testing on my iPad. I can never get AppCentre to work.
I didn't look at landscape mode in my designs. Are you happy with it? I think maybe we could remove the padding above the icon? The other thing is the line-length looks a bit long, but not too bad. I played with having a max-width on the text, but then the icon looks lost.

momo-ozawa · 2021-09-20T09:54:57Z

@osullivanchris Thanks for all the feedback!

I need to make a dark mode asset for you. Will do after hack week.

Sounds good! Thanks!

Do you prefer having the card stuck to the top, like the 'Customise your insights' card, or have some padding above? @leandroalonso preferred the padding in the original design. I'm just trying to be consistent. But without the padding maybe it looks a tad more banner-like

I liked having the card stuck on top like the Customize card since it make the styling for Customize / Grow your audience more consistent. Adding padding is pretty simple though, so lmk if you feel strongly either way (cc: @leandroalonso) 😄

Would you be able to send me an iPad screenshot? I have trouble testing on my iPad. I can never get AppCentre to work.

Definitely!

iPad Air 4th gen (Portrait)	iPad Air 4th gen (Landscape)

I didn't look at landscape mode in my designs. Are you happy with it? I think maybe we could remove the padding above the icon? The other thing is the line-length looks a bit long, but not too bad. I played with having a max-width on the text, but then the icon looks lost.

I'm pretty happy with how landscape looks 👍

By removing padding above the icon, do you mean have it top-aligned with "x Views to your site so far"?

…ntains the insight

peril-wordpress-mobile · 2021-09-20T10:50:55Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

momo-ozawa · 2021-09-20T10:52:18Z

When I tested this on the newly created site I encountered an error:

After going back, switching to some other (older) site, checking out stats (which didn't show Grow Audience card), and coming back again the the newly created site I got this result:

Fixed! a0407f6
Thanks for catching this 🙏

momo-ozawa · 2021-09-20T10:53:04Z

@nikola-milicevic @leandroalonso Ready for a re-review 🙇‍♀️

osullivanchris · 2021-09-20T11:51:52Z

@momo-ozawa thanks for sharing the iPad screenshots. It looks good to me. In general I think landscape is probably fine.

By removing padding above the icon, do you mean have it top-aligned with "x Views to your site so far"?

In the specs I had '4pt below text' where I found when the image was top-aligned perfectly with the 'A tip to grow your audience' heading it looked wrong. But nudging it down slightly looked more 'correct'. But actually in the build we might not have this to begin with, and its probably a ridiculous detail.

On landscape though, I thought with just one line of text, the icon was floating a bit lower than I would like. It just doesn't look as well balanced. But even 4pt won't make a difference. It might look better on landscape if the icon was vertically centred to the block of text including the heading, but its a totally different structure to portrait so I'd imagine its way too messy to do.

leandroalonso

@momo-ozawa I was able to see the Insights and Blogging Reminders card at the same time (Atomic site):

I wonder if that was caused because I didn't do a clean installation? When I did a clean install it all worked fine.

Another thing regarding the ghost cell:

I wonder if we should clean it a little bit (like the icon placeholder and the "set up" button). It looks a bit polluted to me, but that's up to @osullivanchris (and we can tackle on another PR).

momo-ozawa · 2021-09-20T14:24:22Z

@leandroalonso

I wonder if that was caused because I didn't do a clean installation? When I did a clean install it all worked fine.

I think it's because it wasn't a clean installation. However, you bring up a really good point -- I should still handle the case where the customize card is already showing before the user updates the app to a version with Empty Stats. I'll address this issue ~~in the current PR~~ in a future PR.

I wonder if we should clean it a little bit (like the icon placeholder and the "set up" button). It looks a bit polluted to me, but that's up to @osullivanchris (and we can tackle on another PR).

Sounds good -- I'll tackle this in a future PR if needed.

nikola-milicevic

LGTM! 🎉

osullivanchris · 2021-09-22T10:45:39Z

Ghost state isn't a big deal or anything but here's a quick riff on it. I think there's just a little bit too many blocks, and maybe some of them are a bit chunky, that's all. You don't have to match this exactly.

Momo Ozawa added 12 commits September 10, 2021 12:40

Add: grow audience cell

6b2421e

Add: growAudience insight type

e4fda55

Add: stats ghost grow audience cell

283e54b

Update: configure grow audience row using all time stats

fafe50b

Refactor: make the grow audience insight type cacheable

6bedda4

Add: load grow audience card setting

0ca122c

Refactor: load customize card setting if grow audience isn't showing

db1dd65

Add: dismiss grow audience card

df3bfcf

Refactor: combine pinned card display logic into a single method

07922b5

Add: grow audience illustration assets

0f1f78a

Update: display icon in grow audience cell

63e139f

Refactor: update condition for the minimum index for movable cards

f15ff43

momo-ozawa added Stats [Type] Task labels Sep 10, 2021

momo-ozawa added this to the Someday milestone Sep 10, 2021

momo-ozawa requested review from leandroalonso and nikola-milicevic September 10, 2021 15:36

momo-ozawa self-assigned this Sep 10, 2021

momo-ozawa mentioned this pull request Sep 10, 2021

Empty Stats: refactor loading / dismissal logic for the customize card #17141

Merged

4 tasks

momo-ozawa linked an issue Sep 10, 2021 that may be closed by this pull request

Empty Stats: Standalone view #17123

Closed

8 tasks

nikola-milicevic reviewed Sep 13, 2021

View reviewed changes

Base automatically changed from task/empty-stats-refactor-customize-card-management to feature/empty-stats-phase-1 September 14, 2021 09:21

Momo Ozawa added 2 commits September 20, 2021 11:23

Fix: remove insight from list iff it's been dismissed AND the list co…

a0407f6

…ntains the insight

Add: comment

6bd4538

momo-ozawa requested a review from nikola-milicevic September 20, 2021 10:52

leandroalonso approved these changes Sep 20, 2021

View reviewed changes

nikola-milicevic approved these changes Sep 21, 2021

View reviewed changes

momo-ozawa merged commit 225fe01 into feature/empty-stats-phase-1 Sep 21, 2021

momo-ozawa deleted the task/empty-stats-standalone-card branch September 21, 2021 10:29

momo-ozawa mentioned this pull request Sep 22, 2021

Empty Stats: update grow audience ghost cell layout #17205

Merged

4 tasks

momo-ozawa mentioned this pull request Oct 15, 2021

Empty Stats (Phase 1): Publicize and Blogging Reminders #17329

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Empty Stats: standalone card #17142

Empty Stats: standalone card #17142

momo-ozawa commented Sep 10, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Sep 10, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Sep 10, 2021 •

edited

Loading

nikola-milicevic left a comment

nikola-milicevic Sep 13, 2021

momo-ozawa Sep 20, 2021

osullivanchris commented Sep 16, 2021

momo-ozawa commented Sep 20, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Sep 20, 2021

momo-ozawa commented Sep 20, 2021

momo-ozawa commented Sep 20, 2021

osullivanchris commented Sep 20, 2021

leandroalonso left a comment

momo-ozawa commented Sep 20, 2021 •

edited

Loading

nikola-milicevic left a comment

osullivanchris commented Sep 22, 2021

		@IBAction private func actionButtonTapped(_ sender: UIButton) {
		}

Empty Stats: standalone card #17142

Empty Stats: standalone card #17142

Conversation

momo-ozawa commented Sep 10, 2021 • edited Loading

Description

Notes

How to test

Regression Notes

peril-wordpress-mobile bot commented Sep 10, 2021 • edited Loading

peril-wordpress-mobile bot commented Sep 10, 2021 • edited Loading

nikola-milicevic left a comment

Choose a reason for hiding this comment

nikola-milicevic Sep 13, 2021

Choose a reason for hiding this comment

momo-ozawa Sep 20, 2021

Choose a reason for hiding this comment

osullivanchris commented Sep 16, 2021

momo-ozawa commented Sep 20, 2021 • edited Loading

peril-wordpress-mobile bot commented Sep 20, 2021

momo-ozawa commented Sep 20, 2021

momo-ozawa commented Sep 20, 2021

osullivanchris commented Sep 20, 2021

leandroalonso left a comment

Choose a reason for hiding this comment

momo-ozawa commented Sep 20, 2021 • edited Loading

nikola-milicevic left a comment

Choose a reason for hiding this comment

osullivanchris commented Sep 22, 2021

momo-ozawa commented Sep 10, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Sep 10, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Sep 10, 2021 •

edited

Loading

momo-ozawa commented Sep 20, 2021 •

edited

Loading

momo-ozawa commented Sep 20, 2021 •

edited

Loading