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 Send Email Preview feature #31021

Merged
merged 10 commits into from
Jun 21, 2023
Merged

Add Send Email Preview feature #31021

merged 10 commits into from
Jun 21, 2023

Conversation

lezama
Copy link
Contributor

@lezama lezama commented May 28, 2023

Part of Automattic/wp-calypso#76134

Proposed changes:

  • Adds a "Preview" button to the Pre Publish Newsletter Panel.
  • The "Preview" button opens a "Send a test email" modal showing the current user email and a "Send" button.
Screenshot 2023-06-20 at 15 46 49

Future iteration will use WordPress/gutenberg#50605

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

peKye1-6z-p2

Does this pull request change what data or activity we track or use?

No

Testing instructions:

  • Build/Run this branch on your test site.
  • Create a new post on your self-hosted site.
  • Click Publish, in Pre Publish, click "Preview" at the bottom of the Newsletter panel.
  • A "Send a test email" modal should open.
  • Click on the "Send" button.
  • You should receive a test email!

@lezama lezama requested a review from a team May 28, 2023 16:01
@github-actions
Copy link
Contributor

github-actions bot commented May 28, 2023

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack plugin. Once the plugin is active, go to Jetpack > Jetpack Beta > Jetpack and enable the add/preview-email-v2 branch.

  • To test on Simple, run the following command on your sandbox:

    bin/jetpack-downloader test jetpack add/preview-email-v2
    

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions bot added [Block] Subscriptions [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ labels May 28, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 28, 2023

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ⚠️ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.


Jetpack plugin:

  • Next scheduled release: July 5, 2023.
  • Scheduled code freeze: June 26, 2023.

@github-actions github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label May 28, 2023
@lezama lezama mentioned this pull request May 29, 2023
3 tasks
@lezama lezama force-pushed the add/preview-email-v2 branch 2 times, most recently from f129afb to d6a8b72 Compare May 30, 2023 00:38
@lezama lezama changed the base branch from trunk to add/preview-email-endpoint May 30, 2023 00:39
@lezama lezama removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label May 30, 2023
@github-actions github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label May 30, 2023
@lezama lezama added [Status] In Progress and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels May 30, 2023
@lezama lezama force-pushed the add/preview-email-endpoint branch from 3a57e43 to 7faf279 Compare June 1, 2023 12:15
@github-actions github-actions bot added [JS Package] Boost Score Api Admin Page React-powered dashboard under the Jetpack menu RNA labels Jun 1, 2023
@lezama lezama force-pushed the add/preview-email-endpoint branch 2 times, most recently from 4cb108b to 2f7c719 Compare June 1, 2023 17:00
@lezama lezama force-pushed the add/preview-email-v2 branch 2 times, most recently from 9947dc5 to c920115 Compare June 1, 2023 17:31
@github-actions github-actions bot added the Docs label Jun 1, 2023
@lezama lezama force-pushed the add/preview-email-endpoint branch from 57a1a34 to daf75d3 Compare June 1, 2023 19:52
Base automatically changed from add/preview-email-endpoint to trunk June 1, 2023 20:05
@lezama lezama closed this in #31028 Jun 1, 2023
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. and removed Docs [Status] Needs Review To request a review from Crew. Label will be renamed soon. RNA labels Jun 21, 2023
Copy link
Contributor

@edanzer edanzer left a comment

Choose a reason for hiding this comment

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

This is testing as expected, and I'm signing off on that basis. Code changes look right too, but I can see that Jetpack crew already signed off on the code, and they know Jetpack much better than me.

Note: I did have an issue where /send-email-preview/ endpoint did not work initially. I had to reset permalinks to get it working. But I don't that issue is specific to this PR.

@lezama lezama merged commit 29a0617 into trunk Jun 21, 2023
@lezama lezama deleted the add/preview-email-v2 branch June 21, 2023 22:44
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jun 21, 2023
@github-actions github-actions bot added this to the jetpack/12.3 milestone Jun 21, 2023
@lezama lezama mentioned this pull request Jun 21, 2023
3 tasks
@samiff samiff added [Status] Needs Testing We need to add this change to the testing call for this month's release [Status] UI Changes Add this to PRs that change the UI so documentation can be updated. and removed [Status] Needs Testing We need to add this change to the testing call for this month's release labels Jun 26, 2023
@crisbusquets
Copy link
Contributor

crisbusquets commented Jun 30, 2023

Moved the button to the Newsletter PrePublish panel.

Screenshot 2023-06-20 at 15 38 16

Hi @lezama !
Can we change it to a link that provides more context? For example:

Screenshot 2023-06-30 at 18 15 06

I wouldn't use a button, the information architecture feels weird —the button component in the pre-publish panel seems to only be used for the two main actions (Publish or Cancel).

What do you think?

@lezama
Copy link
Contributor Author

lezama commented Jul 3, 2023

I copied the button from the Social Preview panel above so they had similar patterns.

Screenshot 2023-07-03 at 11 38 20

Happy to do whatever, maybe change both 😆

@crisbusquets
Copy link
Contributor

I copied the button from the Social Preview panel above so they had similar patterns.

Screenshot 2023-07-03 at 11 38 20 Happy to do whatever, maybe change both 😆

Oh! I wasn't aware this one existed 🥲

Do you know who worked on that? I prefer to use links instead of buttons, but I'd like to know the reason behind the Social Previews design.

Also, are we able to change or add additional text to the Newsletter accordion?

@lezama
Copy link
Contributor Author

lezama commented Jul 3, 2023

Do you know who worked on that? I prefer to use links instead of buttons, but I'd like to know the reason behind the Social Previews design.

Looks like @manzoorwanijk did the coding.

Also, are we able to change or add additional text to the Newsletter accordion?

yes, we can easily add something like the Social one has.

@crisbusquets
Copy link
Contributor

Do you know who worked on that? I prefer to use links instead of buttons, but I'd like to know the reason behind the Social Previews design.

Looks like @manzoorwanijk did the coding.

Also, are we able to change or add additional text to the Newsletter accordion?

yes, we can easily add something like the Social one has.

Cool, thanks!

I've reviewed it again, and I'm ok with using the button. A link would be less prominent, and I guess we want people to be aware they can preview the email they're going to send.

Can we add Preview what this newsletter will look like before sending it to your audience. on the top of the first radio button?

Screenshot 2023-07-04 at 09 24 22

Figma file (last frame on the right)

@lezama
Copy link
Contributor Author

lezama commented Jul 4, 2023

the buttons would change who receives/access the newsletter, maybe we should add two texts?

@crisbusquets
Copy link
Contributor

the buttons would change who receives/access the newsletter, maybe we should add two texts?

I don't understand this 😅 Do you mean the radio buttons?

@lezama
Copy link
Contributor Author

lezama commented Jul 4, 2023

it confuses me that the text is not next to the button, it makes me think that the options there are just for the Preview 🤷

@crisbusquets
Copy link
Contributor

it confuses me that the text is not next to the button, it makes me think that the options there are just for the Preview 🤷

Oh, ok. Now I get it 😬

Maybe the other way around is better?

Screenshot 2023-07-04 at 16 36 54

@lezama
Copy link
Contributor Author

lezama commented Jul 4, 2023

I like it better! 🤝

@lezama
Copy link
Contributor Author

lezama commented Jul 4, 2023

I guess we should avoid "your" here too 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Subscriptions [Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Status] UI Changes Add this to PRs that change the UI so documentation can be updated. [Tests] Includes Tests [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants