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

Add promote posts button to stats entries, with feature flag #66024

Merged
merged 5 commits into from
Aug 2, 2022

Conversation

j6ll
Copy link
Contributor

@j6ll j6ll commented Jul 27, 2022

Proposed Changes

Hi Team, this is my first calypso PR and part of a series to add entrypoints in calypso for a new post promotion widget. Which will allow wordpress.com users to pay for advertising and serve the posts as ads on various A8C products.

Info specifically on these tasks here (pbpazC-3cN-p2) and the wider project thread here (pbpazC-2Pd-p2)

The majority of this feature is done outside of WP.com as a standalone widget, here we only need to add buttons to Calypso to show the widget and to pass it the site & post id's.

Note: We will be iterating on a bunch of PRs to complete this feature, but to keep these smaller we will be merging one-by-one under a feature flag.

Here are our provided designs:

Screenshot 2022-07-28 at 11 05 39

And here is the screenshot in my development environment:

Testing Instructions

  • Enable the feature flag promote-posts either on startup or via url using the ?flags=promote-post query string on the following urls
  • Visit the stats page for your favourite blog for example: http://calypso.localhost:3000/stats/day/something.wordpress.com
  • In the posts and pages section, hover over a post and you should see a "speaker" icon like in the designs abover
    Screenshot 2022-07-28 at 11 20 45
  • clicking the button should open the widget. However, this is not in production yet so without setting up your environment you can check the network tab and notice that it's trying to hit the script widget.js - this will repeatedly fail. In a working dev environment - it only ever loads the script once.
  • Test the above Steps without the feature flag enabled, ensure widget.js is not loaded in the f12 -> network tab and that the speaker button does not appear on hover.

Without local instance of the widget running
Screenshot 2022-07-28 at 11 29 46

With local instance of the widget running
Screenshot 2022-07-28 at 11 33 47

Pre-merge Checklist

Related to #

@j6ll j6ll requested a review from a team as a code owner July 27, 2022 18:29
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 27, 2022
@j6ll j6ll added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jul 27, 2022
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 27, 2022
@j6ll j6ll removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 27, 2022
@j6ll j6ll changed the base branch from trunk to add/promote-posts July 27, 2022 18:30
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 27, 2022
@matticbot
Copy link
Contributor

matticbot commented Jul 27, 2022

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~354 bytes added 📈 [gzipped])

name   parsed_size           gzip_size
stats      +2001 B  (+0.3%)     +565 B  (+0.3%)
home        +169 B  (+0.0%)      +82 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@j6ll j6ll force-pushed the add/promote-posts-stats branch 4 times, most recently from 1d44c0b to bc19b17 Compare July 29, 2022 12:52
Base automatically changed from add/promote-posts to trunk July 31, 2022 22:00
@j6ll j6ll requested a review from gravityrail August 1, 2022 10:24
Copy link
Contributor

@benzhu56 benzhu56 left a comment

Choose a reason for hiding this comment

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

Works great and code LG.
I happened to notice one thing, that a page would also show up in the list, however, it would have an id of 0. Not sure if it has an actual id and are we able to get the actual id.
E.g in following example, the top page has an id of 0, (I printed it out in console).
image

@j6ll
Copy link
Contributor Author

j6ll commented Aug 2, 2022

I happened to notice one thing, that a page would also show up in the list, however, it would have an id of 0.

Thanks @benzhu56 spotted this too. My thoughts were that we should still send show the user the promote button - but send them to a blank widget (not pre-populated). Then we need the widget just to handle 0 or invalid post Id's properly - not show it as a validation error on the creative screen..

I created a follow on ticket for that here: (350-gh-tumblr/wordads-picard). The other part of this longer term would be the post picker in the widget - I think this is maybe being discussed but not ticketed yet.

benzhu56 and others added 4 commits August 2, 2022 10:33
* Add promote post to my home swipeable

* Use wpcom-proxy for requests

Co-authored-by: Ben Zhu <ben.zhu@a8c.com>
The button appears after the "external" link icon - these both appear on hovering the post title
Copy link
Contributor

@gravityrail gravityrail left a comment

Choose a reason for hiding this comment

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

This LGTM

@@ -572,6 +572,7 @@ describe( 'utils', () => {
)
).toEqual( [
{
id: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a heads up that the home page post ID isn't always going to be 0 (you can switch any post with a post_type of page to be the home page too, and these will all have an ID)

Also there are other URLs, like taxonomy pages or the blog page, that also might not have IDs.

@gravityrail gravityrail changed the title Add/promote posts stats Add promote posts button to stats entries, with feature flag Aug 2, 2022
@gravityrail
Copy link
Contributor

I have tested this and, while it's clearly part of a work-in-progress, it doesn't break anything and is required to unblock further work so I'm going to merge it.

@gravityrail gravityrail merged commit f1a0467 into trunk Aug 2, 2022
@gravityrail gravityrail deleted the add/promote-posts-stats branch August 2, 2022 20:25
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 2, 2022
@a8ci18n
Copy link

a8ci18n commented Aug 2, 2022

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7462764

Thank you @jamesgill3 for including a screenshot in the description! This is really helpful for our translators.

@a8ci18n
Copy link

a8ci18n commented Aug 12, 2022

Translation for this Pull Request has now been finished.

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.

None yet

5 participants