Skip to content

Conversation

@mmtr
Copy link
Member

@mmtr mmtr commented Aug 6, 2020

Changes proposed in this Pull Request:

Some sites such as any WP.com simple site might have a more restricted KSES logic that doesn't allow <button> elements in a post content.

To work around that, this PR modifies the output of the Donations block so all <button> elements are replaced with semantically equivalent <div role="button"> elements, which are allowed in WP.com. We still need to allow the data-* attributes in WP.com KSES, but that will be handled in separate WP.com patch. The contenteditable attribute has been also removed from the post content and it's set not dynamically with JS so it doesn't need to be allowed by KSES.

It also simplifies the general markup by removing all wp-block-buttons and wp-block-button classes, so we need to override less styles.

Jetpack product discussion

p1HpG7-9NV-p2

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

No.

Testing instructions:

  • On a WP.com sandbox, apply D47697-code and D47694-code (if D47694-code cannot be applied cleanly, it'll likely need a "rebase". See PCYsg-iiu-p2 #committing-block-changes-to-wordpress-com for instructions).
  • Sandbox a simple test site, the API and the store (PCYsg-lW4-p2).
  • Make sure your test site has a paid plan.
  • Go to wordpress.com/block-editor and select your test site.
  • Insert a donations block (you'd need to set up the connection to Stripe if it's not set yet).
  • Edit the block at your wish and publish the post.
  • Make sure the block is displayed correctly in the published post (so far you can only navigate through the tabs).

Proposed changelog entry for your changes:

N/A.

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello mmtr! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D47694-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link
Collaborator

jetpackbot commented Aug 6, 2020

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16743

Scheduled Jetpack release: September 1, 2020.
Scheduled code freeze: August 25, 2020

Generated by 🚫 dangerJS against 4ef0c99

@kwight
Copy link
Contributor

kwight commented Aug 6, 2020

Fantastic 🎉 Tried it with a variety of tab combinations and currencies, nothing unexpected (I did notice that on the front-end, the custom amount validation works well for USD, but not for other currencies).

kwight
kwight previously approved these changes Aug 6, 2020
Copy link
Contributor

@kwight kwight left a comment

Choose a reason for hiding this comment

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

One followup mentioned about front-end validation, but otherwise button extraction works great.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 10, 2020
@jeherve jeherve added this to the 8.9 milestone Aug 10, 2020
@mmtr mmtr merged commit 3204915 into master Aug 10, 2020
@mmtr mmtr deleted the update/donations-save-kses branch August 10, 2020 11:15
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants