-
Notifications
You must be signed in to change notification settings - Fork 815
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
Subscriptions: allow pre-selecting newsletter categories in block settings #41567
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 5 files.
|
This will need to be released after backend changes are in place. Leaving as draft for now but I would love a review to catch any obvious issues that will need to be addressed. |
52a99c9
to
b4c974d
Compare
b4c974d
to
97ca4c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WFM, nice job!
@holdercp I'm coming to this PR late, sorry, but I had a question about the user experience. Are you planning on updating the Jetpack settings in Jetpack > Settings > Newsletter to expand the Newsletter categories a bit? Right now, I don't think it's very clear that you can enable newsletter categories, and then have different groups of subscribers depending on the different subscribe blocks you set up on your site: ![]() I wonder if it would also be worth updating the subscription modal as well. Right now it's not very clear what you're subscribing to with the different options: ![]() Overall I think it's a great and useful new feature, we're essentially introducing a way to have multiple subscription lists on a single site, but I don't think we currently highlight that at all anywhere. |
@jeherve Those are great observations. I think the corresponding wpcom settings could be more clear as well: Regarding the modal, I believe the idea is that the topic/category selection is abstracted away from the visitor and has already been pre-selected by the author. For example, an author could configure the subscribe blocks on their posts related to "Cooking" to automatically subscribe visitors subscribing via those blocks to the "Cooking" category. There is a "Select which topics to subscribe to" screen in the modal flow, which will show up when newsletter categories are enabled and that is the default experience. Only when authors configure a subscribe block to automatically subscribe visitors to specific categories will that screen in the modal be skipped. @davemart-in We don't really have a good "link" to the subscribe block feature from these settings screens. It's not very clear that visitors will get to choose which categories they want to subscribe to (in the default case, through the modal screen) or that authors can now pre-select specific categories that visitors will be automatically subscribed to if they configure the block to do so. Maybe a copy update and a link to the subscribe block documentation might suffice here? What do you think? |
I like that idea @holdercp. What do you think of updating the test to read:
|
Sounds great. I'll create some tickets. Edit: ticket |
Closes Automattic/loop#378
New to block development and Jetpack so please review with a critical eye.
Works in concert with 172401-gh-Automattic/wpcom
Proposed changes:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
selected_newsletter_categories