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: Add post-submission options. #13745

Merged
merged 21 commits into from
Oct 18, 2019
Merged

Conversation

pento
Copy link
Contributor

@pento pento commented Oct 14, 2019

This PR adds post-submission options to the contact-form block.

Fixes #708, #12286.

Changes proposed in this Pull Request:

  • Adds a new InspectorControl panel to to the contact form block, which allows setting an optional message or custom redirect URL to be used after the contact form is submitted.
  • Adds support for these settings to the contact form submission handling.

NOTE: While this PR technically adds support for these options to the shortcode, this is just a side-effect of how the contact form works, rather than a deliberate decision, and should be considered unsupported. Support has not been added to the TinyMCE plugin.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Adds features to the Contact Form module. pb5gDS-7W-p2

Testing instructions:

  • Create a new post in the block editor, and add a Form block.
  • In the sidebar, go through the Confirmation Message options. Test that they behave as described.

Screenshots

Proposed changelog entry for your changes:

  • Contact Form Block: Allow setting a custom post-submission message or redirect URL for the form.

@pento pento added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Contact Form [Status] In Progress labels Oct 14, 2019
@pento pento requested a review from a team as a code owner October 14, 2019 04:01
@pento pento self-assigned this Oct 14, 2019
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello pento! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D33974-code before merging this PR. Thank you!

@jetpackbot
Copy link

jetpackbot commented Oct 14, 2019

Warnings
⚠️

pre-commit hook was skipped for one or more commits

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 f694283

@matticbot
Copy link
Contributor

pento, Your synced wpcom patch D33974-code has been updated.

@scruffian
Copy link
Member

I'm not sure a custom URL is the best UX here. I expect most users won't have a place for their users to go to when they find these settings, so they'll need to create one. For this reasons I would prefer we had a button to create a new page and a dropdown list of all their pages.

I think users are more likely to want to redirect to their own site than an external URL and if they change the slug of their page, we can make sure the redirect doesn't break.

@davemart-in
Copy link
Contributor

I'm not sure a custom URL is the best UX here. I expect most users won't have a place for their users to go to when they find these settings, so they'll need to create one. For this reasons I would prefer we had a button to create a new page and a dropdown list of all their pages.

I'm not against this. I think you're probably right. Some thoughts/questions that come to mind:

A) How much scope do we think this would add?
B) This is edge case, but what if someone already has 1000 pages/posts? Is there a performance concern if we load all 1000 into a single dropdown? Would navigating that big of a list be reasonable without some sort of search function built in to filter the list?
C) If we show a button to create a new page, assuming that happens in a new tab, how do we get them back to this tab, and get the dropdown list of posts/pages refreshed after they've created a new page?
D) I'm assuming that we'd need to show 3 options under "Message Type": 1) Text message, 2) Redirect to a Page/Post, 3) Redirect to an external address.

I'm going to cc @davipontesblog to see if he has any feedback on this.

@scruffian
Copy link
Member

scruffian commented Oct 14, 2019

If we show a button to create a new page, assuming that happens in a new tab

I was assuming this would happen in within Gutenberg in the same page...

@scruffian
Copy link
Member

This is edge case, but what if someone already has 1000 pages/posts? Is there a performance concern if we load all 1000 into a single dropdown?

The same kind of issues will come up in the Navigation block, so if it's a problem we can look at how they solve it; and if they haven't then we can invest the effort to solve it for them!

@davemart-in
Copy link
Contributor

I was assuming this would happen in within Gutenberg in the same page...

@pento, I pinged @scruffian on Slack and spoke with him about this just now. His proposal is that once a user clicks the "Make a thank you page for me" option that we would create the page behind the scenes and add some default thank you page content automatically (that they can modify later).

Here's a user flow for Ben's proposal:

JP Form - Thanks v1

Also, here's a quick video walkthrough of a rough mockup: https://cl.ly/01615a7c53e6

Thoughts on how much additional scope this will add in number of days?

If we feel good about this and the scope isn't too crazy, I'll flesh this out more and update the brief.

cc @huguesvincent

@matticbot
Copy link
Contributor

pento, Your synced wpcom patch D33974-code has been updated.

@pento
Copy link
Contributor Author

pento commented Oct 14, 2019

Thanks for the feedback, @davemart-in and @scruffian!

For the page redirect, I swapped the TextControl for a URLInput, which is what the block editor uses in the popover when you add a link to text. This allows searching existing posts/pages, or adding a custom URL.

I don't believe it's a viable option to use a dropdown. There have been substantial efforts towards doing a scalable dropdown in the block editor (for example, WordPress/gutenberg#16666), but there are significant hurdles to overcome.

I've had a look around, and I can't find any examples in the editor of a user flow that includes adding a new page, so it's a bit tricky to say how long it would take to build.

From a basic implementation perspective, I don't think there would be any major hurdles. It should (assuming no weird complications in this step) be just a REST API call to create the page.

From a UX perspective, I strongly suspect this will manifest as a ball of edge cases to deal with, though. Off the top of my head:

  • How do we allow the author to edit the new page, or at least point them to where they need to go to edit the new page? (Noting that "open in a new tab" will be a hack at best, since it won't work in the native apps.)
  • How do we deal with a page already existing that has the same title or slug? That'll cause confusion once they make it back to the pages list. Sites with multiple forms (each with their own thank you page) would be likely to run into this issue.
  • They may have a menu configured to automatically add new pages: should we override that setting when creating this page?
  • The author may have permissions to create a page, but not publish it. We could either not show them the button to create a page, or... I'm not sure how we should handle linking to a draft page here. 🙂

There are almost certainly more exciting cases that we'd encounter along the way.

Side note: Given the existence of #13468, do I need to worry about the codeclimate check failing? @kraftbj, are the codeclimate config changes likely to merge at some point?

@matticbot
Copy link
Contributor

pento, Your synced wpcom patch D33974-code has been updated.

@pento pento added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Oct 15, 2019
@davemart-in
Copy link
Contributor

I swapped the TextControl for a URLInput, which is what the block editor uses in the popover when you add a link to text. This allows searching existing posts/pages, or adding a custom URL.

Very cool!

From a UX perspective, I strongly suspect this will manifest as a ball of edge cases to deal with

Thanks. You've highlighted a number of interesting edge cases.

Don't get me wrong, I think this would be an interesting problem to solve, but I think at this point, based on what we know, my preference would be:

  1. Let's wrap this project up without the "Create a thank you page for me" button.
  2. After this PR has shipped we'll ask Gary will shift gears to work on the single block plugin proof of concept. Priority-wise, that's just something that is time sensitive that we're going to need to figure out for upcoming sprints.
  3. We can then add the "Create a thank you page for me" button idea as an issue that we can circle back to at a later time.

@matticbot
Copy link
Contributor

pento, Your synced wpcom patch D33974-code has been updated.

@matticbot
Copy link
Contributor

pento, Your synced wpcom patch D33974-code has been updated.

@matticbot
Copy link
Contributor

pento, Your synced wpcom patch D33974-code has been updated.

@pento pento requested a review from jeherve October 16, 2019 22:43
@pento pento added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Oct 16, 2019
@pento
Copy link
Contributor Author

pento commented Oct 16, 2019

All of the feedback to date has been addressed. The codeclimate check is still failing, but looking at recent PR merges, it seems like that isn't a blocker?

@jeherve, could you give it another once over?

@kraftbj
Copy link
Contributor

kraftbj commented Oct 17, 2019

I defer to @jeherve on the actual review since there's history, but correct, we're working on making codeclimate checks actionable... The LOC checks won't be kept, so removing the codeclimate blocker.

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 looks good to me now. It should be good to merge!

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Oct 17, 2019
@pento pento changed the title Contact Form: Add post-submission thankyou message and redirect options Contact Form: Add post-submission options. Oct 18, 2019
@pento pento merged commit d576123 into master Oct 18, 2019
@pento pento deleted the add/contact-form-submit-message branch October 18, 2019 02:13
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 18, 2019
@jeherve jeherve added [Status] Needs Testing We need to add this change to the testing call for this month's release [Status] Has Changelog and removed [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release labels Oct 18, 2019
jeherve added a commit that referenced this pull request Oct 23, 2019
jeherve added a commit that referenced this pull request Oct 29, 2019
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Contact Form 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.

Contact form: add a redirect option
10 participants