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

Contact Form Gutenblock #10256

Closed
wants to merge 142 commits into from
Closed

Contact Form Gutenblock #10256

wants to merge 142 commits into from

Conversation

georgestephanis
Copy link
Member

@georgestephanis georgestephanis commented Oct 2, 2018

GM2018 Work In Progress.

Do not merge!

This is built using the https://github.com/Automattic/wp-calypso/ build tools -- something like this:

#!/bin/bash

CALYPSO_DIR="/Users/georgestephanis/code/wp-calypso/"
JETPACK_DIR="/Users/georgestephanis/vagrant-local/www/wordpress-develop/public_html/build/wp-content/plugins/jetpack/"

cd $CALYPSO_DIR

cp -rf ${JETPACK_DIR}modules/contact-form/block/* ${CALYPSO_DIR}client/gutenberg/extensions/contact-form/
npm run sdk -- gutenberg ${CALYPSO_DIR}client/gutenberg/extensions/contact-form/
cp -rf ${CALYPSO_DIR}client/gutenberg/extensions/contact-form/build/* ${JETPACK_DIR}modules/contact-form/block/build/

but for testing purposes, the compiled js file is bundled in the pr.

TBD

  • Make the save button text editable.
  • Rendering on the front-end (waiting on externals -- @dmsnell )
  • Deletion of multi fields
  • More transforms
  • ???

Testing instructions

Clone the branch, and add a contact form block in Gutenberg.

This is also mirrored to Calypso in Automattic/wp-calypso#27688

Proposed changelog entry

Add a Gutenberg Block for the Jetpack Contact Form

@matticbot
Copy link
Contributor

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

@jetpackbot
Copy link

jetpackbot commented Oct 2, 2018

Thank you for the great PR description!

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

Scheduled Jetpack release: November 6, 2018.
Scheduled code freeze: October 30, 2018

Generated by 🚫 dangerJS

@jeherve jeherve added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Oct 2, 2018
@kraftbj kraftbj added the DO NOT MERGE don't merge it! label Oct 2, 2018
@mapk
Copy link

mapk commented Oct 24, 2018

Inner Block Picker

  1. Only show the "Contact Form" category with all 11 inner blocks (form fields).
  2. Order the inner blocks most likely to be used with logical groupings (ie. text fields, checkboxes, radio/select, etc.)
  3. Let's update some of these icons as well with Material's icons.

Example of Inner Block Picker (ordered alphabetically)

picker-inner-block

Icons
Checkbox = https://material.io/tools/icons/?icon=check_box&style=outline
Checkbox Multiple = https://material.io/tools/icons/?icon=ballot&style=outline
Date = https://material.io/tools/icons/?icon=today&style=outline
Email = https://material.io/tools/icons/?icon=email&style=outline
Name = https://material.io/tools/icons/?icon=person&style=outline
Radio = https://material.io/tools/icons/?icon=radio_button_checked&style=outline
Select = https://material.io/tools/icons/?icon=calendar_view_day&style=outline
Telephone = https://material.io/tools/icons/?icon=phone&style=outline
Text = https://material.io/tools/icons/?icon=short_text&style=outline
Textarea = https://material.io/tools/icons/?icon=notes&style=outline
URL = https://material.io/tools/icons/?icon=computer&style=outline

@jasmussen any thoughts on the ordering of inner blocks in the picker?
@MichaelArestad @georgestephanis Sound good?

@jasmussen
Copy link
Member

any thoughts on the ordering of inner blocks in the picker?

I think it's alphabetical so I'm not sure there's much you can do.

@georgestephanis
Copy link
Member Author

Icons are swapped over to the specified material design ones, but I'm not sure how I can hide the other block selector tabs --

screen shot 2018-10-25 at 3 18 32 pm

@jeherve jeherve modified the milestones: 6.7, 6.8 Oct 26, 2018
@georgestephanis
Copy link
Member Author

If we can get WordPress/gutenberg#11082 or the like merged to Gutenberg, this should be good to go.

enejb pushed a commit to Automattic/wp-calypso that referenced this pull request Nov 6, 2018
dmsnell added a commit to WordPress/gutenberg that referenced this pull request Nov 7, 2018
Attempt three at including positional information from the parse to enable isomorphic reconstruction of the source `post_content` after parsing.

See alternate attempts: #11082, #11309
Motivated by: #7247, #8760, Automattic/jetpack#10256
Enables: #10463, #10108

## Abstract

Add new `innerContent` property to each block in parser output indicating where in the innerHTML each innerBlock was found.

## Status

 - will update fixtures after design review indicates this is the desired approach
 - all parsers passing new tests for fragment behavior

## Summary

Inner blocks, or nested blocks, or blocks-within-blocks, can exist in Gutenberg posts. They are serialized in `post_content` in place as normal blocks which exist in between another block's comment delimiters.

```html
<!-- wp:outerBlock -->
Check out my
<!-- wp:voidInnerBlock /-->
and my other
<!-- wp:innerBlock -->
with its own content.
<!-- /wp:innerBlock -->
<!-- /wp:outerBlock -->
```

The way this gets parsed leaves us in a quandary: we cannot reconstruct the original `post_content` after parsing because we lose the origin location information for each inner block since they are only passed as an array of inner blocks.

```json
{
	"blockName": "core/outerBlock",
	"attrs": {},
	"innerBlocks": [
		{
			"blockName": "core/voidInnerBlock",
			"attrs": {},
			"innerBlocks": [],
			"innerHTML": ""
		},
		{
			"blockName": "core/innerBlock",
			"attrs": {},
			"innerBlocks": [],
			"innerHTML": "\nwith its own content.\n"
		}
	],
	"innerHTML": "\nCheck out my\n\nand my other\n\n"
}
```

At this point we have parsed the blocks and prepared them for attaching into the JavaScript block code that interprets them but we have lost our reverse transformation.

In this PR I'd like to introduce a new mechanism which shouldn't break existing functionality but which will enable us to go back and forth isomorphically between the `post_content` and first stage of parsing. If we can tear apart a Gutenberg post and reassemble then it will let us to structurally-informed processing of the posts without needing to be aware of all the block JavaScript.

The proposed mechanism is a new property as a **list of HTML fragments with `null` values interspersed between those fragments where the blocks were found**.

```json
{
	"blockName": "core/outerBlock",
	"attrs": {},
	"innerBlocks": [
		{
			"blockName": "core/voidInnerBlock",
			"attrs": {},
			"innerBlocks": [],
			"blockMarkers": [],
			"innerHTML": ""
		},
		{
			"blockName": "core/innerBlock",
			"attrs": {},
			"innerBlocks": [],
			"blockMarkers": [],
			"innerHTML": "\nwith its own content.\n"
		}
	],
	"innerHTML": "\nCheck out my\n\nand my other\n\n",
	"innerContent": [ "\nCheck out my\n", null, "\n and my other\n", null, "\n" ],
}
```

Doing this allows us to replace those `null` values with their associated block (sequentially) from `innerBlocks`.

## Questions

 - Why not use a string token instead of an array?
    - See #11309. The fundamental problem with the token is that it could be valid content input from a person and so there's a probability that we would fail to split the content accurately.

 - Why add the `null` instead of leaving basic array splits like `[ 'before', 'after' ]`?
    - By inspection we can see that without an explicit marker we don't know if the block came before or after or between array elements. We could add empty strings `''` and say that blocks exist only _between_ array elements but the parser code would have to be more complicated to make sure we appropriately add those empty strings. The empty strings are a bit odd anyway.

 - Why add a new property?
    - Code already depends on `innerHTML` and `innerBlocks`; I don't want to break any existing behaviors and adding is less risky than changing.
@catehstn
Copy link

catehstn commented Nov 9, 2018

@georgestephanis WordPress/gutenberg#11334 has now been merged - can we get this one in, now?

@lezama
Copy link
Contributor

lezama commented Nov 9, 2018

@catehstn @Automattic/poseidon is working on it, we should have something by the end of the day.

roccotripaldi pushed a commit to Automattic/wp-calypso that referenced this pull request Nov 9, 2018
enejb pushed a commit to Automattic/wp-calypso that referenced this pull request Nov 10, 2018
* Sync changes over from Automattic/jetpack#10256

* Update from Jetpack.

* Sync changes from Jetpack

* Import from Jetpack.

* Rename files and update them to use look more like the rest of calypso

* Add contact form to index.json so that we can test it out!

* Minor remove spaces

* Remove wp global comment

* Minor Formatting code

* Update i18n to use the jetpack i18n

* Refactor jetpack fieldf label

This was done for the field form to be simpler
as well as address
- improved padding and remove borders of the label

* Added innerblock descriptions

* Contact form: updated descriptions of blocks

* Contact form: update text for contact form block

* Update the component to be functions instead

* Add editField and editMultifield functions that make thing more DIY

* Minor fix: align the checkbox

* Add the toggle require as inline element

* Minor css fixes that collapse the spacing around the require toggle

* Contact form: fixed required jump

* Form Component: Add template back

* Minor regression fix for the mutileselect

* Contact form: updated styles of the multiple field add option button

* Make the fieldDefaults overwritable

* Update the components categories to be jetpack from common

* Contact form: Refactored styles

* now uses class names - easier to parse
* Prettied up a few buttons
* more coming soon - stay tuned!

* Contact form: refactored jetpackMultiple styles to be hardened against change
* also looks nicer

* Remove tranforms for now.

* Refactor icons  so that they are more DIY

* Contact form: rewrote checkbox - single - and styled it nice and purty

* Contact form: updated labels

* Contact form - made buttons disappear when block is not selected

* Update the multi select so that it works better.

- Improved the onKeyPress focus
- Adds enter and backspace keyboard events to work as expected

* Update the x to be a trash can on multi select

* Update te category to be jecpack instead of widgets

* Contact form: remove inspector settings for most fields

* Contact form: add transforms back in

* Contact form: Updated labels
* Checkbox Multiple is now Checkbox group
* Updated prefilled label copy to be more intuitive

* Add default first field on new multi fields

* Add the TextControl into the form for email and subject

* Contact form: updated copy for Select label

* Add initial settings form to the contact form

User is asked to fill out the email and subjet before creating the form.

* Udpate to use contact-form block from form

* Update classes to use contact-form

Also add has-intro class

* Contact form: styled intro

* Update the parent of child block to contact-form

* Update te editor.js to fix merge

* Contact form: updated intro styles and copy

* Contact form: fixed CSS regression

* Add email keyboard for contact form block

* Fix naming of the classes

* Renamed Jetpack Form to Jetpack Contact Form component

* Contact form: updated inspector copy and layout for contact form block itself

* Add a conditional 'is-selected' class to jetpack field

* Contact form: updated some classes and tidied up code

* Remove the jetpack textdomain

* destructuring props

* destructuring props

* deconstruct this.props in jetpack-contact-form.jsx

* extract info messages into a reusable message.

* destructuring all of the things

* deconstruct this.props on required toggle component

* destructuring props

* deconstruct this.props in jetpack option component

* destructuring props

* Reset the npm shrinkwrap

* Revert "Reset the npm shrinkwrap"

This reverts commit 379d08432e2d9cdd743efee5709b9e8b538f102c.

* revert shrinkwrap

* revert removal of simple payments

* Remove the checkbox label blank so that the user fill it out.

* ES6fying
@lezama
Copy link
Contributor

lezama commented Nov 12, 2018

This has been merged in Automattic/wp-calypso#27996.

Thanks @georgestephanis !

@lezama lezama closed this Nov 12, 2018
@lezama lezama deleted the gm18/grunionblock branch November 12, 2018 12:55
@lezama
Copy link
Contributor

lezama commented Nov 12, 2018

In order to test the new Contact Form Block please follow the instructions on #10537

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE don't merge it! [Feature] Contact Form [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.