Skip to content

Build placeholders with JS and optimize spinner animations #3531

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.

Sign up for GitHub

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

Merged
Crabcyborg merged 1 commit into from
Oct 2, 2026
Merged

Build placeholders with JS and optimize spinner animations #3531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 32 additions & 0 deletions classes/controllers/FrmFieldsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@ public static function load_single_field( $field_object, $values, $form_id = 0 )

if ( $ajax_loading && $ajax_this_field ) {
$li_classes = self::get_classes_for_builder_field( array(), $display, $field_obj );

if ( isset( $values['placeholder_manifest'] ) ) {
self::add_builder_placeholder_to_manifest( $field_object, $display, $li_classes, $values['placeholder_manifest'] );
return;
}

include FrmAppHelper::plugin_path() . '/classes/views/frm-fields/back-end/ajax-field-placeholder.php';
return;
}
Expand Down Expand Up @@ -271,6 +277,32 @@ public static function load_single_field( $field_object, $values, $form_id = 0 )
require FrmAppHelper::plugin_path() . '/classes/views/frm-forms/add_field.php';
}

/**
* Record shared attributes and leave a minimal placeholder in its grid row.
*
* @since x.x
*
* @param object $field Field object with its id and owning form.
* @param array $display Field display options, including the builder type.
* @param string $li_classes Classes used by the original placeholder.
* @param object{definitions: array, fields: array}&\stdClass $manifest Shared attribute definitions and ordered field records.
*
* @return void
*/
private static function add_builder_placeholder_to_manifest( $field, array $display, $li_classes, $manifest ) {
$definition = array( $li_classes . ' frm_field_loading', (int) $field->form_id, $display['type'] );
$index = array_search( $definition, $manifest->definitions, true );

if ( false === $index ) {
$index = count( $manifest->definitions );
$manifest->definitions[] = $definition;
}

$placeholder = count( $manifest->fields );
$manifest->fields[] = array( (int) $field->id, $index );
echo '<li data-frm-placeholder="' . esc_attr( $placeholder ) . '"></li>';
}

/**
* @since 3.0
*
Expand Down
21 changes: 20 additions & 1 deletion classes/views/frm-forms/form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@
$grid_helper = new FrmFieldGridHelper();
$values['count'] = 0;

if ( ! empty( $values['ajax_load'] ) ) {
$values['placeholder_manifest'] = (object) array(
'definitions' => array(),
'fields' => array(),
);
}

foreach ( $values['fields'] as $field ) {
++$values['count'];
$grid_helper->set_field( $field );
Expand All @@ -62,10 +69,22 @@
}
$grid_helper->force_close_field_wrapper();
unset( $grid_helper );
}
}//end if
?>
</ul>

<?php
if ( ! empty( $values['placeholder_manifest']->fields ) ) {
wp_print_inline_script_tag(
wp_json_encode( $values['placeholder_manifest'], JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT ),
array(
'id' => 'frm-field-placeholders',
'type' => 'application/json',
)
);
}
?>

<?php if ( ! FrmAppHelper::is_admin_page() ) : ?>
<p id="frm-form-button">
<button class="frm_button_submit" disabled="disabled">
Expand Down
2 changes: 1 addition & 1 deletion js/formidable_admin.js
View file
Open in desktop

Large diffs are not rendered by default.

20 changes: 12 additions & 8 deletions js/src/admin/admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ const { initFieldListHoverPill } = require( './fieldListHoverPill' );
const { initShowBoxIconSwap } = require( './showBoxIconSwap' );
const { hydrateBuilderSelect, hydrateBuilderSelectsIn } = require( './sharedSelectOptions' );
const { processFieldLoadBatch } = require( './fieldLoadBatch' );
const { hydrateFieldPlaceholders } = require( './fieldPlaceholders' );

// Footer scripts can restore placeholders before add-ons inspect the builder's fields.
hydrateFieldPlaceholders();

window.FrmFormsConnect = window.FrmFormsConnect || ( function( document, window, $ ) {
const el = {
Expand Down Expand Up @@ -2660,11 +2664,12 @@ window.frmAdminBuildJS = function() {
let placeholderSpinnerObserver;

/**
* Give each field placeholder its spinner only once it scrolls into view.
* Give each field placeholder a spinner only while it is in the viewport.
*
* A long form can have hundreds of placeholders, and most of them are swapped for the real
* field before anyone scrolls to them, so a placeholder that is never seen never gets one.
* The placeholder already holds the spinner's space, so adding it does not move anything.
* Remove spinners that leave the viewport so their animations do not keep running offscreen.
* The placeholder holds the spinner's space, so adding or removing it does not move fields.
*
* @since x.x
*
Expand All @@ -2676,10 +2681,7 @@ window.frmAdminBuildJS = function() {
return;
}

placeholderSpinnerObserver = new IntersectionObserver(
handlePlaceholderIntersections,
{ root: postBodyContent }
);
placeholderSpinnerObserver = new IntersectionObserver( handlePlaceholderIntersections );
placeholders.forEach( placeholder => placeholderSpinnerObserver.observe( placeholder ) );
}

Expand All @@ -2693,11 +2695,10 @@ window.frmAdminBuildJS = function() {
entries.forEach(
( { target, isIntersecting } ) => {
if ( ! isIntersecting ) {
target.querySelector( '.frm_visible_spinner' )?.remove();
return;
}

placeholderSpinnerObserver.unobserve( target );

// A placeholder that failed to load shows an error message instead.
if ( target.hasChildNodes() ) {
return;
Expand Down Expand Up @@ -2822,6 +2823,7 @@ window.frmAdminBuildJS = function() {
fieldIds.forEach( fieldId => {
const field = document.getElementById( `frm_field_id_${ fieldId }` );
if ( field?.classList.contains( 'frm_field_loading' ) ) {
placeholderSpinnerObserver?.unobserve( field );
field.textContent = __( 'Unable to load field.', 'formidable' );
}
} );
Expand Down Expand Up @@ -11856,6 +11858,8 @@ window.frmAdminBuildJS = function() {
},

buildInit() {
hydrateFieldPlaceholders();

@franky-the-going-merry franky-the-going-merry Bot Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: this second call looks redundant. formidable_admin is registered with in_footer = true, so it always runs after the builder markup, and the call at the top of this file already hydrates and removes the manifest. By the time buildInit() runs, this just does a getElementById that finds nothing. One call site makes it clearer which one guarantees hydration before observeFieldPlaceholders().

Suggested change
hydrateFieldPlaceholders();


document.addEventListener( 'focusin', event => {
if ( event.target.matches( 'select[data-frm-options]' ) ) {
hydrateBuilderSelect( event.target );
Expand Down
26 changes: 26 additions & 0 deletions js/src/admin/fieldPlaceholders.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Restore placeholder attributes before the builder scans its grid or starts requests.
*
* @since x.x
* @return {void}
*/
export function hydrateFieldPlaceholders() {
const manifestElement = document.getElementById( 'frm-field-placeholders' );
const fieldsContainer = document.getElementById( 'frm-show-fields' );
if ( ! manifestElement || ! fieldsContainer ) {
return;
}

@franky-the-going-merry franky-the-going-merry Bot Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: the call at the top of admin.js runs this at module load with no guard. If the manifest script is ever empty or mangled (a CSP/minifier plugin, or wp_json_encode() returning false), JSON.parse throws and no later code in the bundle runs, including frmAdminBuildJS. Before this PR a bad placeholder only affected that one field. A try { ... } catch ( e ) { return; } around the parse would keep the failure contained. It only takes a malformed manifest to trigger, so it is a robustness nicety rather than a defect.


const { fields, definitions } = JSON.parse( manifestElement.textContent );
fieldsContainer.querySelectorAll( 'li[data-frm-placeholder]' ).forEach( field => {
const [ fieldId, definitionIndex ] = fields[ field.dataset.frmPlaceholder ];
const [ className, formId, type ] = definitions[ definitionIndex ];
field.id = `frm_field_id_${ fieldId }`;
field.className = className;
field.dataset.fid = fieldId;
field.dataset.formid = formId;
field.dataset.ftype = type;
delete field.dataset.frmPlaceholder;
} );
manifestElement.remove();
}
80 changes: 80 additions & 0 deletions tests/phpunit/fields/test_FrmFieldsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,86 @@
#[\PHPUnit\Framework\Attributes\CoversClass( FrmFieldsController::class )]
class test_FrmFieldsController extends FrmUnitTest {

public function test_builder_placeholder_manifest_preserves_attributes_and_order() {
$form_id = $this->factory->form->create();
$manifest = (object) array(
'definitions' => array(),
'fields' => array(),
);
$expected = array();

foreach ( array( 'text', 'text', 'html' ) as $type ) {
$field = $this->factory->field->create_and_get(
array(
'form_id' => $form_id,
'type' => $type,
)
);
ob_start();
FrmFieldsController::load_single_field(
$field,
array(
'ajax_load' => true,
'count' => 11,
)
);
$legacy_html = ob_get_clean();

ob_start();
FrmFieldsController::load_single_field(
$field,
array(
'ajax_load' => true,
'count' => 11,
'placeholder_manifest' => $manifest,
)
);
$placeholder = ob_get_clean();
$index = count( $expected );
$this->assertSame( '<li data-frm-placeholder="' . $index . '"></li>', $placeholder );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

$this->assertSame( (int) $field->id, $manifest->fields[ $index ][0] );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

$definition = $manifest->definitions[ $manifest->fields[ $index ][1] ];
$this->assertStringContainsString( 'class="' . esc_attr( $definition[0] ) . '"', $legacy_html );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertStringContainsString()


The method you are trying to call is not defined, which can result in a fatal error.

$this->assertStringContainsString( 'data-formid="' . esc_attr( $definition[1] ) . '"', $legacy_html );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertStringContainsString()


The method you are trying to call is not defined, which can result in a fatal error.

$this->assertStringContainsString( 'data-ftype="' . esc_attr( $definition[2] ) . '"', $legacy_html );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertStringContainsString()


The method you are trying to call is not defined, which can result in a fatal error.

$expected[] = (int) $field->id;
}

$this->assertCount( 2, $manifest->definitions );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertCount()


The method you are trying to call is not defined, which can result in a fatal error.

$this->assertSame( $expected, array_column( $manifest->fields, 0 ) );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

$this->assertSame( $manifest->fields[0][1], $manifest->fields[1][1] );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

}

public function test_builder_placeholder_manifest_keeps_initial_and_non_ajax_fields_rendered() {
$form_id = $this->factory->form->create();
$field = $this->factory->field->create_and_get(
array(
'form_id' => $form_id,
'type' => 'text',
)
);
$manifest = (object) array(
'definitions' => array(),
'fields' => array(),
);

foreach ( array( array( true, 10 ), array( false, 11 ) ) as $settings ) {
ob_start();
FrmFieldsController::load_single_field(
$field,
array(
'ajax_load' => $settings[0],
'count' => $settings[1],
'placeholder_manifest' => $manifest,
)
);
$html = ob_get_clean();
$this->assertStringContainsString( 'id="frm_field_id_' . $field->id . '"', $html );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertStringContainsString()


The method you are trying to call is not defined, which can result in a fatal error.

$this->assertStringNotContainsString( 'data-frm-placeholder=', $html );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertStringNotContainsString()


The method you are trying to call is not defined, which can result in a fatal error.

}
$this->assertSame( array(), $manifest->fields );

@deepsource-io deepsource-io Bot Oct 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Call to an undefined method test_FrmFieldsController::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

}

public function test_builder_batches_omit_only_received_definitions() {
$definitions = array(
'known' => array( 'value' => 'Saved option' ),
Expand Down
Loading