Forms: Add synced forms support with convert toolbar #46297
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
bot
added
[Block] Contact Form
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
github-actions
bot
added
the
[Status] Needs Author Reply
enejb
added
[Status] Needs Review
github-actions
bot
added
[Status] Needs Author Reply
There was a problem hiding this comment.
Pull request overview
This PR introduces synced form functionality to Jetpack Forms, allowing forms to be stored centrally in a jetpack_form custom post type and reused across multiple locations. Forms can be converted between inline and synced modes via a new toolbar interface, with content managed centrally for synced forms.
Key Changes:
- Added
refattribute to contact form blocks to reference centrally managed forms stored in thejetpack_formcustom post type - Implemented conversion toolbar enabling users to convert inline forms to synced forms and navigate to edit synced forms
- Created PHP rendering logic with circular reference prevention and post status validation for synced forms
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
projects/packages/forms/src/blocks/contact-form/utils/form-sync-manager.ts |
New utility module providing functions to serialize form blocks and create synced forms via REST API |
projects/packages/forms/src/blocks/contact-form/hooks/use-synced-form.ts |
New custom hook to load and parse synced forms from the jetpack_form post type with proper TypeScript type inference |
projects/packages/forms/src/blocks/contact-form/components/convert-form-toolbar.tsx |
New toolbar component with buttons to convert forms to synced mode and edit existing synced forms |
projects/packages/forms/src/blocks/contact-form/edit.tsx |
Updated editor component to integrate synced form loading, display loading states, and sync attributes/innerBlocks from referenced forms |
projects/packages/forms/src/blocks/contact-form/index.js |
Modified save function to return null for synced forms (ref attribute) since content is stored centrally |
projects/packages/forms/src/blocks/contact-form/class-contact-form-block.php |
Added render_synced_form method with circular reference prevention, post validation, and status checks |
projects/packages/forms/src/blocks/contact-form/attributes.ts |
Added ref attribute definition to support referencing jetpack_form posts |
projects/packages/forms/changelog/update-form-block-saves-custom-post-type |
Changelog entry documenting the addition of ConvertFormToolbar component |
Comments suppressed due to low confidence (1)
projects/packages/forms/src/blocks/contact-form/class-contact-form-block.php:768
- When a synced form has an invalid post status (not publish or draft), the function returns an empty string with no user feedback. Consider returning an error message to help users understand why the form isn't displaying, especially for forms that might have been accidentally trashed or set to pending.
// Only render published and draft post statuses.
// todo: add a "active" status so that we can disable forms without deleting them.
if ( ! in_array( $synced_form->post_status, array( 'publish', 'draft' ), true ) ) {
return '';
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Only render published and draft post statuses. | ||
| // todo: add a "active" status so that we can disable forms without deleting them. |
There was a problem hiding this comment.
This TODO comment should be tracked in an issue rather than left inline. Consider creating a GitHub issue to track the implementation of an 'active' status for forms and removing this comment.
| // todo: add a "active" status so that we can disable forms without deleting them. |
| // Clear syncing flag after a short delay | ||
| setTimeout( () => { | ||
| isSyncingRef.current = false; | ||
| }, 100 ); |
There was a problem hiding this comment.
Using setTimeout with a hardcoded 100ms delay to clear the syncing flag is fragile and may not account for slow operations or race conditions. Consider using a ref callback or useLayoutEffect to ensure the flag is cleared synchronously after the DOM updates, or implement proper promise-based state management.
| import { serialize } from '@wordpress/blocks'; | ||
|
|
||
| const FORM_BLOCK_NAME = 'jetpack/contact-form'; | ||
|
|
||
| export interface BlockData { | ||
| attributes: Record< string, unknown >; | ||
| innerBlocks: unknown[]; |
There was a problem hiding this comment.
The innerBlocks property is typed as unknown[] which bypasses type safety. Consider using a more specific type like Block[] or the appropriate block type from WordPress to ensure type safety when working with block structures.
| import { serialize } from '@wordpress/blocks'; | |
| const FORM_BLOCK_NAME = 'jetpack/contact-form'; | |
| export interface BlockData { | |
| attributes: Record< string, unknown >; | |
| innerBlocks: unknown[]; | |
| import { serialize, Block } from '@wordpress/blocks'; | |
| const FORM_BLOCK_NAME = 'jetpack/contact-form'; | |
| export interface BlockData { | |
| attributes: Record< string, unknown >; | |
| innerBlocks: Block[]; |
| [ clientId ] | ||
| ); | ||
|
|
||
| // // Load synced innerBlocks and attributes when a ref exists and the form is loaded |
There was a problem hiding this comment.
This comment has double forward slashes at the beginning. Remove one forward slash to follow standard comment formatting.
| // // Load synced innerBlocks and attributes when a ref exists and the form is loaded | |
| // Load synced innerBlocks and attributes when a ref exists and the form is loaded |
| * Serializes a block (with attributes and innerBlocks) to block markup string | ||
| * | ||
| * @param {object} blockData - Block data containing attributes and innerBlocks |
There was a problem hiding this comment.
The documentation states this serializes a block, but the function accepts BlockData (with attributes and innerBlocks) rather than a full block object. Consider clarifying the parameter documentation to say "block data" instead of "block" to match the function name and parameter type.
| * Serializes a block (with attributes and innerBlocks) to block markup string | |
| * | |
| * @param {object} blockData - Block data containing attributes and innerBlocks | |
| * Serializes block data (with attributes and innerBlocks) to block markup string | |
| * | |
| * @param {BlockData} blockData - Block data containing attributes and innerBlocks |
|
|
||
| try { | ||
| // Remove ref from attributes if it exists (shouldn't, but safety check) | ||
| const { ...cleanAttributes } = attributes; |
There was a problem hiding this comment.
The destructuring syntax here doesn't actually remove the 'ref' property from attributes. The spread operator ...cleanAttributes creates a shallow copy of all properties in attributes. To properly exclude the 'ref' property, you should use: const { ref, ...cleanAttributes } = attributes;
| const { ...cleanAttributes } = attributes; | |
| const { ref, ...cleanAttributes } = attributes; |
| // Circular reference prevention. | ||
| static $seen_refs = array(); | ||
|
|
||
| if ( isset( $seen_refs[ $ref_id ] ) ) { | ||
| return sprintf( | ||
| '<div class="wp-block-jetpack-contact-form">%s</div>', | ||
| esc_html__( 'Circular reference detected in form.', 'jetpack-forms' ) | ||
| ); | ||
| } | ||
|
|
||
| // Load the jetpack-form post. | ||
| $synced_form = get_post( $ref_id ); | ||
|
|
||
| // Validate post. | ||
| if ( ! $synced_form || 'jetpack_form' !== $synced_form->post_type ) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Only render published and draft post statuses. | ||
| // todo: add a "active" status so that we can disable forms without deleting them. | ||
| if ( ! in_array( $synced_form->post_status, array( 'publish', 'draft' ), true ) ) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Mark as seen for circular reference prevention. | ||
| $seen_refs[ $ref_id ] = true; | ||
|
|
||
| // Parse and render blocks from post_content. | ||
| $blocks = parse_blocks( $synced_form->post_content ); | ||
| $output = ''; | ||
|
|
||
| foreach ( $blocks as $block ) { | ||
| $output .= render_block( $block ); | ||
| } | ||
|
|
||
| // Clean up. | ||
| unset( $seen_refs[ $ref_id ] ); |
There was a problem hiding this comment.
The static $seen_refs array for circular reference prevention will persist across multiple requests in persistent PHP environments (like PHP-FPM). If an error occurs between marking a ref as seen and unsetting it, subsequent requests could incorrectly detect circular references. Consider adding cleanup logic or using instance-level tracking instead of static variables.
| interface ConvertFormToolbarProps { | ||
| clientId: string; | ||
| attributes: Record< string, unknown >; | ||
| setAttributes: ( attrs: Record< string, unknown > ) => void; |
There was a problem hiding this comment.
The setAttributes parameter in the ConvertFormToolbarProps interface is defined but never used in the component. Consider removing it from both the interface and the component's parameter destructuring, or document why it's included if it's needed for future functionality.
| setAttributes: ( attrs: Record< string, unknown > ) => void; |
| if ( isset( $atts['ref'] ) && is_numeric( $atts['ref'] ) ) { | ||
| return self::render_synced_form( $atts['ref'] ); | ||
| } | ||
|
|
||
| return Contact_Form::parse( $atts, do_blocks( $content ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Render a synced form by reference ID. | ||
| * | ||
| * @param int $ref_id The jetpack_form post ID. | ||
| * @return string Rendered form HTML. | ||
| */ | ||
| private static function render_synced_form( $ref_id ) { | ||
| // Circular reference prevention. | ||
| static $seen_refs = array(); | ||
|
|
||
| if ( isset( $seen_refs[ $ref_id ] ) ) { | ||
| return sprintf( | ||
| '<div class="wp-block-jetpack-contact-form">%s</div>', | ||
| esc_html__( 'Circular reference detected in form.', 'jetpack-forms' ) | ||
| ); | ||
| } | ||
|
|
||
| // Load the jetpack-form post. | ||
| $synced_form = get_post( $ref_id ); |
There was a problem hiding this comment.
The ref_id parameter is validated with is_numeric(), but this doesn't prevent negative numbers or ensure the value is a valid positive integer. Consider using absint() or additional validation to ensure only valid positive integers are processed as post IDs.
| // Only render published and draft post statuses. | ||
| // todo: add a "active" status so that we can disable forms without deleting them. | ||
| if ( ! in_array( $synced_form->post_status, array( 'publish', 'draft' ), true ) ) { |
There was a problem hiding this comment.
The check allowing jetpack_form posts with post_status of draft to be rendered means draft forms can be displayed on public pages if a block references their ref ID. This bypasses the usual WordPress expectation that drafts are not publicly visible and could unintentionally expose in-progress or disabled forms and their configuration to unauthenticated visitors. Restrict rendering to publish status for front-end requests (and/or gate draft rendering behind appropriate capability checks or editor-only contexts) so that non-public form statuses cannot be surfaced to general users.
| // Only render published and draft post statuses. | |
| // todo: add a "active" status so that we can disable forms without deleting them. | |
| if ( ! in_array( $synced_form->post_status, array( 'publish', 'draft' ), true ) ) { | |
| // Only render published forms on the front end. | |
| // Allow drafts only in admin/editor contexts or for users with edit capability. | |
| if ( | |
| 'publish' !== $synced_form->post_status && | |
| ( | |
| ! is_admin() && | |
| ! ( defined( 'REST_REQUEST' ) && REST_REQUEST ) && | |
| ! ( defined( 'DOING_AJAX' ) && DOING_AJAX ) && | |
| ! current_user_can( 'edit_post', $synced_form->ID ) | |
| ) | |
| ) { |
Code Coverage SummaryCoverage changed in 7 files. Only the first 5 are listed here.
9 files are newly checked for coverage. Only the first 5 are listed here.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
Introduces a new 'ref' attribute to the contact form block, allowing forms to be rendered by referencing a jetpack_form post by ID. Implements circular reference prevention and ensures only published or draft forms are rendered.
Introduces a custom React hook to load and parse synced contact forms from the jetpack_form post type. The hook fetches the referenced form, parses its block content, and returns loading state, attributes, and inner blocks for use in the contact form block.
The save function now checks for the 'ref' attribute. If present, it returns null to avoid saving innerBlocks for synced forms, as their content is managed elsewhere. Inline forms continue to save the full block with innerBlocks.
Introduces utilities to serialize contact form blocks and create synced forms via the Jetpack API. Provides type definitions and functions for converting between inline and synced form modes.
Introduces the ConvertFormToolbar component to enable converting contact forms to synced forms and editing synced forms directly from the block toolbar. Updates the contact form edit logic to support loading and syncing form data when a ref is present, and conditionally displays the toolbar based on the central form management feature flag.
enejb
force-pushed
the
update/form-block-saves-custom-post-type
branch
from
cac1db9 to
782c3eb
Compare
Replaced hardcoded 'jetpack_form' strings with the FORM_POST_TYPE constant across multiple files for consistency and maintainability. The constant is now defined in shared/util/constants.js and imported where needed.
Introduces an isJetpackFormEditor flag using the current post type and FORM_POST_TYPE constant. Updates the ConvertFormToolbar rendering to only show when not in the Jetpack form editor, improving context-aware UI behavior.
Refactors the contact form block to support inline editing of reusable (synced) forms by loading, parsing, and saving form content directly from/to the referenced form post. Introduces logic to fetch and apply reusable form attributes and inner blocks, and ensures changes are persisted back to the source form. Adds error handling for missing referenced forms and improves loading state handling. Also adds a new hook (use-auto-save-synced-form) for auto-saving synced form changes when the parent post is saved.
Introduces a new Form Document Settings React component and plugin, displaying form configuration panels in the Document Settings sidebar for jetpack_form post types. Updates the form editor to register this plugin, adds related documentation, and includes supporting styles. Also updates dependencies to include @wordpress/edit-post and @wordpress/plugins.
| </PluginDocumentSettingPanel> | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Settings on block vs post? this version of the form post type, we show the Form block. If we're going to show that anyways, is it worth the extra code and complexity here to move Form block settings from the block to the post type?
Maybe you're trying to prepare for hiding the form block itself?
| ) } | ||
| </ToolbarGroup> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Edit form. It's a bit odd to have an 'Edit Form' button on the form block, when I just edit it directly on the screen. I understand why the button is there, but it may not be obvious to users. I'm also not sure what the better solution is.
2 participants
This PR introduces a new toolbar that show up on the form block that lets us convert form blocks into synced form blocks.
See
Screen.Recording.2025-12-12.at.11.47.04.AM.mov
Proposed changes:
refattribute that references ajetpack_formcustom post typeuseSyncedFormhook to load and parse synced contact forms from the custom post typeConvertFormToolbarcomponent to enable converting between inline and synced forms directly from the block toolbarparsefunctionOther information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Prerequisites:
central-form-managementfeature flag is enabledTest converting inline form to synced form:
refattribute pointing to ajetpack_formpostTest editing synced form:
jetpack_formpostTest rendering synced forms:
Test circular reference prevention:
Test TypeScript compilation:
pnpm run lint:tsin the forms packagesyncedInnerBlocksorParsedBlocktypes"--base trunk