Skip to content

Conversation

@jeffersonrabb
Copy link
Contributor

@jeffersonrabb jeffersonrabb commented Jul 8, 2019

The Contact Form block's date picker enqueues several scripts, which causes the block to break in AMP environments.

Changes proposed in this Pull Request:

Testing instructions:

  • Spin up a new Jurassic Ninja instance using this URL: https://jurassic.ninja/create/?jetpack-beta&branch=add/amp-contact-form
  • Connect Jetpack
  • Install and activate the AMP plugin: https://wordpress.org/plugins/amp/.
  • Go to the AMP settings page (/wp-admin/admin.php?page=amp-options) and switch mode to Transitional.
  • Create a new post and insert the Form block, and set it up to send to an email you receive.
  • Add a Date Picker form element to the form.
  • Publish or preview the post, and submit the form.
  • Switch to AMP version (either use admin nav to toggle or simply add ?amp to the end of the URL).
  • Submit the form.
  • Verify that form submission works correctly in AMP and non-AMP environments, and that date picker value is included in both.
  • Observe the date picker UI looks different in AMP version:

Screen Shot 2019-07-08 at 8 30 48 AM

Proposed changelog entry for your changes:

  • Contact Form block AMP compatibility.

@jetpackbot
Copy link
Collaborator

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 43ea931

@westonruter
Copy link
Contributor

Part of #9730. Will ✅ this:

image

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well for me, although there are some layout issues with the AMP date picker right now:

image

@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 Jul 18, 2019
@jeherve jeherve added this to the 7.6 milestone Jul 18, 2019
@jeherve jeherve merged commit 511df9e into master Jul 19, 2019
@jeherve jeherve deleted the add/amp-contact-form branch July 19, 2019 11:08
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jul 19, 2019
jeherve added a commit that referenced this pull request Jul 19, 2019
jeherve added a commit that referenced this pull request Jul 30, 2019
* Add initial changelog / testing list changes for 7.6

* Update stable tag to 7.5.3

* changelog: add #12957

* Changelog: add #12932

* Changelog: add #12867

* Changelog: add #12823

* changelog: add #12969

* changelog: add #13012

* changelog: add #12974

* Changelog: add #13059

* Changelog: add #13079

* Changelog: add #12924

* changelog: add #12954

* Changelog: add #12959

* Changelog: add #12977

* Changelog: add #12830

* Changelog: add #12926

* Changelog: add #12958

* Changelog: add #12999

* Changelog: add #13077

* Changelog: add #13083

* Changelog: add #13087

* Changelog: add #13110

* Changelog: add #13116

* Changelog: add #13117

* Changelog: add #12821

* Changelog: add #13120

* changelog: add #13139

* Changelog: add #13143

* Changelog: add #13147

* Testing list: add section about sync
jeherve added a commit that referenced this pull request Jul 30, 2019
* Add initial changelog / testing list changes for 7.6

* Update stable tag to 7.5.3

* changelog: add #12957

* Changelog: add #12932

* Changelog: add #12867

* Changelog: add #12823

* changelog: add #12969

* changelog: add #13012

* changelog: add #12974

* Changelog: add #13059

* Changelog: add #13079

* Changelog: add #12924

* changelog: add #12954

* Changelog: add #12959

* Changelog: add #12977

* Changelog: add #12830

* Changelog: add #12926

* Changelog: add #12958

* Changelog: add #12999

* Changelog: add #13077

* Changelog: add #13083

* Changelog: add #13087

* Changelog: add #13110

* Changelog: add #13116

* Changelog: add #13117

* Changelog: add #12821

* Changelog: add #13120

* changelog: add #13139

* Changelog: add #13143

* Changelog: add #13147

* Testing list: add section about sync
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