Widget Customizer: Dynamically-created inputs cause from to replace itself without event to trigger re-initialization

[westonruter]

In the customizer, with every input/change event, a widget's fields get submitted via Ajax to the widget's update callback to obtain the updated widget instance (to pass along to the preview), and as part of this the widget's form callback is also executed and the new widget form is returned being populated with the sanitized instance. On the widgets admin page, there is a Save button which causes the widget form to get completely replaced with this sanitized widget form. But since in the customizer the form gets submitted with each user input, it cannot replace the form or else the user would continually lose their inputs if they enter them faster than the Ajax response returns.

When the Ajax response returns with the newly-sanitized widget form, we compare the existing widget form already loaded with the new widget form and check to verify that all of the same field elements are there, as if they are, then we can iterate over the original inputs in parallel with the sanitize inputs, and copy the state over from the sanitized input to the original input if sanitization caused it to be different (and if the user is not still focused in that field).

If, however, it finds that the input fields in the sanitized form do not match the current widget input fields, then the entire form is currently getting replaced (as it does on the widgets admin page).

With this background in mind: if a widget dynamically creates fields using JavaScript when the form is first rendered, any user inputs in that form will likely cause a field mismatch in the response, and the form will get replaced. Besides the poor experience of the form getting replaced unexpectedly, this also will result in any dynamically-generated fields getting lost because there is currently no event triggered when a widget form is updated (see #19675: Add a jQuery triggers to the widget save action).

There are two suggestions I have for how the above problems can be solved:

1) In the customizer, if there is a field mismatch between a sanitized form and the existing form, instead of replacing the form as on the widgets admin page, we should stop the live field updates and make the Apply button re-appear so that when this button is pressed, the form fields get disabled until the form is replaced with the new sanitized form.

2) We need to trigger an event when the widget form is updated so that any dynamically-generated fields can be re-initialized. Again, see #19675.

Also reported at:
​http://wordpress.org/support/topic/initialise-javascript-on-add-save-widget?replies=3
​http://wordpress.stackexchange.com/questions/138355/using-javascript-on-the-new-widget-preview-customizer-page

[westonruter]

Re-work how and when widget forms get updated. Add widget-form-update event for widget form updates in both customizer and widgets admin page (fixes #19675).

[westonruter]

In this patch 27491.diff:

• Re-work how and when widget forms get updated
• Replace ad hoc hooks system with jQuery events
• Add widget-form-update event for widget form updates in both customizer and widgets admin page (fixes #19675)
• Enter into a non-live form update mode, where the Apply button is restored when a sanitized form does not have the same fields as currently in the form, and so the fields cannot be easily updated to their sanitized values without doing a complete form replacement.

Patch also pushed to GitHub: ​https://github.com/x-team/wordpress-develop/pull/5/files

Provided an example plugin which incorporates Chosen and shows how the new widget-form-update event is used: ​https://gist.github.com/westonruter/9676069

[westonruter]

Refresh patch. Conflicts also resolved at ​https://github.com/x-team/wordpress-develop/pull/5/files

[ocean90]

I don't want to add hooks/events to the current system now. This more a part of #19675 and #21170 and both aren't in the scope of 3.9.

[westonruter]

Replying to ocean90:

I don't want to add hooks/events to the current system now. This more a part of #19675 and #21170 and both aren't in the scope of 3.9.

How's the patch with the hooks removed? Also, in the mean time how should we recommend widgets re-initialize dynamic fields when the form updates? As noted on #19675, currently plugin authors have to hack the Ajax success event when editing a widget on the widgets admin page. In the customizer, however, a separate hack would be needed.

[adamsilverstein]

I tried testing 27491.2.diff​ - after installing I lose live updating between the widget and the preview: (​screencast)

can we just use triggers on add new widget and save widget? like -

jQuery( document ).trigger( 'customizer-widget-added', [widget] );
jQuery( document ).trigger( 'customizer-widget-saved', [widget] );

we have similar triggers in heartbeat and autosave, they will get real hooks when we add those to core but these should work for now.

[westonruter]

Fix logic error for when to update setting. Also pushed to ​https://github.com/x-team/wordpress-develop/pull/5/files

[westonruter]

Replying to adamsilverstein:

I tried testing 27491.2.diff​ - after installing I lose live updating between the widget and the preview: (​screencast)

Sorry, I had a logic error in my patch. Refreshed. Change was ​https://github.com/x-team/wordpress-develop/commit/2da26f8c35ed16062581fe320182d802f363e6a0

Replying to adamsilverstein:

can we just use triggers on add new widget and save widget? like -

jQuery( document ).trigger( 'customizer-widget-added', [widget] );
jQuery( document ).trigger( 'customizer-widget-saved', [widget] );

we have similar triggers in heartbeat and autosave, they will get real hooks when we add those to core but these should work for now.

That's right. This is what the patch introduces, though a bit differently. Whenever the form is re-rendered, it executes:

event_data = {
	'widget_id': control.params.widget_id,
	'widget_id_base': control.params.widget_id_base,
	'new_form': r.data.form,
	'hard': ! control.live_update_mode, // dynamic fields may need to be re-initialized (e.g. Chosen)
	'customize_control': control
};
widget_root.trigger( 'widget-form-update', [ event_data ] );

(How this gets triggered, on which element and with what parameters, is of course up for discussion.)

In the context of the customizer, it doesn't make so much sense to trigger update when the settings are saved, because the widget form gets submitted any time a change is made to a field (even several times while typing into a single field). And anyway, for dynamic fields, the concern is more for when the widget form gets updated rather than when the widget gets saved, as the need is to know when the form needs to be re-initialized for any dynamic fields.

[ocean90]

+ 1 for $document.trigger().

Provided an example plugin which incorporates Chosen...

Thanks, looks good. But it seems like Chosen isn't usable until the form gets updated once.

Are there any use cases for widget-sanitary-field and widget-unsanitary-field?

[westonruter]

Replying to ocean90:

Provided an example plugin which incorporates Chosen...

Thanks, looks good. But it seems like Chosen isn't usable until the form gets updated once.

Ah, you're right. Newly-added widgets with Chosen aren't working until you update, while previously-existing widgets with Chosen are working. There is a problem with the deep cloning of widget templates. I'm looking into it.

Replying to ocean90:

Are there any use cases for widget-sanitary-field and widget-unsanitary-field?

The idea was that these events could be used to toggle an invalid class on on these fields, to facilitate live indicators of which fields have sanitization errors. Upon blurring the field, the value would get updated to be the sanitized value, to the widget-unsanitary-field could be used to warn the user of invalid input.

[westonruter]

Replying to ocean90:

Provided an example plugin which incorporates Chosen...

Thanks, looks good. But it seems like Chosen isn't usable until the form gets updated once.

OK, the primary problem here is newly added widgets are added by grabbing the HTML for the widget template and then replacing __i__ with the widget number, and then inserting the HTML into the document. So any event handlers added directly to the widget template itself will not be carried over to the newly-added widget. This is the same behavior as on the widgets admin page. I realize this is why event delegation has been so critical for any listening to any events on widget fields.

But any elements which have events or data attached directly to them are currently broken both for newly-added widgets on the widgets admin page and in the customizer alike.

By the way, with Chosen I tried changing the customizer widget addition method to use jQuery.clone( deep ) instead of manipulating the HTML of the widget template, but this also is failing in the case of Chosen because it seems it is not designed to be cloned: ​http://stackoverflow.com/a/17156928/93579

So, I think what should be done is we should stop rendering out the widget template as HTML and instead include it on the page in an unparsed form (e.g. in a data attribute). Then when the widget is added apply trigger a widget-added event (or widget-form-rendered to also be used when updates happen) so that any event handlers and dynamic fields can be initialized. This should be done on both the Customizer and the Widgets Admin page to handle setup for newly added widgets.

[westonruter]

So, I think what should be done is we should stop rendering out the widget template as HTML and instead include it on the page in an unparsed form (e.g. in a data attribute). Then when the widget is added apply trigger a widget-added event (or widget-form-rendered to also be used when updates happen) so that any event handlers and dynamic fields can be initialized. This should be done on both the Customizer and the Widgets Admin page to handle setup for newly added widgets.

Well, abandoning the old widget templating method would break plugins which manipulate the template via the DOM upon document ready. For example Jetpack's Widget Visibility injects the "Visibility" button before the "Save" button when the page first loads: ​https://github.com/Automattic/jetpack/blob/e9a1fe859cdbd3b2c8869de4a6dbd6cbe85fb526/modules/widget-visibility/widget-conditions/widget-conditions.js#L31-L45

So instead, when adding a widget I think we should use jQuery.clone( deep ) to copy the widget template for the new instance, then trigger a widget-added event. Then when the form gets replaced as result of a save or update action, then a widget-updated event can fire and at this point the same widget form initialization routines can be run which were done initially upon document ready.

Disregard Chosen from this discussion because it seems to be fundamentally broken in this regard.

[westonruter]

Use widget-updated and widget-added events, triggered on document. Break up widget-form-updated hard/soft variants into separate widget-updated and widget-synced events. Eliminate widget-sanitary-field and widget-unsanitary-field events, pending validation of need. Commits pushed to GitHub: ​https://github.com/x-team/wordpress-develop/pull/5/files

[westonruter]

I got stuck down a rabbit hole of trying to improve the way that widgets get cloned from their templates so that events and data attached to the widget templates would be persisted via a deep copy when the widget was added to a widget area: ​https://github.com/x-team/wordpress-develop/commit/542ad8f342afee43feab8f8017bac755cbfacf90

But I found that a widget with a jQuery UI control failed when deep cloning just as a widget with Chosen in it. So I abandoned that path. However, in the future the way that widget controls get instantiated should really start out with a widget template not pulled from a hidden DOM element, but instead pull from a template string that is re-parsed each time. What widgets now do to manipulate the widget templates upon DOM ready (e.g. Jetpack's Widget Visibility), they should instead transition to do in a widget-added event. Widget controls at the moment are not just accommodating to dynamic behaviors, as they have been largely static HTML forms. This needs to change in the future. Widgets should be completely overhauled to use Backbone.

So, I updated the example widget to use the widget-updated and widget-added events, which in the latter case cleans up any inert Chosen field copied during the shallow clone, and re-initializes the Chosen select: ​https://gist.github.com/westonruter/9676069

[westonruter]

Re: ocean90's note on IRC: "I'm getting the Apply button also for Widget Visibility." Remove OPTION from elements examined when syncing sanitized form fields. Only consider :inputs with [name]s when syncing sanitized states. Disregard input type and nodeName when computing form field signature. See additional commits since last patch at ​https://github.com/x-team/wordpress-develop/compare/cd317e3ff2d4fafc3a56560614b781fbb51f5d25...e735fb403f9028284e20b1626f4987a77723583c

[westonruter]

With my latest patch, option elements are disregarded when syncing a sanitized form with the loaded form. It is expected that the JS in the form will be populating the select elements dynamically to correspond with any server-side processing to supply the options. Also, now that only elements with name attributes are considered, the sample plugin using Chosen will not trigger an "Apply" to appear because the named input elements will not get out of sync with the inputs with name attributes in the sanitized form.

[ocean90]

option elements are disregarded when syncing a sanitized form with the loaded form.

But you are still doing .find( ':input[name]' ); which includes options elements too. Not if that is an issue, because I now have an issue with the "Flexible Posts Widget" plugin (​Download here and needs patch from #27619).
Under the "Taxonomy & Term" tab there is a dropdown menu which adds a checkbox list based on the value of the dropdown menu. Without your patch it works as expected, but with the patch I first have to click the "Apply" to get the checkbox list.

[westonruter]

Restore live update mode if sanitized fields are now aligned with the existing fields. Changes from previous patch: ​https://github.com/x-team/wordpress-develop/commit/439d8d9c03c3985510e38feb4d3dbcfe27defad0

[westonruter]

Replying to ocean90:

Without your patch it works as expected, but with the patch I first have to click the "Apply" to get the checkbox list.

Without the patch, it works but poorly. What's actually happening is that when you change the select, it is submitting the form for sanitization. When the Ajax response returns, the inputs in the sanitized form are different than the inputs in the existing form (namely, the number of checkbox inputs have changed). Without the patch, the existing form was getting replaced with the sanitized form right away since the existing inputs could not be aligned with the sanitized inputs.

Replacing the form right away when the inputs can't be aligned, however, is not a good experience and is what the patch seeks to avoid. If, for example, you start typing into a field and every input event causes the form fields to get replaced, then the user will lose a lot of the input they are entering. So this is why the Apply button is supposed to appear if it is determined that the sanitized form can't be synced without replacing the entire form: it forces the user to leave focus on any active element so that when the form does update, they don't lose anything they are inputting.

[ocean90]

In 27909:

Widget Customizer: Improve support for dynamically-created inputs.

• Re-work how and when widget forms get updated.
• Replace ad hoc hooks system with jQuery events,
• Add widget-updated/widget-synced events for widget soft/hard updates.
• Enter into a non-live form update mode, where the Apply button is restored when a sanitized form does not have the same fields as currently in the form, and so the fields cannot be easily updated to their sanitized values without doing a complete form replacement. Also restores live update mode if sanitized fields are aligned with the existing fields again.

Note: jQuery events are *not* final yet, see #19675.

props westonruter.
see #27491.