Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class WPCOM_REST_API_V2_Endpoint_Block_Editor_Assets extends WP_REST_Controller
'jetpack/address',
'jetpack/ai-assistant',
'jetpack/ai-chat',
'jetpack/blog-stats',
'jetpack/blogging-prompt',
'jetpack/blogroll',
'jetpack/blogroll-item',
Expand All @@ -61,6 +62,7 @@ class WPCOM_REST_API_V2_Endpoint_Block_Editor_Assets extends WP_REST_Controller
'jetpack/contact-info',
'jetpack/cookie-consent',
'jetpack/donations',
'jetpack/dropzone',
'jetpack/email',
'jetpack/event-countdown',
'jetpack/eventbrite',
Expand All @@ -69,35 +71,56 @@ class WPCOM_REST_API_V2_Endpoint_Block_Editor_Assets extends WP_REST_Controller
'jetpack/field-consent',
'jetpack/field-date',
'jetpack/field-email',
'jetpack/field-file',
'jetpack/field-multiple-choice',
'jetpack/field-phone',
'jetpack/field-name',
'jetpack/field-number',
'jetpack/field-option-checkbox',
'jetpack/field-option-radio',
'jetpack/field-radio',
'jetpack/field-rating',
'jetpack/field-select',
'jetpack/field-single-choice',
'jetpack/field-slider',
'jetpack/field-telephone',
'jetpack/field-text',
'jetpack/field-textarea',
'jetpack/field-time',
'jetpack/field-url',
'jetpack/form-progress-indicator',
'jetpack/form-step',
'jetpack/form-step-container',
'jetpack/form-step-divider',
'jetpack/form-step-navigation',
'jetpack/gif',
'jetpack/goodreads',
'jetpack/google-calendar',
'jetpack/image-compare',
'jetpack/input',
'jetpack/input-range',
'jetpack/input-rating',
'jetpack/instagram-gallery',
'jetpack/label',
'jetpack/latex',
'jetpack/like',
'jetpack/mailchimp',
'jetpack/map',
'jetpack/markdown',
'jetpack/nextdoor',
'jetpack/opentable',
'jetpack/option',
'jetpack/options',
'jetpack/payment-buttons',
'jetpack/payments-intro',
'jetpack/paypal-payment-buttons',
'jetpack/paywall',
'jetpack/phone',
'jetpack/phone-input',
'jetpack/pinterest',
'jetpack/podcast-player',
'jetpack/rating-priciness',
'jetpack/rating-spiciness',
'jetpack/rating-star',
'jetpack/recurring-payments',
'jetpack/related-posts',
Expand All @@ -114,6 +137,7 @@ class WPCOM_REST_API_V2_Endpoint_Block_Editor_Assets extends WP_REST_Controller
'jetpack/timeline',
'jetpack/timeline-item',
'jetpack/tock',
'jetpack/top-posts',
'jetpack/whatsapp-button',
'premium-content/buttons',
'premium-content/container',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: other

Block editor: expand allowed block types for the mobile editor
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,4 @@
add_action( 'enqueue_block_editor_assets', array( Contact_Form_Block::class, 'load_editor_scripts' ), 9 );

// Load styles in the editor iframe context
if ( is_admin() ) {
add_action( 'enqueue_block_assets', array( Contact_Form_Block::class, 'load_editor_styles' ), 9 );
}
Comment on lines -17 to -19
Copy link
Member Author

@dcalhoun dcalhoun Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coder-karen my understanding is that utilizing is_admin() is the latest recommendation for enqueueing block editor content assets intended for the editor only (excluded from the site's front end). Unfortunately, the approach is problematic for REST API endpoints (like the block editor assets endpoint used for the mobile apps), as they are not "admin," which leads to the absence of this asset entirely—it's neither registered or enqueued.

To be clear, removing this is_admin() check does results in loading an additional stylesheet for the site's front end:

Image

I recognize this is likely undesired. I do not plan to merge this as-is currently.

My questions:

  • What, specifically, are we avoiding with scoping this asset for the admin? Are the styles available/duplicated elsewhere? Are they truly only relevant to the admin? If so, how?
  • Would the is_admin() usage cease and the REST API issue become moot when VULCAN-68 is completed?
  • Can you think of any low-effort workarounds to support the REST API but avoid duplicative style loads for the web?

Thank you for your help! 🙇🏻‍♂️

Copy link
Member

@simison simison Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, removing this is_admin() check does results in loading an additional stylesheet for the site's front end

Then that's a no-go, we shouldn't load more bytes in the frontend.

Have you looked at adding "is rest request" check? Looks like there's a helper:

/**
* Whether the current request is an authenticated REST API request.
*
* @return bool
*/
protected static function is_authenticated_rest_request() {
return wp_is_serving_rest_request() && current_user_can( 'read' );
}

There's also REST_REQUEST constant, some usage example here as alternative:

// Only render the email version in non-frontend contexts.
if ( is_feed() || wp_is_xml_request() ||
( defined( 'REST_REQUEST' ) && REST_REQUEST && ! wp_is_json_request() ) ||
( defined( 'REST_API_REQUEST' ) && REST_API_REQUEST ) ||
( defined( 'WP_CLI' ) && WP_CLI ) ||
wp_is_jsonp_request() ) {
return render_for_email( $data, $styles );
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simison thank you for providing feedback.

I agree. If the stylesheet in question should not be loaded in the site's front end, then we should not load the unnecessary bytes. My questions largely sought to under why the stylesheet should not be loaded (i.e., is it duplicating styles provided elsewhere?) and if there might be alternative style organization that would negate the need to avoid loading it.

Have you looked at adding "is rest request" check? Looks like there's a helper:

Yes, I actually introduced the referenced helper to the Jetpack codebase in 21ec749.

There's also REST_REQUEST constant, some usage example here as alternative:

The aforementioned helper and the utilities provided by WordPress core rely upon the REST_REQUEST. Unfortunately, as mentioned in #44979 (comment), I believe the REST_REQUEST constant is defined too late for the particular problem discussed in this thread. The constant is set after Jetpack decides whether to enqueue the targeted stylesheet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there's some CSS that specifically addresses editor only in the file. Much of it could be only in a separate view CSS file, but there's a lot of legacy here and much of the frontend styles are in grunion.scss.

Because some of the styles are needed just in editor, moving shared CSS around now wouldn't help though?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for sharing the context regarding the editor stylesheet and accompanying grunion stylesheet.

Because some of the styles are needed just in editor, moving shared CSS around now wouldn't help though?

It's unclear to me currently if there is a possibility of relocating the missing styles to an always-loaded shared stylesheet. We'd need to dive a bit deeper into the specific missing styles to understand if they are truly editor-only or could be relocated to a shared place.

For example, elements like form-step and form-progress-indicator are un-styled in the mobile app editor due to the missing stylesheet.

Web GutenbergKit
jetpack-forms-web jetpack-forms-gutenberg-kit

Presumably, those styles—form-step and form-progress-indicator—are necessary for both the admin editor and the site front end. For the editor, it appears those disparate styles are ultimately bundled with contact-form/editor.scss and that is the currently admin-only stylesheet; on the site front end, those styles appear to be loaded as an individual stylesheets—e.g., jetpack-forms/dist/blocks/form-progress-indicator/style.css

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lezama was looking a bit at organising our styles better for new fields, so I wonder if he would have thoughts here on short-term improvements?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I attempted to refactor how we enqueue/register things in #45190. But after testing the endpoint before and after the change, I realized that form-progress-indicator/style.css was already being returned by the endpoint when running trunk 😅. I might be missing something, happy to jump on a quick call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lezama thank you for looking into this. It's surprising that you say the form-progress-indicator styles are included in the endpoint return when testing the trunk branch. What specific link tag is returned that contains those styles?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha! I am sorry, I just tested again and now I don't see the styles in trunk but I do if I am running #45190. Not sure what I did Yesterday differently.

add_action( 'enqueue_block_assets', array( Contact_Form_Block::class, 'load_editor_styles' ), 9 );
Loading