-
Notifications
You must be signed in to change notification settings - Fork 826
Forms: Improve passing PHP data to Javascript #45076
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: changed | ||
|
||
Forms: Consolidate data to frontend. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
* External dependencies | ||
*/ | ||
import jetpackAnalytics from '@automattic/jetpack-analytics'; | ||
import { getScriptData } from '@automattic/jetpack-script-data'; | ||
import { Modal, __experimentalVStack as VStack } from '@wordpress/components'; // eslint-disable-line @wordpress/no-unsafe-wp-apis | ||
import { useState } from '@wordpress/element'; | ||
import { __ } from '@wordpress/i18n'; | ||
|
@@ -20,7 +21,7 @@ import './style.scss'; | |
*/ | ||
import type { Integration } from '../../../../types'; | ||
|
||
const isMailPoetEnabled: boolean = !! window?.jpFormsBlocks?.defaults?.isMailPoetEnabled; | ||
const isMailPoetEnabled = Boolean( getScriptData()?.forms?.isMailPoetEnabled ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An example of replacing jpFormsBlocks method with the new getScriptsData() single-source-of-data method. |
||
|
||
const IntegrationsModal = ( { | ||
isOpen, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,12 +10,16 @@ | |
use Automattic\Jetpack\Connection\Manager as Connection_Manager; | ||
use Automattic\Jetpack\Constants; | ||
use Automattic\Jetpack\Extensions\Contact_Form\Contact_Form_Block; | ||
use Automattic\Jetpack\Forms\Dashboard\Dashboard as Forms_Dashboard; | ||
use Automattic\Jetpack\Forms\Dashboard\Dashboard_View_Switch; | ||
use Automattic\Jetpack\Forms\Jetpack_Forms; | ||
use Automattic\Jetpack\Forms\Service\MailPoet_Integration; | ||
use Automattic\Jetpack\Forms\Service\Post_To_Url; | ||
use Automattic\Jetpack\Redirect; | ||
use Automattic\Jetpack\Status; | ||
use Automattic\Jetpack\Terms_Of_Service; | ||
use Automattic\Jetpack\Tracking; | ||
use Jetpack_AI_Helper; | ||
use Jetpack_Options; | ||
use WP_Block; | ||
use WP_Block_Patterns_Registry; | ||
|
@@ -102,6 +106,9 @@ public static function init() { | |
|
||
// Schedule our daily cleanup | ||
add_action( 'wp_scheduled_delete', array( $instance, 'daily_akismet_meta_cleanup' ) ); | ||
|
||
// Provide consolidated Forms script data to both editor and admin via JetpackScriptData. | ||
add_filter( 'jetpack_admin_js_script_data', array( __CLASS__, 'add_forms_script_data' ), 10, 1 ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This hooks is how we call the method that adds our data to Jetpack's script data. |
||
} | ||
|
||
return $instance; | ||
|
@@ -3379,4 +3386,50 @@ public function redirect_edit_feedback_to_jetpack_forms() { | |
wp_safe_redirect( $redirect_url ); | ||
exit; | ||
} | ||
|
||
/** | ||
* Add Forms script data to the global JetpackScriptData object for use across admin and editor. | ||
* | ||
* All Forms-related data (including capabilities used by Forms UIs) is nested under the 'forms' key. | ||
* | ||
* @param array $data Existing script data. | ||
* @return array Modified script data including Forms data. | ||
*/ | ||
public static function add_forms_script_data( $data ) { | ||
if ( ! is_array( $data ) ) { | ||
$data = array(); | ||
} | ||
|
||
$forms = isset( $data['forms'] ) && is_array( $data['forms'] ) ? $data['forms'] : array(); | ||
$dashboard_view_switch = new Dashboard_View_Switch(); | ||
$ai_feature = Jetpack_AI_Helper::get_ai_assistance_feature(); | ||
$has_ai = ! is_wp_error( $ai_feature ) ? $ai_feature['has-feature'] : false; | ||
|
||
// From jpFormsBlocks in class-contact-form-block.php. | ||
$forms['formsResponsesUrl'] = $dashboard_view_switch->get_forms_admin_url(); | ||
$forms['preferredView'] = $dashboard_view_switch->get_preferred_view(); | ||
$forms['isMailPoetEnabled'] = Jetpack_Forms::is_mailpoet_enabled(); | ||
// From config in class-dashboard.php. | ||
$forms['blogId'] = get_current_blog_id(); | ||
$forms['gdriveConnectSupportURL'] = esc_url( Redirect::get_url( 'jetpack-support-contact-form-export' ) ); | ||
$forms['pluginAssetsURL'] = Jetpack_Forms::assets_url(); | ||
$forms['siteURL'] = ( new Status() )->get_site_suffix(); | ||
$forms['hasFeedback'] = ( new Forms_Dashboard() )->has_feedback(); | ||
$forms['hasAI'] = $has_ai; | ||
$forms['enableIntegrationsTab'] = apply_filters( 'jetpack_forms_enable_integrations_tab', true ); | ||
$forms['renderMigrationPage'] = $dashboard_view_switch->is_jetpack_forms_announcing_new_menu(); | ||
$forms['dashboardURL'] = add_query_arg( 'jetpack_forms_migration_announcement_seen', 'yes', $dashboard_view_switch->get_forms_admin_url() ); | ||
// New data. | ||
$forms['canInstallPlugins'] = current_user_can( 'install_plugins' ); | ||
$forms['canActivatePlugins'] = current_user_can( 'activate_plugins' ); | ||
|
||
// Include nonces only on the Forms dashboard screen to avoid leaking tokens elsewhere. | ||
if ( is_admin() && isset( $_GET['page'] ) && 'jetpack-forms-admin' === $_GET['page'] ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- read-only context | ||
$forms['exportNonce'] = wp_create_nonce( 'feedback_export' ); | ||
$forms['newFormNonce'] = wp_create_nonce( 'create_new_form' ); | ||
} | ||
|
||
$data['forms'] = $forms; | ||
return $data; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If adopted, this new method would be the way that we pass all data in the Jetpack Forms to the frontend, replacing the 'config' on the dashboard and the jpFormsBlock object in the editor. I've basically recreated each item from jpFormsBlock and config objects here as part of this PR. In future PRs, we can refactor individual files to use this rather than config / jpFormsBlocks. I tried doing all of that here, but it expanded to much. Also note that I've added two new items here for canInstallPlugins and canActivatePlugins. Once merged, I'd update this PR to reference these properties rather than needing to pass both properties to both jpFormsBlocks and config separately. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,16 +111,6 @@ public function load_admin_scripts() { | |
|
||
// Adds Connection package initial state. | ||
Connection_Initial_State::render_script( self::SCRIPT_HANDLE ); | ||
|
||
$api_root = defined( 'IS_WPCOM' ) && IS_WPCOM | ||
? sprintf( '/wpcom/v2/sites/%s/', esc_url_raw( rest_url() ) ) | ||
: '/wp-json/wpcom/v2/'; | ||
|
||
wp_add_inline_script( | ||
self::SCRIPT_HANDLE, | ||
'window.jetpackFormsData = ' . wp_json_encode( array( 'apiRoot' => $api_root ) ) . ';', | ||
'before' | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In reviewing all the data we're passing to the frontend, I found the api_root element we set up here is not used, so I've removed it. |
||
} | ||
|
||
/** | ||
|
@@ -230,7 +220,6 @@ public function render_dashboard( $extra_config = array() ) { | |
'enableIntegrationsTab' => self::$show_integrations, | ||
'renderMigrationPage' => $this->switch->is_jetpack_forms_announcing_new_menu(), | ||
'dashboardURL' => add_query_arg( 'jetpack_forms_migration_announcement_seen', 'yes', $this->switch->get_forms_admin_url() ), | ||
'isMailpoetEnabled' => Jetpack_Forms::is_mailpoet_enabled(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we refactor to use the getScriptsData method, we can remove each item from the config object here, as I've done with MailPoet. |
||
); | ||
|
||
if ( ! empty( $extra_config ) ) { | ||
|
@@ -246,7 +235,7 @@ public function render_dashboard( $extra_config = array() ) { | |
* | ||
* @return boolean | ||
*/ | ||
private function has_feedback() { | ||
public function has_feedback() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This had to to be made public because it's now used in the add_forms_script_data() method above. |
||
$posts = new \WP_Query( | ||
array( | ||
'post_type' => 'feedback', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,13 @@ | |
* External dependencies | ||
*/ | ||
import jetpackAnalytics from '@automattic/jetpack-analytics'; | ||
import { getScriptData } from '@automattic/jetpack-script-data'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { useState, useCallback } from 'react'; | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { useIntegrationsStatus } from '../../blocks/contact-form/components/jetpack-integrations-modal/hooks/use-integrations-status'; | ||
import { config } from '../index'; | ||
import AkismetDashboardCard from './akismet-card'; | ||
import CreativeMailDashboardCard from './creative-mail-card'; | ||
import GoogleSheetsDashboardCard from './google-sheets-card'; | ||
|
@@ -32,7 +32,7 @@ const Integrations = () => { | |
mailpoet: false, | ||
} ); | ||
|
||
const isMailpoetEnabled = config( 'isMailpoetEnabled' ); | ||
const isMailpoetEnabled = Boolean( getScriptData()?.forms?.isMailPoetEnabled ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An example of replacing the config object/method with the new getScriptData() single-source-of-data method. |
||
|
||
const toggleCard = useCallback( ( cardId: keyof typeof expandedCards ) => { | ||
setExpandedCards( prev => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,6 +106,48 @@ export interface JPFormsBlocksDefaults { | |
isMailPoetEnabled?: boolean; | ||
} | ||
|
||
/** | ||
* Forms script data exposed via JetpackScriptData.forms | ||
*/ | ||
export interface FormsScriptData { | ||
/** Whether MailPoet integration is enabled across contexts. */ | ||
isMailPoetEnabled?: boolean; | ||
/** Whether the current user can install plugins (install_plugins). */ | ||
canInstallPlugins?: boolean; | ||
/** Whether the current user can activate plugins (activate_plugins). */ | ||
canActivatePlugins?: boolean; | ||
/** Whether to show the Integrations tab in the dashboard UI. */ | ||
enableIntegrationsTab?: boolean; | ||
/** Whether to render the migration/announcement page instead of the main dashboard. */ | ||
renderMigrationPage?: boolean; | ||
/** Whether there are any feedback (form response) posts on the site. */ | ||
hasFeedback?: boolean; | ||
/** Whether AI Assist features are available for the site/user. */ | ||
hasAI?: boolean; | ||
/** The URL of the Forms responses list in wp-admin. */ | ||
formsResponsesUrl?: string; | ||
/** Current site blog ID. */ | ||
blogId?: number; | ||
/** Support URL for Google Drive connect guidance. */ | ||
gdriveConnectSupportURL?: string; | ||
/** Base URL to static/assets for the Forms package. */ | ||
pluginAssetsURL?: string; | ||
/** The site suffix/fragment for building admin links. */ | ||
siteURL?: string; | ||
/** The dashboard URL with migration acknowledgement parameter. */ | ||
dashboardURL?: string; | ||
/** Nonce for exporting feedback responses (dashboard-only). */ | ||
exportNonce?: string; | ||
/** Nonce for creating a new form (dashboard-only). */ | ||
newFormNonce?: string; | ||
} | ||
|
||
declare module '@automattic/jetpack-script-data' { | ||
interface JetpackScriptData { | ||
forms?: FormsScriptData; | ||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to extend Jetpack's script data type. |
||
/** | ||
* Augments the global Window interface to include Jetpack Forms block defaults. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: enhancement | ||
|
||
Forms: Consolidate data to frontend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In reviewing all the data we're passing to the frontend, I found these are no longer used, so I've removed them.