xwp / wp-widget-customizer

[OBSOLETE] Widget Customizer plugin for WordPress (now merged into Core)
http://wordpress.org/plugins/widget-customizer/
54 stars 18 forks source link

See live preview changes as-you-type, without hitting Update button #93

Closed westonruter closed 10 years ago

westonruter commented 10 years ago

There's a couple more things I'd like to do for this, but it is ready for review.

Fixes #45

Remaining tasks:

Not in scope:

westonruter commented 10 years ago

@shaunandrews @MichaelArestad @bobbravo2 please try this out.

shaunandrews commented 10 years ago

This is super slick. Played with it for a bit, and it works pretty well. There's a noticeable delay, but it doesn't feel like a problem. Really nice work.

westonruter commented 10 years ago

@shaunandrews thanks! I'd like your thoughts on the way sanitization is handled. Given my sample single widget, if you make it sanitize by removing all numbers from the string, for example:

- $newoptions['title'] = sanitize_text_field( wp_unslash( $_POST['single-title'] ) );
+ $newoptions['title'] = preg_replace( '/\d/', '', sanitize_text_field( wp_unslash( $_POST['single-title'] ) ));

Then reload and see what happens when you start typing and then enter a digit. You'll see the box goes red indicating that it is not currently sanitary. When you blur the input, the sanitized value will then be supplied.

This also works client side as well. If you provide an HTML5 pattern attribute on an input, for example to only accept letters and spaces:

pattern="[A-Za-z ]*"

Then if you enter a number it won't even try submitting the input to the server. It will go red right away and will only submit the input value once you blur the input.

shaunandrews commented 10 years ago

Can we provide an error message along with the red bg? Anything to make it more obvious whats happening an why.

On Feb 11, 2014, at 9:02 PM, Weston Ruter notifications@github.com wrote:

@shaunandrews thanks! I'd like your thoughts on the way sanitization is handled. Given my sample single widget, if you make it sanitize by removing all numbers from the string, for example:

  • $newoptions['title'] = sanitize_text_field( wp_unslash( $_POST['single-title'] ) );
  • $newoptions['title'] = preg_replace( '/\d/', '', sanitize_text_field( wp_unslash( $_POST['single-title'] ) )); Then reload and see what happens when you start typing and then enter a digit. You'll see the box goes red indicating that it is not currently sanitary. When you blur the input, the sanitized value will then be supplied.

This also works client side as well. If you provide an HTML5 pattern attribute on an input, for example to only accept letters and spaces:

pattern="[A-Za-z ]*" Then if you enter a number it won't even try submitting the input to the server. It will go red right away and will only submit the input value once you blur the input.

— Reply to this email directly or view it on GitHub.

westonruter commented 10 years ago

Perhaps show the error message at the bottom next to where the spinner appears?

shaunandrews commented 10 years ago

Or above/below the field with the error? Putting it in context would help make it easier to understand.

On Feb 11, 2014, at 9:10 PM, Weston Ruter notifications@github.com wrote:

Perhaps show the error message at the bottom next to where the spinner appears? — Reply to this email directly or view it on GitHub.

MichaelArestad commented 10 years ago

Above the error field would be the most logical.

Michael

On Tuesday, February 11, 2014 at 7:17 PM, Shaun Andrews wrote:

Or above/below the field with the error? Putting it in context would help make it easier to understand.

On Feb 11, 2014, at 9:10 PM, Weston Ruter <notifications@github.com (mailto:notifications@github.com)> wrote:

Perhaps show the error message at the bottom next to where the spinner
appears?

Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub (https://github.com/x-team/wp-widget-customizer/pull/93#issuecomment-34832211).

westonruter commented 10 years ago

@shaunandrews @MichaelArestad Yeah, the problem though is positioning the error messages. Who knows how a widget is going to be laid out. What if it is made the responsibility of the widget to display these error messages and show the error indicators? By default, a widget's input values would just update to the sanitized value when you blur the field, but a widget could provide some custom styling and messaging when it is invalid and when it gets sanitized.

But we could go ahead and provide live error handling for core widgets, and other custom widgets could follow that pattern. An example of a widget that needs custom handling is the RSS widget: we don't want it to try to submit the RSS URL every time a keystroke is made on the input! That could potentially result in a lot of 404s. This widget also renders an error message in side of the widget control itself if there was an HTTP error.

westonruter commented 10 years ago

@shaunandrews @MichaelArestad I'm just removing the error indicator altogether for now. Widgets in the admin don't have any indicator for when a field got sanitized, so out of the box it seems not needed in customizer. So I've removed these styles and classes (f92a887).

The RSS widget is the only one which I believe renders an error message. There is now a hooks framework (in 59c20fc) for widgets to register custom handlers for when a widget form updates, when a field needs to be sanitized, etc.