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

Jetpack: Subscriptions - Decouple shortcode from Block rendering #26866

Closed
oskosk opened this issue Oct 17, 2022 · 3 comments · Fixed by #26947 or #27519
Closed

Jetpack: Subscriptions - Decouple shortcode from Block rendering #26866

oskosk opened this issue Oct 17, 2022 · 3 comments · Fixed by #26947 or #27519
Assignees
Labels
[Block] Contact Form Form block (also see Contact Form label) [Block] Subscriptions [Feature] Contact Form [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 [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Comments

@oskosk
Copy link
Contributor

oskosk commented Oct 17, 2022

The Subscribe block currently saves a shortcode block in the post HTML, and the relies on the shortcode frontend code for rendering it.

This has become inconvenient, and we should detach the block from the shortcode.

Image

@oskosk oskosk added [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. [Block] Subscriptions labels Oct 17, 2022
@oskosk oskosk changed the title Jetpack: Contact Form - Decouple Contact form shortcode from Block rendering Jetpack: Contact Form - Decouple shortcode from Block rendering Oct 17, 2022
@oskosk oskosk changed the title Jetpack: Contact Form - Decouple shortcode from Block rendering Jetpack: Subscriptions - Decouple shortcode from Block rendering Oct 17, 2022
@ice9js ice9js self-assigned this Oct 18, 2022
@ice9js
Copy link
Member

ice9js commented Oct 18, 2022

I just want to make sure we're on the same page before I start updating the code:

The submission logic looks like it can be left as is, as it really only depends on the right request being sent for it to trigger.

As for the block itself, I see two ways to go about it, in order to be able to display the success message:

  • Make the block fully dynamic, removing save() altogether and doing all the rendering in render_block()
  • Make save() return the form part of the block and treat it as a static block. In case of a submission just suplement it with an appropriate success/error message above/instead of the form.

I think the second approach makes sense as we're not breaking up the form which could cause some issues with proper error message placement. Either way we're not doing that yet so even more reason to start with the static form/dynamic message approach.
Actually, having gone through all the block options thoroughly now, having a fully dynamic block makes the most sense to me, rather than attempting to inject additional HTML into it.

@oskosk
Copy link
Contributor Author

oskosk commented Nov 18, 2022

Reopening. @ice9js this doesn't seem to have been addressed by #26947

I still see the block generating shortcode in the editor

To reproduce:

  • Launch a JN site on trunk
  • Connect Jetpack
  • enable subscriptions from Jetpack -> Settings -> Discussion (at the bottom)
  • Create a post
  • Add a subscription block
  • Switch to code editor view

@ice9js
Copy link
Member

ice9js commented Nov 21, 2022

As already mentioned in the conversation on the old PR, this was intentional to preserve compatibility during testing or in case anything goes wrong: the shortcode was saved but it wasn't actually used to generate the output when the post is rendered.

Now that the initial PR is merged, I opened up another one to completely remove the old save() implementation and prevent any future confusion regarding that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Contact Form Form block (also see Contact Form label) [Block] Subscriptions [Feature] Contact Form [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 [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
3 participants