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

[Stats Refresh] Insights Latest Post Summary: update design #11289

Merged
merged 2 commits into from
Mar 18, 2019

Conversation

ScoutHarris
Copy link
Contributor

Fixes #11284

This updates Insights Latest Post Summary to match the new design:

  • Move “views” to top and use full number
  • Move “likes” and “comments” to bottom as table rows
  • The Views number now abbreviates after 99,999.

To test:


  • On a site that has a post with views/comments/likes, go to Insights > Latest Post Summary.
  • Verify the view is now:

lps


  • On a site that has a post with no views/comments/likes, go to Insights > Latest Post Summary.
  • Verify the view is still: (the point being there should be no change here)

no_data


  • On a site with a more than 99,999 views, go to Insights > Latest Post Summary.
  • Verify the number is abbreviated.
    • Note: to test this, in LatestPostSummaryCell:configure, you can modify the value used in this line:
      viewsDataLabel.text = lastPostInsight.viewsCount.abbreviatedString(forHeroNumber: true)

abbr

@ScoutHarris ScoutHarris added this to the 12.1 milestone Mar 15, 2019
@ScoutHarris ScoutHarris self-assigned this Mar 15, 2019
@ScoutHarris ScoutHarris requested a review from jklausa March 15, 2019 20:19
@ScoutHarris
Copy link
Contributor Author

@SylvesterWilmott -


ipad_port


ipad_land

@ScoutHarris ScoutHarris mentioned this pull request Mar 15, 2019
45 tasks
Copy link
Contributor

@jklausa jklausa left a comment

Choose a reason for hiding this comment

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

One nitpick/question, other than that 👍

@@ -168,7 +168,7 @@ private extension OverviewCell {
])
}

private enum ChartBottomMargin {
enum ChartBottomMargin {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm blind (and my cmd-f has broken) nothing else in this PR references ChartBottomMargin — why the change 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.

Oh, sorry, I failed to commit this separately. I just noticed this private was unnecessary since this enum is in a private extension, so I removed it.

@ScoutHarris ScoutHarris merged commit 7ce8785 into develop Mar 18, 2019
@ScoutHarris ScoutHarris deleted the feature/11284-lps_update_design branch March 18, 2019 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants