Skip to content
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.

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

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

Merged
merged 8 commits into from
Feb 12, 2014

Conversation

westonruter
Copy link
Contributor

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

Fixes #45

Remaining tasks:

  • Don't submit value for preview if it ends in trailing whitespace, since sanitize_text_field will strip it out and make it seem there is an error in the field. (Core widgets use strip_tags anyway, so this doesn't seem to be a need.)
  • Force an input to be updated even if it is focused if the form was submitted by pressing Enter (so that a blur doesn't have to happen)
  • When doing a full form replacement (for non-live preview widgets), try to preserve cursor location after replacing the entire form in the field which was being edited. Chrome seems to want to select all of the contents when focusing.
  • Ensure compatibility with RSS widget, which can show an error message. See wp_widget_rss_form
  • Provide facility for widgets to provide custom live-preview support, for handling when fields get submitted for preview, and how the fields get updated when the updated form returns via Ajax.
  • Add support for Twenty Fourteen Ephemera widget
  • Fire events on controls that are unsanitary.

Not in scope:

  • When a widget is submitted for live preview, the Ajax handler should respond not only with the widget form, but it should also allow the widget to send back (via a filter) arbitrary information about error messages or statuses.
  • Yellow-fade inputs and the widget-inside as a whole when the form gets fields updated with sanitized values back from the server. Right now it adds a red background color to invalid fields.

Always attempt to re-focus last focused element
Use document.activeElement instead of manually tracking focus
@westonruter
Copy link
Contributor Author

@shaunandrews @MichaelArestad @bobbravo2 please try this out.

@shaunandrews
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link
Contributor Author

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

@shaunandrews
Copy link
Contributor

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
Copy link
Contributor

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 (#93 (comment)).

@westonruter
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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.

westonruter added a commit that referenced this pull request Feb 12, 2014
See live preview changes as-you-type, without hitting Update button
@westonruter westonruter merged commit 44243ac into develop Feb 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants