Gutenpack: Add/contact form to the gutenberg editor #10537

enejb · 2018-11-05T14:32:17Z

Simplifies the work done by @georgestephanis in #10256

By removing the js code since it should be automatically be included as part of the jetpack blocks.
Now found here Automattic/wp-calypso#27996

Changes proposed in this Pull Request:

Todo:

[ ] Rendering on the front-end (waiting on externals) -
WordPress/gutenberg#11334

Testing instructions:

Go to https://jurassic.ninja/create/?gutenberg&gutenpack&shortlived&jetpack-beta&calypsobranch=add/gutenblocks/contact-form&branch=add/contact-form-gutenblock-sdk

Connect Jetpack
Visit the editor. Add the new contact form.
Does it work as expected?

Proposed changelog entry for your changes:

Add the contact form to the Gutenberg editor.

matticbot · 2018-11-05T14:32:24Z

Caution: This PR has changes that must be merged to WordPress.com
Please review this diff before merging: D20308-code. (newly created revision)

jetpackbot · 2018-11-05T14:34:13Z

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.

Scheduled Jetpack release: November 26, 2018.
Scheduled code freeze: November 19, 2018

Generated by 🚫 dangerJS

jeherve

Not sure if this is ready for review yet, but I had a small question.

jeherve · 2018-11-05T14:47:46Z

modules/contact-form/grunion-contact-form.php

@@ -232,6 +232,97 @@ function __construct() {
 		 */
 		wp_register_style( 'grunion.css', GRUNION_PLUGIN_URL . 'css/grunion.css', array(), JETPACK__VERSION );
 		wp_style_add_data( 'grunion.css', 'rtl', 'replace' );
+
+		if ( function_exists( 'register_block_type' ) ) {


Should we use Jetpack_Gutenberg::is_gutenberg_available here?

Yes good call. The plan is to convert things to jetpack_register_block calls which do that check as well.

matticbot · 2018-11-12T18:15:19Z

Caution: This PR has changes that must be merged to WordPress.com
Please review this diff before merging: D20308-code. (updated diff)

enejb · 2018-11-12T20:09:13Z

@georgestephanis Can you elaborate a bit more about the tests that you want to see?

georgestephanis · 2018-11-12T21:59:00Z

Given post_content that it renders properly, primarily.

enejb · 2018-11-13T23:59:48Z

Also Fixes Automattic/wp-calypso#28521

matticbot · 2018-11-14T22:32:17Z

Caution: This PR has changes that must be merged to WordPress.com
Please review this diff before merging: D20308-code. (updated diff)

georgestephanis · 2018-11-15T18:47:01Z

the failing tests seem to be coming from

Argument 1 passed to iterator_to_array() must implement interface Traversable, instance of DOMNodeList given, called in /tmp/wordpress-master/src/wp-content/plugins/jetpack/tests/php/modules/contact-form/test_class.grunion-contact-form.php on line 690 and defined

matticbot · 2018-11-15T20:13:45Z

D20923-code. (newly created revision)

This reverts commit 105fcca.

This reverts commit 0f1a632.

ensure we are exposing the contact form's sub-blocks in block availability data. the sub blocks will not be in blocks-manifest.json, so we'll need to ensure we are also including blocks that have been registered internally within jetpack

georgestephanis

Apart from some very minor stuff, very happy to clear this.

Thanks for the unit tests, refactoring how we're building the html into more manageable chunks, and the new assertions! It looks super cool!

georgestephanis · 2018-11-16T22:54:42Z

modules/contact-form/grunion-contact-form.php

-		wrapper.appendChild(menu);
-		<?php endif; ?>
-		menu.style.display = 'block';
+            var menu = document.getElementById( 'feedback-export' ),


This and subsequent lines probably shouldn't be indented with spaces?

georgestephanis · 2018-11-16T22:56:40Z

modules/contact-form/grunion-contact-form.php

@@ -2016,6 +2125,10 @@ static function get_compiled_form( $feedback_id, $form ) {
 		return $compiled_form;
 	}

+	static function remove_empty( $single_value ) {


Probably could use docblock to explain, but I assume this is here so that we can leave null false 0 and such that evaluate falsey in the array instead of stripping them out with the '' ?

georgestephanis · 2018-11-16T22:57:48Z

modules/contact-form/grunion-contact-form.php

+					}
+ 					if ( is_array( $val ) ) {
+						$val =  array_filter( $val, array( __CLASS__, 'remove_empty' ) );
+						$att_strs[] = esc_html( $att ) . '=\'' . implode( ',', array_map( 'esc_html', $val ) ) . '\'';


It's kinda awkward to do '\'' all the time -- any reason to not just do "'" to avoid the escaping?

* Add the gutenberg contact form gutenberg blocks * Gutenpack: Add contact form block * Update form block to use contact-form * Add do_blocks call to the contact render form * Add register_contact_form_blocks block method * Provide default labels * Updates to fix tests the default value of a lable should be null of the type is not set * Minor fixes * Use the jetpack_register_block call to register the contact form blocks Make sure we load them before we load the gutenberg blocks * Bug fix: Only display selections when the option has a value. * Added tests that show what the form doesn't recieve a lavel that we defaul to the default label. * Add tests that shows that we remove empty options when we pass an string containing the empty options * Minor bug fix. Add closing div to the phone field. * Add support for class added in gutenberg * Refactored field Render method so that it is easier to follow. More DRY field render method * Minor fixes * Add unitests for output. * Unit Test fixes so that they work with older version of PHP * Fix: Add more liberal url validaton This removed the validation error when a user doesn't add a http:// or https:// infront of the url. * Improved pattern for the url. * Revert "Improved pattern for the url." This reverts commit 105fcca. * Revert "Fix: Add more liberal url validaton" This reverts commit 0f1a632. * Add support for the defaultValue block attribute * Gutenberg: Conact form ensure we are exposing the contact form's sub-blocks in block availability data. the sub blocks will not be in blocks-manifest.json, so we'll need to ensure we are also including blocks that have been registered internally within jetpack * Fix the tests again :P * Fixes indentation of the js code * Remove comment * Add php doc block for remove_empty * Fixes ugly quotes. * Fix the tests since we are expecting the double quotes now. * With escaped quotes

jeherve · 2018-11-20T10:48:00Z

Cherry-picked to branch-6.8 in ea44f76

ockham · 2018-11-23T17:37:47Z

It looks like the Fusion-generated counterpart patch (D20308-code) was never merged to WP.com, which is now causing trouble for a PR of mine that seeks to modify class.jetpack-gutenberg.php. Could someone who authored, reviewed, or merged this PR please take care of that? 😁

ockham · 2018-11-23T17:40:42Z

Oh, looks like D20308-code doesn't contain the changes that this PR made to class.jetpack-gutenberg.php (possibly because at the time it was merged, that file wasn't on Fusion's list yet). Maybe I can manually re-trigger patch creation...

ockham · 2018-11-23T19:32:27Z

Fixed, thanks to @enejb's D21287-code.

enejb requested a review from a team November 5, 2018 14:32

matticbot added the Touches WP.com Files label Nov 5, 2018

jeherve added [Feature] Contact Form [Status] In Progress [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Nov 5, 2018

jeherve added this to the 6.8 milestone Nov 5, 2018

jeherve reviewed Nov 5, 2018

View reviewed changes

jeherve assigned enejb Nov 9, 2018

jeherve mentioned this pull request Nov 9, 2018

Gutenberg: add blocks to Jetpack #10326

Closed

22 tasks

lezama previously approved these changes Nov 10, 2018

View reviewed changes

lezama added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Nov 12, 2018

jeherve modified the milestones: 6.8, 6.7.1 Nov 12, 2018

enejb dismissed lezama’s stale review via aca0004 November 12, 2018 17:07

enejb force-pushed the add/contact-form-gutenblock-sdk branch from aca0004 to 2bd59a1 Compare November 12, 2018 18:15

lezama requested a review from a team November 12, 2018 19:42

lezama mentioned this pull request Nov 12, 2018

Contact Form Gutenblock #10256

Closed

5 tasks

georgestephanis added the [Tests] Needs Tests Some Unit Tests would be really useful to include with this PR. label Nov 12, 2018

enejb modified the milestones: 6.7.1, 6.8 Nov 12, 2018

enejb and others added 4 commits November 15, 2018 22:04

Revert "Improved pattern for the url."

296df81

This reverts commit 105fcca.

Revert "Fix: Add more liberal url validaton"

d2650ce

This reverts commit 0f1a632.

Add support for the defaultValue block attribute

efc76e6

Gutenberg: Conact form

30deca4

ensure we are exposing the contact form's sub-blocks in block availability data. the sub blocks will not be in blocks-manifest.json, so we'll need to ensure we are also including blocks that have been registered internally within jetpack

roccotripaldi force-pushed the add/contact-form-gutenblock-sdk branch from 93c213f to 30deca4 Compare November 16, 2018 03:40

enejb removed the [Tests] Needs Tests Some Unit Tests would be really useful to include with this PR. label Nov 16, 2018

Fix the tests again :P

dd89d1b

lezama previously approved these changes Nov 16, 2018

View reviewed changes

georgestephanis reviewed Nov 16, 2018

View reviewed changes

georgestephanis previously approved these changes Nov 16, 2018

View reviewed changes

enejb added 4 commits November 16, 2018 15:25

Fixes indentation of the js code

1705d28

Remove comment

ebd7d0a

Add php doc block for remove_empty

d221495

Fixes ugly quotes.

53876cf

enejb dismissed stale reviews from georgestephanis and lezama via 53876cf November 16, 2018 23:26

enejb added 2 commits November 16, 2018 15:45

Fix the tests since we are expecting the double quotes now.

dca6d2a

With escaped quotes

c903b63

georgestephanis approved these changes Nov 19, 2018

View reviewed changes

georgestephanis merged commit ccda7b6 into master Nov 19, 2018

matticbot added the [Status] Needs Changelog label Nov 19, 2018

enejb deleted the add/contact-form-gutenblock-sdk branch November 19, 2018 15:42

jeherve removed [Status] Needs Changelog [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Nov 20, 2018

ockham mentioned this pull request Nov 23, 2018

Gutenberg: Use Blocks from staging dir if proxied #10713

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gutenpack: Add/contact form to the gutenberg editor #10537

Gutenpack: Add/contact form to the gutenberg editor #10537

enejb commented Nov 5, 2018 •

edited by jeherve

Loading

matticbot commented Nov 5, 2018

jetpackbot commented Nov 5, 2018 •

edited

Loading

jeherve left a comment

jeherve Nov 5, 2018

enejb Nov 6, 2018

matticbot commented Nov 12, 2018

enejb commented Nov 12, 2018

georgestephanis commented Nov 12, 2018

enejb commented Nov 13, 2018

matticbot commented Nov 14, 2018

georgestephanis commented Nov 15, 2018

matticbot commented Nov 15, 2018

georgestephanis left a comment

georgestephanis Nov 16, 2018

georgestephanis Nov 16, 2018

georgestephanis Nov 16, 2018

jeherve commented Nov 20, 2018

ockham commented Nov 23, 2018

ockham commented Nov 23, 2018

ockham commented Nov 23, 2018

Gutenpack: Add/contact form to the gutenberg editor #10537

Gutenpack: Add/contact form to the gutenberg editor #10537

Conversation

enejb commented Nov 5, 2018 • edited by jeherve Loading

Changes proposed in this Pull Request:

Todo:

Testing instructions:

Proposed changelog entry for your changes:

matticbot commented Nov 5, 2018

jetpackbot commented Nov 5, 2018 • edited Loading

jeherve left a comment

Choose a reason for hiding this comment

jeherve Nov 5, 2018

Choose a reason for hiding this comment

enejb Nov 6, 2018

Choose a reason for hiding this comment

matticbot commented Nov 12, 2018

enejb commented Nov 12, 2018

georgestephanis commented Nov 12, 2018

enejb commented Nov 13, 2018

matticbot commented Nov 14, 2018

georgestephanis commented Nov 15, 2018

matticbot commented Nov 15, 2018

georgestephanis left a comment

Choose a reason for hiding this comment

georgestephanis Nov 16, 2018

Choose a reason for hiding this comment

georgestephanis Nov 16, 2018

Choose a reason for hiding this comment

georgestephanis Nov 16, 2018

Choose a reason for hiding this comment

jeherve commented Nov 20, 2018

ockham commented Nov 23, 2018

ockham commented Nov 23, 2018

ockham commented Nov 23, 2018

enejb commented Nov 5, 2018 •

edited by jeherve

Loading

jetpackbot commented Nov 5, 2018 •

edited

Loading