Display server-sent Customizer setting validation errors before save is attempted

[westonruter]

Following up on #34893.

At the moment, the setting validation errors that are returned by WP_Customize_Setting::validate() are only displayed when a save is attempted. This is the only situation where these errors are sent back from the server, as part of the customize_save_response. However, now that there is strict validation available in the Customizer, if a validation error does happen when editing a setting, then the behavior is for the Customizer preview to act as if the supplied setting does not exist at all. This is a bad user experience because then their changes in the preview just disappear. In this case, I should think it is unlikely that the user would try to save their changes since they don't appear properly in the preview, and thus the validation errors would never get displayed to the user.

With transactions, this issue would be addressed by continually sending the setting values to the server to amend the transaction, and in the response the validation errors could be included, as noted in #34893 (comment 42):

• Selective refresh seems to revert to the previous setting value when a setting is invalid. Perhaps we should do this but also keep it faded/in an updating state to indicate that there is a problem with the current setting (hence the previous value being used)?

Yeah, the revert to the previous setting is a result of the invalid settings now being (properly) rejected, and so they are treated as if they weren't supplied at all. This would be true in both selective refresh and full-page refresh. At the moment, it would be up to the control to display the notification using JS to give feedback for why an update isn't appearing, as currently PHP-supplied error notifications are currently only being sent back upon attempted Save. However, with transactions (#30937) each change to a setting would get submitted to the server via a PATCH request, and so this is where I think we'd need to ultimately provide this feedback for why a setting change isn't appearing in the preview. I don't know if we should display any invalidity state inside the preview itself.

In the mean time, without transactions in core, there needs to be an alternate mechanism for giving early feedback on validation errors. I suggest that we can send back the setting validation states whenever the preview is refreshed or whenever a selecting refresh occurs.

[westonruter]

In 36944.0.wip.diff​:

• Send back all validities for all settings, not just the error notifications for invalid settings.
• Explicitly link a control notification to its corresponding setting.
• Keep track of which notifications are from the server versus which are created in JS.
• Prevent submitting settings if there are validity errors that are not from the server.
• Skip removing all notification errors at save, only removing errors if they are absent from the subsequent save response.
• Add wp.customize.findControlsForSettings( settingIds ) helper.

What is next is to make sure the setting validity check are done during full refresh and partial refresh requests, and sent back to the Customizer as well.

[westonruter]

​https://github.com/xwp/wordpress-develop/pull/151/commits/e39c8ce8937d5d4e17c3bc5e9054ad2e08ae240f

[westonruter]

In 36944.1.diff​:

• Export setting validities with full refreshes and selective refreshes, giving near immediate feedback for validation errors.
• Introduce WP_Customize_Manager::prepare_setting_validity_for_js().

The UX should be so much better to show/hide notifications for server-side setting validation handling.

@fjarrett @mrahmadawais @aristath @natewr would one or more of you please test?

[westonruter]

Add qunit test: ​https://github.com/xwp/wordpress-develop/pull/151/commits/fc148309c5c88c05ecd30dc591ee276a54975298

[mrahmadawais]

Replying to westonruter:

The UX should be so much better to show/hide notifications for server-side setting validation handling.

@fjarrett @mrahmadawais @aristath @natewr would one or more of you please test?

Sure.

So, to test it, I need to apply this patch to the core, what else? Should I install the validation gist plugin as an example?

[westonruter]

Replying to mrahmadawais:

So, to test it, I need to apply this patch to the core, what else? Should I install the validation gist plugin as an example?

Exactly. Apply the patch and add the ​Customize Validate Entitled Settings demo plugin. You should see then that validation errors happen in conjunction with selective refreshes and full refreshes.

[NateWr]

I had a quick test with the demo plugin. I really like it. A couple of thoughts:

1. It can be a little bit disorienting the way that the preview falls back to the currently saved value. For instance, I title a widget Hello and then save it. Then I type Hello World and see it in the preview. Then I add a ! and the preview goes back to showing Hello. I don't think it's a critical problem, especially since the error message appears there. But can still feel a bit odd. If it could show the last valid value that'd be great.

2. Skimming the code in the diff and the demo plugin, it looks like JS Setting objects now have a Notification object attached, and these are passed to a control, which has a renderNotifications method. So to render a validation error in a custom control we just need to do some work in the renderNotifications method, correct? That seems pretty straightforward if correct, and if I understand the code correctly it seems like errors will be announced (wp.ally.speak) prior to this, meaning we don't need to reproduce that. Very nice.

3. I'm wondering how I would go about extending a Notification object's data param. For my use case, I have a kind of "uber" Control. It saves to a single Setting, but in the process it compiles a bunch of different input fields into a single serialized string. So for me, the setting id won't be sufficient for determining where to display the error message. I would need additional params attached to the Notification. It seems to include a data param which looks like it is where extra data could be attached, but I'm not seeing anything in api._handleSettingValidities that would allow me to attach extra params. Can you point me to what I'm missing?

[westonruter]

Replying to NateWr:

I had a quick test with the demo plugin. I really like it. A couple of thoughts:

Thanks for the great feedback!

1. It can be a little bit disorienting the way that the preview falls back to the currently saved value. For instance, I title a widget Hello and then save it. Then I type Hello World and see it in the preview. Then I add a ! and the preview goes back to showing Hello. I don't think it's a critical problem, especially since the error message appears there. But can still feel a bit odd. If it could show the last valid value that'd be great.

That's a good idea. And actually, this improvement you describe could be something we get for free with transactions (#30937). The problem with implementing it right now (before transactions) is that the setting values entered do not persist anywhere except in the life of the request. But with transactions, each update would get written to the transaction, with any invalid values being rejected from being written to the transaction (I presume). So yeah, in your example “Hello World” would get written to the transaction but then “Hello World!” would get rejected, so the preview would remain with “Hello World”.

2. Skimming the code in the diff and the demo plugin, it looks like JS Setting objects now have a Notification object attached, and these are passed to a control, which has a renderNotifications method. So to render a validation error in a custom control we just need to do some work in the renderNotifications method, correct? That seems pretty straightforward if correct, and if I understand the code correctly it seems like errors will be announced (wp.ally.speak) prior to this, meaning we don't need to reproduce that. Very nice.

Yes, a control can override how notifications are displayed by overriding the renderNotifications in the wp.customize.Control subclass.

3. I'm wondering how I would go about extending a Notification object's data param. For my use case, I have a kind of "uber" Control. It saves to a single Setting, but in the process it compiles a bunch of different input fields into a single serialized string. So for me, the setting id won't be sufficient for determining where to display the error message. I would need additional params attached to the Notification. It seems to include a data param which looks like it is where extra data could be attached, but I'm not seeing anything in api._handleSettingValidities that would allow me to attach extra params. Can you point me to what I'm missing?

This sounds very similar to what is needed for Customize Posts as well. For this plugin, a post setting contains the title, content, excerpt, and all the other core fields. When an invalid title is supplied, the entire setting is marked as invalid. However, there needs to be a way to indicate which specific control should receive the notification even though they all share the same post setting. The short answer is that the validate method for the setting needs to do something like this:

return new WP_Error( 'invalid_title', __( 'Invalid title' ), array( 'post_field' => 'post_title' ) )

And then this post_field data will be included in the Notification object's data property, which you can use to determine whether or not the error should be displayed for a given control. In the case of Customize Posts, it should only get rendered on the Title control. I'm going to work on this for Customize Posts and I'll share with you the PR so you can review and adapt for your as well.

Otherwise, a +1 from you for commit?

[westonruter]

​https://github.com/xwp/wordpress-develop/pull/151/commits/e4279662ed6c5440a309d1c2daa168db39558640

[westonruter]

In 36944.3.diff​:

• Introduce Setting#findControls to split out logic from wp.customize.findControlsForSettings, allowing individual settings to override the logic for how associated controls are found.

@NateWr this change should help you give some control (pun intended) over how controls are listed for a given setting with associated notifications. You can override a setting's findControls method to handle a case when a single setting is associated with multiple controls, or when a single control has multiple fields that map to a setting with an object value (e.g. nav menu items or widget). I added support for handling notifications for a setting associated with multiple controls in the Customize Posts plugin: ​https://github.com/xwp/wp-customize-posts/pull/180

[NateWr]

Cheers @westonruter. I had a quick look over the PR and I think it makes sense. I see now how you set the setting_property param from the server side, so that's useful to know.

Yes, a +1 from me for commit, but I'm not exactly anyone to check with on that sort of thing. :)

[mrahmadawais]

Replying to westonruter:

So, I went ahead and tested the patch. Works perfect. The UX is much better now. I had a similar concern as of what @NateWr had on extending a Notification object's data param to display the notification for the right Setting ID (which may be attached to another Setting ID triggering the validation error) but you have already answered and addressed it in the PR 180.

+1 for the commit for sure.

[westonruter]

@NateWr @mrahmadawais I just updated the “Customize Validate Entitled Settings” demo plugin to also implement this routing of the error notifications to the title field: ​https://gist.github.com/westonruter/1016332b18ee7946dec3/revisions#diff-c7d2182050c602e788c279958c8c0006"

Thanks both for reviewing!

[westonruter]

In 37700:

Customize: Update server-sent setting validation notifications as changes are entered.

Send back setting validities with full refreshes and selective refreshes so that invalid settings can have notifications displayed immediately before attempting save, and so that these notifications can be cleared as soon as the input is corrected.

• Splits out JS logic for listing controls into separate methods wp.customize.Setting.prototype.findControls() and wp.customize.findControlsForSettings().
• Adds a setting property to the data on notifications added to controls that are synced from their settings.
• Adds selective-refresh-setting-validities message sent from preview to pane.
• Changes WP_Customize_Manager::validate_setting_values() to return when settings are valid as well as invalid.
• Adds WP_Customize_Manager::prepare_setting_validity_for_js().
• Add setting validities to data exported to JS in Customizer Preview and in selective refresh responses.

Fixes #36944.

[westonruter]

In 37867:

Customize: Always define functions reflowPaneContents, findControlsForSettings, and _handleSettingValidities on wp.customize.

Moves definitions of functions outside of a jQuery.ready() callback, as these functions needn't be deferred to DOM ready. This change also ensures that the functions are available if customize-controls is enqueued outside of the Customizer context, as the ready callback short-circuits if _wpCustomizeSettings is not defined. The findControlsForSettings and _handleSettingValidities functions were misplaced in r37700.

See #34893.
See #36944.
See #29071.

[westonruter]

In 40319:

Customize: Prevent client-side validation from being cleared when no corresponding server-side validation is present.

See #36944.
Fixes #39770.

[swissspidy]

In 40345:

Customize: Prevent client-side validation from being cleared when no corresponding server-side validation is present.

See #36944.
Fixes #39770.

Merges [40319] to the 4.7 branch.