-
Notifications
You must be signed in to change notification settings - Fork 800
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
Earn: Move donations block to Jetpack Beta Blocks #16545
Conversation
Caution: This PR has changes that must be merged to WordPress.com |
80f624d
to
22ee00a
Compare
This is an automated check which relies on E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16545 |
3fda9f9
to
d57418d
Compare
@Automattic/serenity feel free to hop onto this branch, just let folks know if y'all are planning to rebase. |
Fired this up quickly to check regressions with the Payments block – just noting a few things for later sake, understanding it's plumbing that's the focus of this PR. Applying the branch through Jetpack Beta on an existing JN site caused fatals, but doing a clean JN install with the link in the instructions worked fine: The Donations block displays its "read more" link in line with the Upgrade button, rather than below it like the Payments button. Trying to use the Payments block on the front end, I had the same experience as @gwwar : the button would resolve to the post URL with a hash ( |
0959761
to
109dcf6
Compare
@mmtr I ended up needing to back out the membership registration refactors. Tracing the code we were getting to the correct register_jetpack_blocks calls, but the render_callback for the payment buttons did not trigger. Let's try this improvement out in smaller a follow up PR |
Alright @Automattic/serenity I think this one is good to test, since this is already rather large. I'll add a few follow up issues. Edit: Added: |
Thanks for reverting my changes! Main reason why I did refactor the registration of the Payments block is because I wasn't able to see the Donations block, but I think I was missing to activate the beta blocks because I can see it now (I wrongly assumed they are enabled by default on a dev environment). |
I ran through this with both the Donations and Payments blocks, on both Simple and JP sites, didn't see anything unexpected 🎉 (I didn't expect to see the Donations block in the inserter at all without a plan, and didn't expect any front-end output for Donations yet either.) |
Thanks for the extensive review @jeherve ! I can't accept my own PR, but I did verify that improvements added by @mmtr work well and don't regress the payments button in #16545 (comment). 💖 Was there any other feedback we should address in this PR? The team is planning to immediately follow up on the Button improvement and serialization options in the next PR. |
…ble, make sure block has correct classname
5e9ba6d
to
ee58ff9
Compare
It should help us avoid having to skip the wpcalypso/import-docblock rule like here: #16545 (comment) More info about the update: Automattic/wp-calypso#44388
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.
Rebased to fix unrelated test failures. This should now be good to merge. We can keep iterating on the block in future PRs.
It should help us avoid having to skip the wpcalypso/import-docblock rule like here: #16545 (comment) More info about the update: Automattic/wp-calypso#44388
Thanks for the review @jeherve! Do you know why merging is still blocked by the "Required review" check even after your approval? |
As discussed in p1HpG7-9Kg-p2#comment-40839, we're moving development of the Donation block to Jetpack from the FSE plugin. This block will allow customers on Jetpack and WordPress.com to accept one-time or recurring monthly/annual donations.
plan_check
property during block registration which makes the block non-insertable unless thejetpack_block_editor_enable_upgrade_nudge
filter is enabled (it's disabled by default on JP sites).This PR builds on top of work in:
What @gwwar and @mmtr changed from copied FSE files
jetpack/donations
full-site-editing
tojetpack
wp-block-jetpack-donations
define( 'JETPACK_BETA_BLOCKS', true )
is set on your WordPress instanceimport { __experimentalBlock as Block } from '@wordpress/block-editor';
What we're not including in this PR
We still need to pick final serialization options for the published block, but this can be done in a follow up PR. With Donations now in Jetpack it's not unreasonable to nest a child payments button, or instead opt to go with something a bit more custom but duplicate some of the iframe handling. Other pieces like the Edit view and backend glue pieces needed for the actual transactions are mostly done, but may need some follow up
Testing Instructions
Jetpack sites
Simple sites
Atomic Sites
add/donation-block-scaffolding
add_filter( 'jetpack_blocks_variation', function() { return 'beta'; } );
to activate beta blocks. A quick way of doing this is modifying a simple plugin with the plugin editor, like hello dolly.