WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 13 months ago

Last modified 33 hours ago

#27355 closed task (blessed) (fixed)

Customizer: Add framework for selective refresh (partial preview refreshes)

Reported by: westonruter Owned by: westonruter
Milestone: 4.5 Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch has-unit-tests commit has-screenshots
Focuses: javascript Cc:

Description (last modified by westonruter)

In regards to the Widget Customizer merge in #27112, we agreed to remove the partial preview refreshing of widgets (live previews via postMessage transport and re-rendering widgets via Ajax then doing DOM replacements). This live preview functionality for widgets was removed in favor of generalizing the functionality so that any customize control can add support for doing so. This partial preview refresh is key to keeping things DRY by eliminating the current requirement that all setting updates via postMessage transport re-implement all rendering logic in JavaScript for components that are normally written out in PHP—but even when the render logic is attempted to be written in JS, it will never be able to account for any filters added in PHP.

For instance, with such partial preview refreshes, the Navigation Menu control needn't do a full refresh of the preview: the menu's container could be targeted for a partial refresh, where the new menu gets rendered via an Ajax request and the response replaces the old menu.

Originally mentioned at https://core.trac.wordpress.org/ticket/27112#comment:10

Feature plugin available
WordPress.org: https://wordpress.org/plugins/customize-partial-refresh/
GitHub project: https://github.com/xwp/wp-customize-partial-refresh


For summaries of the feature as merged into 4.5, see Make Core posts:

Change History (114)

#1 @samuelsidler
3 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Version changed from 3.4 to trunk

This ticket was mentioned in IRC in #wordpress-dev by ocean90. View the logs.


3 years ago

#3 @westonruter
3 years ago

  • Focuses ui added
  • Milestone changed from 3.9 to Future Release

There is too much work to do here to make it for 3.9. Punting this to future release.

For reference, the partial preview refresh code removed from Widget Customizer can be seen here: https://github.com/x-team/wp-widget-customizer/compare/without-partial-previews

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


3 years ago

#5 @johnbillion
3 years ago

  • Keywords needs-patch added
  • Version changed from trunk to 3.4

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


3 years ago

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


3 years ago

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


3 years ago

#9 follow-up: @celloexpressions
2 years ago

We might need to squeeze this into 4.1, as Twenty Fifteen implements colorschemes in a way that desperately begs for this. We'll see if it's feasible to get it patched in time, given our two other big high-priority Customizer API tickets that are still in progress/waiting for commit: #28709 and #29572. See #29988.

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


2 years ago

#11 in reply to: ↑ 9 @westonruter
2 years ago

Replying to celloexpressions:

We might need to squeeze this into 4.1, as Twenty Fifteen implements colorschemes in a way that desperately begs for this. We'll see if it's feasible to get it patched in time, given our two other big high-priority Customizer API tickets that are still in progress/waiting for commit: #28709 and #29572. See #29988.

Added a twentyfifteen-specific patch to do partial preview refreshes of inline styles: #29988 (comment)

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


2 years ago

#13 @celloexpressions
2 years ago

  • Milestone changed from Future Release to 4.1
  • Owner set to westonruter
  • Status changed from new to assigned

Per IRC discussion, this could actually be fairly straightforward as a core API, probably simpler than implementing in a theme anyway. @westonruter will take a look for 4.1 next week if there's time after #28709 is done, and if we make it in we'll have Twenty Fifteen leverage it.

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.


2 years ago

#16 @westonruter
2 years ago

Just wanted to jot down this idea I had regarding this: when a partial is in the process of being refreshing, we could apply a 0.5 opacity with a CSS transition (by default) to indicate that it is in the process of refreshing. A theme could override this with different behavior if they wanted.

Also wanted to share the idea whereby partial refresh will nicely make possible inline editing. If each partial registered has an associated CSS selector for the element inside the preview that should be refreshed when the associated setting is changed from the Customizer pane, we can use this in the opposite direction as well: when clicking on an element in the preview, any controls that are associated with that element by the partials' selectors can be identified, and the entire list of controls relevant to that context can be filtered down. What's more is that these controls could be loaded up on the frontend outside the context of the traditional Customizer at all. You could be on a page and shift-click an element (for example), to trigger the Customizer control related to that element to be dynamically created and presented as a floating box next to the element. Or the control could slide up from the bottom of the window if on mobile.

For settings registered with transport=postMessage, we should also let these be eligible for inclusion in the frontend in the same way, even though there is no specific partial that they are associated with. Perhaps the CSS selector shouldn't be a property of a new “Partial” object at all, but rather a property on the Customizer Setting object. This would allow any setting to be associated with one (or more) CSS selectors. The “postMessage” transport would then be synonymous with just a “js” transport. And actually, “transport” becomes somewhat of a poor name since there is no cross-frame communication at all if these interactions are happening on the front-end.

Anyway, I wanted to dump out these ideas here, even though they should get a separate ticket, so that the ideas can help guide the implementation of partial preview refreshes as a first step.

This ticket was mentioned in Slack in #core by westonruter. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by westonruter. View the logs.


2 years ago

#19 @westonruter
2 years ago

In 27355.wip.diff (HEAD: 6847dde):

  • Clean up customize-setting classes for coding standards, phpDoc.
  • Introduce Setting.selector for frontend partial association.
  • Eliminate duplicated values exported in preview.
  • Let Pane sync settings into customizer via postMessage.
  • Sync settings objects (w/ selector) instead of just the values.
  • Add WP_Customize_Setting::json().
  • Update TwentyFourteen to re-use selector defined in PHP.

See GitHub PR: https://github.com/xwpco/wordpress-develop/pull/51

With the selector now in place, the next step is to add support for the partialRefresh transport type.

#20 follow-up: @celloexpressions
2 years ago

  • Focuses ui removed

A thought:

  • If a setting has a selector and transport: postMessage, do $( selector ).html( to ); for the setting in core, eliminating the need to implement it in JS. If no selector is specified, require implementing it manually as you must currently do. In other words, by specifying a selector you say that the value of that setting is the content of that selector, so no other handling is needed.
  • That wouldn't let you specify a selector for ones that you have to actually use it yourself in JS, but on the other hand such JS is typically very dom-oriented and front-end, so specifying a selector in PHP then getting it from the API in JS would be fairly roundabout anyway.
  • That's not directly related to partial refreshes, but makes it even easier to implement partial refreshes where the entire contents of the selector is the value of the setting.

Also,

  • Unfortunately, we're not going to be able to make any changes to Twenty Fourteen here, since it needs to be 100% back-compat to 3.8. But we can do whatever we'd like for Twenty Fifteen.
  • WP_Customize_Manager::update_settings() could be really useful. It should probably wrap a singular version, which would be a bit nicer than modifying properties directly, as the current get_ methods require. Of course, we would need to introduce this for all of the Customizer objects - panels, sections, settings, and controls. Probably needs a new ticket, maybe not 4.1 material.
  • Other than that, the current patch looks like a good start.

#21 in reply to: ↑ 20 @westonruter
2 years ago

Replying to celloexpressions:

  • If a setting has a selector and transport: postMessage, do $( selector ).html( to ); for the setting in core, eliminating the need to implement it in JS. If no selector is specified, require implementing it manually as you must currently do. In other words, by specifying a selector you say that the value of that setting is the content of that selector, so no other handling is needed.

Yeah, good thought. However, only sometimes is the JS behavior just sticking the value into a certain element as the HTML or (better) text content. Changing a color, for instance, wouldn't make sense at all to use $( setting.selector ).text( value ). So then that would necessitate having some declarative way in PHP to indicate how JS should handle the updating of a setting on the element(s) found by the selector: do you update the text content? Do you replace the HTML for the entire element? Do you change an attribute's value? Do you change some style property? So you can see that this quickly can get out of hand, this automatic generating JS logic to apply setting changes. So I think for now it should continue to be a manual update if postMessage transport is supplied.

  • That's not directly related to partial refreshes, but makes it even easier to implement partial refreshes where the entire contents of the selector is the value of the setting.

Yeah, if the transport is partialRefresh, then we can automatically apply the change because by definition we're updating a specific element, and not having to do any special logic. We would, however, be firing events when a partial gets updated so that Customizer preview JS can re-attach any event handlers or construct dynamic elements.

Even in this case, however, I think the default handler for refreshing a partial's element should be allowed to be explicit. When attaching these default behaviors, we could check to see if the theme already defines custom behaviors for a partial refresh and defer to it.

  • Unfortunately, we're not going to be able to make any changes to Twenty Fourteen here, since it needs to be 100% back-compat to 3.8. But we can do whatever we'd like for Twenty Fifteen.

Thanks for pointing that out. I'll revert.

  • WP_Customize_Manager::update_settings() could be really useful. It should probably wrap a singular version, which would be a bit nicer than modifying properties directly, as the current get_ methods require. Of course, we would need to introduce this for all of the Customizer objects - panels, sections, settings, and controls. Probably needs a new ticket, maybe not 4.1 material.

OK, so $wp_customize->update_setting( $id, $array ) and $wp_customize->update_settings( array( $id1 => array(…), $id2 => array(…), … ) )? And then implement update_panel(s), update_section(s), update_control(s) as well? Good stuff. Yeah, I agree that this should go in another ticket.

This ticket was mentioned in Slack in #core by westonruter. View the logs.


2 years ago

#23 @westonruter
2 years ago

I suggest punting this to 4.2 and instead rely on JS in the Customizer for compiling the CSS for TwentyFifteen. For TwentyFifteen and #29988, using JS to manage the CSS makes for a much better experience in that the changes happen instantly without any HTTP requests, and it is DRY in that the JS-rendered CSS gets stored in the theme_mod to then be output via PHP for normal visitors. The caveat is that the settings that comprise the CSS must only be edited in the Customizer (including the background color), so any other admin pages that touch those Appearance settings have to be disabled (#25569 and #25571).

In 29988#comment:20 I also get into the use of logic-less Mustache templates for applying settings to templates. This is a piece missing from Core I think. Underscore templates often have JS code embedded within them, and this makes them unusable for rendering settings onto templates via PHP. Likewise, PHP “templates” are even more logic-filled and so are unsuitable for rendering settings onto templates with JS.

Taylor Buley presents a great idea in his talk Taylor Buley: How Parade.com Uses the WordPress Theme Customizer API for how they used Mustache templates for rendering template partials as opposed to straight PHP, and this enabled them to use postMessage transport much more extensively.

I know it is unrealistic to expect developers to start writing Mustache templates exclusively, so the partial refresh proposed in this ticket is sort of a stopgap.

#24 @celloexpressions
2 years ago

  • Milestone changed from 4.1 to Future Release

#25 @westonruter
2 years ago

For a client project I'm assigned to, we're going to be heavily using widgets and the Customizer. (Big surprise.) I've gone back to the old Widget Customizer feature-as-plugin code and resurrected the partial refresh logic we had in there and put it into a new standalone plugin: Customize Partial Refresh.

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


2 years ago

#27 @westonruter
2 years ago

  • Milestone changed from Future Release to 4.2

Adding this to 4.2 since it is closely related to #30937 (Customize Transactions), and since transactions currently re-implement the preview refresh functionality to do a regular location.reload(), resulting in a momentary page redraw which would be eliminated by partial refreshing (and make refreshing faster across the board, anyway).

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


2 years ago

#29 @celloexpressions
2 years ago

  • Milestone changed from 4.2 to Future Release

#30 @westonruter
2 years ago

  • Description modified (diff)

#31 @westonruter
23 months ago

  • Milestone changed from Future Release to 4.3

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


23 months ago

#33 @obenland
22 months ago

  • Milestone changed from 4.3 to Future Release

#34 @wonderboymusic
19 months ago

  • Milestone changed from Future Release to 4.4

@westonruter has expressed interest in shepherding this in 4.4

#35 @westonruter
19 months ago

  • Type changed from enhancement to task (blessed)

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


19 months ago

#37 @westonruter
19 months ago

See also #33738 which highlights a problem with postMessage for previewing changes: it fails to take into account any PHP filters that may have been applied, such as applying wptexturize to the blogname. This specific issue would be solved by the partialRefresh transport, at the expense of a slower preview update (though still much faster still than a full page refresh).

#38 @westonruter
18 months ago

  • Summary changed from Customizer: Add framework for partial preview refreshes to Customizer: Add framework for selective refresh (partial preview refreshes)

This ticket was mentioned in Slack in #core by westonruter. View the logs.


18 months ago

This ticket was mentioned in Slack in #core by westonruter. View the logs.


18 months ago

#41 @wonderboymusic
17 months ago

  • Milestone changed from 4.4 to Future Release

#42 @westonruter
17 months ago

Yep. I have a work-in-progress patch, but making a general “selective refresh” framework that can be used for nav menus, widgets, and any other thing that needs to be re-rendered via PHP is going to take time to nail, and it will require community testing to make sure the use cases are accounted for. So yeah, this needs feature plugin.

#43 @westonruter
17 months ago

@valendesigns is now picking up work on Selective Refresh in the context of the “Customize Partial Refresh” feature-as-plugin. We had a chat about a possible architecture for the framework, and I summarized it at https://github.com/xwp/wp-customize-partial-refresh/issues/11#issuecomment-156346576

Feedback welcome! We want to get this refined for the 4.5 merge window.

Last edited 17 months ago by westonruter (previous) (diff)

#44 @valendesigns
16 months ago

I've created a PR https://github.com/xwp/wp-customize-partial-refresh/pull/17 for this feature in the wp-customize-partial-refresh plugin. I would appreciate all feedback and discussion around this approach with anyone who is interested.

Last edited 16 months ago by valendesigns (previous) (diff)

#45 @westonruter
16 months ago

  • Milestone changed from Future Release to 4.5

This ticket was mentioned in Slack in #core by westonruter. View the logs.


15 months ago

This ticket was mentioned in Slack in #core by westonruter. View the logs.


15 months ago

This ticket was mentioned in Slack in #core by mike. View the logs.


15 months ago

#49 @rabmalin
15 months ago

I just found from Make post that this ticket is expecting feedback from Theme and Plugin Developer. Being a Theme developer I would also like to help in this feature. What specific thing are we expecting from Theme developer? Ease in using feature? Testing several widgets?

#50 @westonruter
15 months ago

@rabmalin Thank you. It's specifically the framework proposed here in this PR: https://github.com/xwp/wp-customize-partial-refresh/pull/17

What the framework should make easy is the ability to use PHP to render the changes to a given setting. Let's say you have an option that contains a welcome message on your site. You want this message be texturized and to allow shortcodes to be embedded in it, and anything else that the_content filters apply. You should then be able to register a new Customizer control for this option, say it a textarea, and with selective refresh you can make changes to the text in the control and watch only the element containing the welcome message refresh, not the entire page, making the changes _much_ faster to appear. The framework introduced to Core should make it easy to do this, and the patch in the PR is a proposed API for this.

Note that this does not include widgets in its scope. It is intended as the underlying selective-refresh framework which can be used by widgets and nav menus and anything else to be previewed. The existing selective refresh for nav menus should be re-implemented to use this framework.

#51 follow-up: @rabmalin
15 months ago

Great. Nice work done. Following things came to my mind after testing feature/framework branch of the repo.

  • add_theme_support declaration could be omitted. I would like to go with Nick in this. If transport is partialRefresh then framework should automatically apply that method.
  • Separate line for selector assignment in separate line could be simplified by passing it as arguments in add_setting.
  • When I tested using partialRefresh, I noticed two request on every change. One was AJAX request and another was request for whole site. I guess that was for full page refresh. May be this is a bug.
  • It would be great to some kind of notice if transport is set partialRefresh and selector is not set.

#52 in reply to: ↑ 51 @westonruter
14 months ago

Replying to rabmalin:

Great. Nice work done. Following things came to my mind after testing feature/framework branch of the repo.

  • add_theme_support declaration could be omitted. I would like to go with Nick in this. If transport is partialRefresh then framework should automatically apply that method.
  • Separate line for selector assignment in separate line could be simplified by passing it as arguments in add_setting.

Note that the selector won't have to be defined outside of the args passed into WP_Customize_Setting once a core merge happens because at that time it will be a core property defined on the class, and so it will recognized automatically.

  • When I tested using partialRefresh, I noticed two request on every change. One was AJAX request and another was request for whole site. I guess that was for full page refresh. May be this is a bug.
  • It would be great to some kind of notice if transport is set partialRefresh and selector is not set.

I've just updated the patch with a round of improvements, including most of the feedback so far: https://github.com/xwp/wp-customize-partial-refresh/pull/17#issuecomment-172401750

OK, pushed 93765c4 which introduced several changes:

  • Rename to follow selective-refresh naming.
  • Re-use postMessage transport so that JS updates can be used for live approx. updates followed by selective refresh PHP-rendered data.
  • Send selective refresh requests in batch, with Ajax response returning multiple partials
  • Use postMessage to send selective refresh partials into preview
  • Send message to preview when selective refresh starts so element can be styled.
  • Remove spinner since selective refresh can be indicated in preview elements.
  • Refresh if no elements in preview match selector.
  • Inject selector property into Setting instances, to anticipate extension to WP_Customize_Setting::json()
  • Let WP_Customize_Setting::value() be default rendered partial, using WP_Customize_Setting::render() if it exists. Supporting render_callback arg would need to be part of the Core merge.
  • Add todos for future improvements.

(Yes, the unit tests are now failing.)

I also created a demo plugin (Site Title Smiles) that uses the plugin at the current state: https://gist.github.com/westonruter/a15b99bdd07e6f4aae7a

Here's a demo video (note how text changes apply immediately via postMessage and then get upgraded to PHP-filtered text to include wptexturize and convert_smilies once the selective refresh gets applied when the Ajax request completes):

http://i1.ytimg.com/vi/ikW8dfaOPng/hqdefault.jpg

I realized a few things when working on this patch.

What we're doing here is basically implementing the logic to calculate and preview what the REST API identifies as the rendered properties, where the REST API's raw properties are what the Customizer actually has stored in the settings. There is a difference here, however, because a setting could be rendered in _multiple_ contexts in the preview, displayed in different ways in different template partials. One partial may consist entirely of the PHP-sanitized setting, whereas another partial may incorporate the setting's value as _part_ of the content. So I think our one-to-one mapping of settings to rendered partials is too simple and we need to devise a one-to-many relationship of a setting to one or more partials. This is somewhat what would be needed by nav menus since changing a nav_menu_item wouldn't correspond to updating a single nav menu item li in the preview: instead, it actually causes the element representing the nav menu to be refreshed. So all nav_menu_item settings would need to be linked to the nav_menu, and the API should support that.

I'll also note that what this plugin now does is provide a way to easily see what the PHP-sanitized setting value will be. This is what the Customize Setting Validation plugin provides _upon save_, where the actual saved values get pushed back into the JS models for the settings. With transactions, each setting change would push the setting value into the transaction post via a REST API PATCH request, and the response there can include the PHP-sanitized value to make sure that the JS setting gets updated to reflect any PHP sanitization at the time of input.

#53 follow-up: @rabmalin
14 months ago

Option 1

  • Use postMessage in transport
  • If selector is set apply Partial Refresh, otherwise apply postMessage.

Option 2

  • Use refresh in transport
  • If selector is set apply Partial Refresh, otherwise apply refresh.

Which would be good option?

#54 in reply to: ↑ 53 @westonruter
14 months ago

Replying to rabmalin:

Which would be good option?

I suggest option 1: augmenting postMessage transport with selective refresh. From the demo video, you can see this has the added benefit of allowing a JS implementation to do an approximate instant preview of a setting change while waiting for the Ajax response to return with the actual rendered contents. The JS preview logic is not required here.

#55 follow-up: @ryan
14 months ago

@westonruter Will the plugin repo be updated? Testing is much easier when the WP update mechanism can be used, especially now that Calypso and Shiny Updates allow auto update of plugins. https://wordpress.org/plugins/customize-partial-refresh/

#56 in reply to: ↑ 55 @westonruter
14 months ago

Replying to ryan:

@westonruter Will the plugin repo be updated? Testing is much easier when the WP update mechanism can be used, especially now that Calypso and Shiny Updates allow auto update of plugins. https://wordpress.org/plugins/customize-partial-refresh/

Once the PR gets merged. I wanted to try out an API incorporating the WP_Customize_Partial (or WP_Customize_Template_Part) which allows multiple settings to be associated with a rendering. In many ways, the class is looking similar to WP_Customize_Control, except the settings are associated for rendering instead of editing.

This ticket was mentioned in Slack in #core by mike. View the logs.


14 months ago

#58 @westonruter
14 months ago

In 36407:

Customizer: Improve parity between JS Setting models in preview with JS Setting models in pane.

  • Ensure that Setting Value objects in preview get initial _dirty flag set if values among POST data.
  • Upon saved event, send saved message to preview with the response to trigger saved event there.
  • Reset _dirty flag for all setting Value objects in preview upon saved.
  • Continue to create settings synced from pane even after initial bootstrap, and create them as dirty.
  • Ensure that id property is set for setting Value objects in preview.

See #27355.
Fixes #35616.

#59 @westonruter
14 months ago

In 36414:

Customizer: Export nonce, theme, and url app settings in preview as exported in pane.

  • Introduce WP_Customize_Manager::get_nonces() to consolidate logic for retrieving nonces.
  • Export nonces centrally in wp.customize.settings.nonce with each request and update nav menus preview to utilize.
  • Send updated nonces to preview upon nonce-refresh.
  • Request full preview refresh if Nav Menu selective refresh request fails (e.g. due to bad nonce).
  • Update nav menus and widgets in Customizer to utilize customize_refresh_nonces for exporting nonces and keeping them up to date.

See #27355.
Fixes #35617.

#60 @westonruter
14 months ago

Here's the latest work done to introduce the “Partial” construct to the Customizer APIs: https://github.com/xwp/wp-customize-partial-refresh/pull/18

Introduced are WP_Customize_Partial PHP class and a corresponding wp.customize.Partial in JS. Partials are registered in PHP via (eventually) $wp_customize->add_partial() and in JS via wp.customize.partial.create().

One interesting bit is how to standardize handling the JS-initialization of any dynamic content, such as Emojis or MediaElement.js players. It will presently copy some of the logic developed for Jetpack's Infinite Scroll, but it would be great of there was a standard event that JS could listen for to make the necessary DOM changes (adding events, replacing elements, etc), similar to the widget-added and widget-updated events that are triggered on document.

#61 @westonruter
14 months ago

Latest changes merged into feature branch PR: https://github.com/xwp/wp-customize-partial-refresh/pull/17

For another example for how to make use of this API, see this “Welcome Message” plugin which adds a TinyMCE rich text editor control to the Customizer with changes that get selectively-refreshed in the preview: https://gist.github.com/westonruter/ed4ae3f8b6f6d653e0c6

Quick demo video: https://www.youtube.com/watch?v=hb3uezGTzFc

Next steps include rounding off some rough edges, and reworking nav menus partial refreshing in Core to make use of this new API to make sure that it handles the (complicated) use case, and then to implement the even more complicated use case of widgets.

Last edited 14 months ago by westonruter (previous) (diff)

This ticket was mentioned in Slack in #core by westonruter. View the logs.


14 months ago

#63 @westonruter
14 months ago

Finally, a new version of the Customize Partial Refresh plugin has been released.

Please review the Core dev chat logs today: https://wordpress.slack.com/archives/core/p1455139493002289

I'm looking for feedback on compatibility between the feature plugin and your themes and especially any widgets that are available. I'm also looking for feedback on the API it provides. I will be writing up a post on Make Core that describes the API, but in the mean time you can look at the existing plugins referenced on the plugin readme or look at how partial refresh is implemented for nav menus and widgets in the plugin itself.

Note that the plugin requires 4.5-alpha, so please use the latest nightly to test.

#64 @ryankienstra
14 months ago

Tested On 8 Themes
All Default Widgets And Menus Work As Expected

@westonruter,
This feature plugin worked exactly as expected for the widgets and menus in the following themes in the WordPress.org theme directory:

Graphene: https://wordpress.org/themes/graphene/
Hueman: https://wordpress.org/themes/hueman/
Responsive: https://wordpress.org/themes/responsive/
Spacious: https://wordpress.org/themes/spacious/
Sparkling: https://wordpress.org/themes/sparkling/
Sydney: https://wordpress.org/themes/sydney/
Twenty Sixteen: https://wordpress.org/themes/twentysixteen/
Zerif Lite: https://wordpress.org/themes/zerif-lite/

These all have over 60,000 active downloads. This isn't to speak for the theme authors, though.

All of the default widgets worked as expected: "Archives," "Calendar," "Categories," "Custom Menu," "Meta," "Pages," "Recent Comments," "Recent Posts," "RSS," "Search," "Tag Cloud," and "Text." Also, the menus update instantly in the Customizer. The "Menu Location" control works well, as do the individual menu controls.

Using a control for a widget quickly produced a change in only the given widget in the preview iframe. This included the "select" element for the "Archives" and "Categories" widgets. I used the theme test data that wordpress.org recommends to test new themes: https://wpcom-themes.svn.automattic.com/demo/theme-unit-test-data.xml

As @westonruter showed in the video on this ticket, only the widget with the change was refreshed. It very briefly became lighter, and refreshed with the new content.

Plugin version: 0.5.4
WordPress version: 4.5-alpha-35776-src

This ticket was mentioned in Slack in #core by westonruter. View the logs.


13 months ago

#67 @westonruter
13 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

27355.0.diff ports the feature plugin into a core patch. I didn't include the unit tests in this patch, aside from changes to the existing unit tests for the feature.

Automated tests pass with patch applied: https://travis-ci.org/xwp/wordpress-develop/builds/110013004

The Selective Refresh functionality can be disabled entirely by filtering selective_refresh out of the array passed into the customize_loaded_components filter.

@DrewAPicture
13 months ago

Docs + standards audit

#68 @DrewAPicture
13 months ago

Added 27355.diff which covers the inline docs audit requested by @mikeschroder as well as some coding standards fixes.

So as not to hold up merge, I've opened #35855 to decide whether we should wrap isset( $this->manager->selective_refresh ) in a class method to DRY up some of the conditionals introduced here.

#69 @westonruter
13 months ago

  • Keywords needs-testing removed

OK, I've finished off the PHP unit tests as well as make a few additional tweaks as seen in the patches and GitHub compare views above. Note that the patch does not include QUnit tests since there are no tests at all for the Customizer preview currently. I've opened #35857 specifically to address that outside the scope of this ticket.

@ocean90 I'll commit the patch once you've had a look.

#70 @westonruter
13 months ago

  • Keywords has-unit-tests added

#71 @ocean90
13 months ago

  • Owner changed from westonruter to ocean90
  • Status changed from assigned to reviewing

#72 follow-ups: @ocean90
13 months ago

  • Owner changed from ocean90 to westonruter
  • Widgets: When adding a widget to an empty sidebar I get a JS error:
Uncaught TypeError: this.targetWindow is not a function @ customize-base.js?ver=4.5-alpha-35776-src-1455792213:707 
api.Messenger.api.Class.extend.receive @ customize-base.js?ver=4.5-alpha-35776-src-1455792213:707
n.isFunction.d	@ jquery.js?ver=1.12.0:2
n.event.dispatch @ jquery.js?ver=1.12.0:3
r.handle @ jquery.js?ver=1.12.0:3
  • Nav Menus: When I enter "Foo<" into the input field for a menu item title I get a PHP warning:
Object {partial: "nav_menu_instance[c306e5285ebf74fc00b87f1d9b839d2a]", error_number: 8, error_string: "Uninitialized string offset: 1", error_file: "/srv/www/wp-develop/svn/src/wp-includes/formatting.php", error_line: 377} @ customize-selective-refresh.js?ver=4.5-alpha-35776-src-1455794958:604 
Object {partial: "nav_menu_instance[c306e5285ebf74fc00b87f1d9b839d2a]", error_number: 8, error_string: "Uninitialized string offset: 1", error_file: "/srv/www/wp-develop/svn/src/wp-includes/formatting.php", error_line: 377} @ customize-selective-refresh.js?ver=4.5-alpha-35776-src-1455794958:604 

(Note: It seems like something is called twice on menu updates.)

  • In WP_Customize_Nav_Menus::filter_wp_nav_menu_args() some docs for $can_partial_refresh might be helpful.
  • WP_Customize_Widgets::filter_wp_kses_allowed_data_attributes() should have the public keyword and not @access private.
  • In WP_Customize_Widgets::end_dynamic_sidebar() let's remove the assert() line.
  • Running all unit tests dies at ~17%: Starting test 'Tests_DB::test_bail'. Database is dead.. It seems like Test_WP_Customize_Selective_Refresh_Ajax is the culprit because it runs on the default group and defines DOING_AJAX.

Other notes:

#73 in reply to: ↑ 72 ; follow-up: @westonruter
13 months ago

Replying to ocean90:

  • Nav Menus: When I enter "Foo<" into the input field for a menu item title I get a PHP warning:
Object {partial: "nav_menu_instance[c306e5285ebf74fc00b87f1d9b839d2a]", error_number: 8, error_string: "Uninitialized string offset: 1", error_file: "/srv/www/wp-develop/svn/src/wp-includes/formatting.php", error_line: 377} @ customize-selective-refresh.js?ver=4.5-alpha-35776-src-1455794958:604 
Object {partial: "nav_menu_instance[c306e5285ebf74fc00b87f1d9b839d2a]", error_number: 8, error_string: "Uninitialized string offset: 1", error_file: "/srv/www/wp-develop/svn/src/wp-includes/formatting.php", error_line: 377} @ customize-selective-refresh.js?ver=4.5-alpha-35776-src-1455794958:604 

I grabbed a stack trace for when this error occurs and I found:

#0 /srv/www/wordpress-develop/src/wp-includes/formatting.php(377): WP_Customize_Selective_Refresh-&gt;handle_error(8, 'Uninitialized s...', '/srv/www/wordpr...', 377, Array)
#1 /srv/www/wordpress-develop/src/wp-includes/formatting.php(240): _wptexturize_pushpop_element('&lt;', Array, Array)
#2 [internal function]: wptexturize('Fosssos&lt;')
#3 /srv/www/wordpress-develop/src/wp-includes/plugin.php(235): call_user_func_array('wptexturize', Array)
#4 /srv/www/wordpress-develop/src/wp-includes/nav-menu-template.php(164): apply_filters('the_title', 'Fosssos&lt;', 4946)
#5 [internal function]: Walker_Nav_Menu-&gt;start_el('&lt;li id=&quot;menu-it...', Object(WP_Post), 0, Object(stdClass))
#6 /srv/www/wordpress-develop/src/wp-includes/class-wp-walker.php(146): call_user_func_array(Array, Array)
#7 /srv/www/wordpress-develop/src/wp-includes/class-wp-wa in <b>/srv/www/wordpress-develop/src/wp-includes/customize/class-wp-customize-selective-refresh.php</b> on line <b>283</b><br />

As such, it looks like it is specifically an issue with wptexturize and not with selective refresh or nav menus in the customizer. If you save the menu item and then exit the Customizer, you'll see the same PHP notice if WP_DEBUG_DISPLAY is enabled. This patch fixes the issue:

  • src/wp-includes/formatting.php

    function wptexturize_primes( $haystack, $needle, $prime, $open_quote, $close_quo 
    374374 */ 
    375375function _wptexturize_pushpop_element( $text, &$stack, $disabled_elements ) { 
    376376        // Is it an opening tag or closing tag? 
    377         if ( '/' !== $text[1] ) { 
     377        if ( isset( $text[1] ) && '/' !== $text[1] ) { 
    378378                $opening_tag = true; 
    379379                $name_offset = 1; 
    380380        } elseif ( 0 == count( $stack ) ) { 

(Note: It seems like something is called twice on menu updates.)

@ocean90 Do you have the same menu appearing in multiple places, such as assigned to a nav menu location and to a Custom Menu widget? If so, then it is expected that the call to render would happen multiple times since there would be multiple placements.

#74 @westonruter
13 months ago

Filed #35864 to address the issue with wptexturize().

#75 in reply to: ↑ 73 @ocean90
13 months ago

Replying to westonruter:

@ocean90 Do you have the same menu appearing in multiple places, such as assigned to a nav menu location and to a Custom Menu widget?

Yeah, you're right. Twenty Sixteen includes the primary menu in the footer too.

#76 in reply to: ↑ 72 @westonruter
13 months ago

Replying to ocean90:

  • Widgets: When adding a widget to an empty sidebar I get a JS error:

I've seen this before as well in certain situations but didn't patch it yet. The issue can be easily reproduced even without selective refresh by adding a button in the preview which does:

wp.customize.preview.send( 'refresh' );
wp.customize.preview.send( 'refresh' );

This will also trigger the error.

I've filed a ticket for this and patched it in #35866.

The alternative fix, to address the symptom instead of the underlying cause, is to debounce the requestFullRefresh function:

  • src/wp-includes/js/customize-selective-refresh.js

    wp.customize.selectiveRefresh = ( function( $, api ) { 
    507507         * 
    508508         * @since 4.5.0 
    509509         */ 
    510         self.requestFullRefresh = function() { 
     510        self.requestFullRefresh = _.debounce( function() { 
    511511                api.preview.send( 'refresh' ); 
    512         }; 
     512        } ); 
    513513 
    514514        /** 
    515515         * Request a re-rendering of a partial. 

But better to fix the underlying issue instead.

Last edited 13 months ago by westonruter (previous) (diff)

#77 @westonruter
13 months ago

In 36583:

Customize: Prevent consecutive refresh requests from preview from causing JS error.

Fixes "Uncaught TypeError: this.targetWindow is not a function".

See #27355.
Fixes #35866.

#78 in reply to: ↑ 72 @westonruter
13 months ago

Replying to ocean90:

  • In WP_Customize_Nav_Menus::filter_wp_nav_menu_args() some docs for $can_partial_refresh might be helpful.
  • WP_Customize_Widgets::filter_wp_kses_allowed_data_attributes() should have the public keyword and not @access private.
  • In WP_Customize_Widgets::end_dynamic_sidebar() let's remove the assert() line.

Done in 27355.4.diff, including the moving of the refresh message receiving from customize-nav-menus.js to the main customize-controls.js file.

  • Running all unit tests dies at ~17%: Starting test 'Tests_DB::test_bail'. Database is dead.. It seems like Test_WP_Customize_Selective_Refresh_Ajax is the culprit because it runs on the default group and defines DOING_AJAX.

I'm trying to figure out why this is happening.

I'll see if I can fix these as well.

#79 in reply to: ↑ 72 @westonruter
13 months ago

Replying to ocean90:

  • Running all unit tests dies at ~17%: Starting test 'Tests_DB::test_bail'. Database is dead.. It seems like Test_WP_Customize_Selective_Refresh_Ajax is the culprit because it runs on the default group and defines DOING_AJAX.
  • Nav Menus: Users with the unfiltered_html cap can break the customizer, the same happens without the patch too, see https://gist.github.com/ocean90/db41b33785679b85c73a

These are addressed in 27355.6.diff. As in the screenshots above, when a partial's content is rendered the parsing of the HTML is wrapped in a try/catch block, and in the case of an error being thrown the error will be displayed at the end of the partial's container. In addition to a JS script error, it also captures document.write() calls which likewise blow away the preview.

  • Nav Menus: Probably unrelated: Assign an author the edit_theme_options cap and enter "foo'foo" into the input field for a menu item title: For some reasons the preview will have slashes. (https://cloudup.com/ccKXry0gjDC)

Looking at this now.

#80 in reply to: ↑ 72 ; follow-up: @westonruter
13 months ago

  • Keywords commit has-screenshots added
  • Status changed from reviewing to accepted

Replying to ocean90:

  • Nav Menus: Probably unrelated: Assign an author the edit_theme_options cap and enter "foo'foo" into the input field for a menu item title: For some reasons the preview will have slashes. (https://cloudup.com/ccKXry0gjDC)

I narrowed this down to the following line in WP_Customize_Nav_Menu_Item_Setting::sanitize():

<?php
$menu_item_value['title'] = apply_filters( 'title_save_pre', $menu_item_value['title'] );

The the wp_filter_kses function is adding the slash which applies on this title_save_pre filter. For some reason, the function does:

<?php
return addslashes( wp_kses( stripslashes( $data ), current_filter() ) );

Which is the reason for the slash being injected, because addslashes() adds slashes before apostrophes, even though there wasn't a slash that got stripped originally by stripslashes. So, to me this function and (wp_filter_post_kses like it) looks like it is doing the wrong thing. The easiest way I see to fix the issue is to bypass those KSES functions altogether with something like nav-menu-item-kses-filter-fix.diff.

All of this to say, no, it is not related to the new selective refresh functionality being introduced here.

Adding commit keyword, and I'll commit in ~12 hours, unless anything else is identified needing to be fixed prior to commit.

#81 in reply to: ↑ 80 @ocean90
13 months ago

Replying to westonruter:

All of this to say, no, it is not related to the new selective refresh functionality being introduced here.

Moved into #35869.

Adding commit keyword, and I'll commit in ~12 hours, unless anything else is identified needing to be fixed prior to commit.

No objections. Only a minor thing which can happen on commit.

#82 @westonruter
13 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 36586:

Customize: Add selective refresh framework with implementation for widgets and re-implementation for nav menus.

See https://make.wordpress.org/core/2016/02/16/selective-refresh-in-the-customizer/.

Props westonruter, valendesigns, DrewAPicture, ocean90.
Fixes #27355.

#83 @westonruter
13 months ago

In 36623:

Customize: Ensure dynamic_sidebar() finishes with removing the sidebar ID from the current_dynamic_sidebar_id_stack.

This ensures that widgets appearing after a nested sidebar will continue to be selective refreshable.

See #27355.

#84 @westonruter
13 months ago

In 36624:

Customize: Let WP_Customize_Selective_Refresh class be final to match manager and other component classes.

This class is not intended to be extended.

See #27355.

#85 @westonruter
13 months ago

In 36643:

Customize: Skip exporting partials to client and handling rendering requests if user can't modify associated settings.

Introduces WP_Customize_Partial::check_capabilities() for parity with WP_Customize_Control::check_capabilities().

See #27355.
Fixes #35914.

#86 @westonruter
13 months ago

In 36689:

Customize: Allow controls to be registered without any associated settings.

  • Improves parity between partials and controls. A partial or control can be settingless if instantiated with settings param as empty array (otherwise, if null, then the partial/control ID is used).
  • Eliminate need to create dummy settings that serve no purpose except to place a control in the UI.
  • Removes dummy settings for create_new_menu and new_menu_name.
  • Introduces WP_Customize_Control::$capability and WP_Customize_Partial::$capability, and if set checks them in the respective check_capabilities() methods.
  • Prevents PHP fatal error from happening when non-existing settings are provided to control: "Call to a member function check_capabilities() on a non-object".
  • Fixes issue where nav menu items and widgets were no longer working with selective refresh because cap check was failing.

See #27355.
Fixes #35926.

#87 @boonebgorges
13 months ago

In 36784:

More specific test for a bad callback in WP_Customize_Partial test.

The 'render_callback' passed to WP_Customize_Partial must either echo or
return a result - not both. When it's detected that the callback echoes and
returns a value, the return value takes precedence. This is now reflected in
the corresponding unit test.

Introduced in [36586].

See #27355. See #36016.

#88 @westonruter
13 months ago

In 36797:

Customize: Use selective refresh to preview changes to site title and tagline in core themes.

Fixes issue where wptexturize and other filters fail to apply when previewing changes via postMessage transport.

See #27355.
Fixes #33738.

#89 @westonruter
13 months ago

In 36801:

Customize: Define params on WP_Customize_Partial::render_callback() for the sake of subclasses overriding this method.

Fixes a strict standards notice regarding the method signature needing to be compatible.

See #27355.

#90 @DrewAPicture
13 months ago

In 36835:

Docs: Improve documentation for WP_Customize_Nav_Menus::filter_nonces(), introduced in [36414].

See #27355, #35617. See #35986.

#91 @DrewAPicture
13 months ago

In 36836:

Docs: Improve DocBlock syntax for WP_Customize_Nav_Menus::customize_dynamic_partial_args(), introduced in [36586].

See #27355. See #35986.

#92 @DrewAPicture
13 months ago

In 36841:

Docs: Improve documentation for WP_Customize_Widgets::customize_dynamic_partial_args(), introduced in [36586].

See #27355. See #35986.

#93 @westonruter
13 months ago

In 36889:

Customize: Fix regressions and harden implementation of selective refresh for nav menus.

  • Request full refresh if there are nav menu instances that lack partials for a changed setting.
  • Restore WP_Customize_Nav_Menus::$preview_nav_menu_instance_args and WP_Customize_Nav_Menus::export_preview_data() from 4.3, and keeping a tally of all wp_nav_menu() calls regardless of whether they can use selective refresh.
  • Ensure that all instances of wp_nav_menu() are tallied, regardless of whether they are made during the initial preview call or during subsequent partial renderings. Export nav_menu_instance_args with each partial rendering response just as they are returned when rendering the preview as a whole.
  • Fix issues with Custom Menu widget where nav menu items would fail to render when switching between menus when a menu lacked items to begin with.
  • Make sure the fallback behavior is invoked when the partial is no longer associated with a menu.
  • Do fallback behavior to refresh preview when all menu items are removed from a menu.

Follows [36586].
See #27355.
Fixes #35362.

#94 @westonruter
13 months ago

In 36890:

Customize: Remove selective refresh error message from appearing inline within the preview.

The error message will still be available in the developer console.

Removes part of [36586].
See #27355.
Fixes #36164.

#95 @westonruter
13 months ago

In 36990:

Customize: Rely on selective refresh exclusively for previewing custom logo changes.

Eliminates JS logic (from [36698]) which attempted to do pure JS update while waiting for the selective refresh response to return. The duplicate JS logic lacked a re-implementation of the image_downsize() functionality available in PHP, and so the JS preview logic would fail to properly preview images that didn't have the exact theme image size generated. To keep the code DRY and to eliminate the momentary display of an improperly-sized image, the duplicated JS logic is now removed.

See #27355.
See #33755.
Fixes #36096.

#96 @DrewAPicture
12 months ago

In 37009:

Docs: Standardize the file header and class summaries for the file containing WP_Customize_Partial, introduced in [36586].

See #27355. See #35986.

#97 @DrewAPicture
12 months ago

In 37010:

Docs: Standardize the file header and class summarries for the file containing WP_Customize_Selective_Refresh, introduced in [36586].

See #27355. See #35986.

#98 @DrewAPicture
12 months ago

In 37011:

Docs: Add a missing version and access information to the DocBlock for the WP_Customize_Selective_Refresh->$manager property, introduced in [36586].

See #27355. See #35986.

#99 @westonruter
12 months ago

In 37040:

Customize: Require opt-in for selective refresh of widgets.

  • Introduces customize-selective-refresh-widgets theme support feature and adds to themes.
  • Introduces customize_selective_refresh arg for WP_Widget::$widget_options and adds to all core widgets.
  • Remove selective_refresh from being a component that can be removed via customize_loaded_components filter.
  • Add WP_Customize_Widgets::get_selective_refreshable_widgets() and WP_Customize_Widgets::is_widget_selective_refreshable().
  • Fix default selector for Partial instances.
  • Implement and improve Masronry sidebar refresh logic in Twenty Thirteen and Twenty Fourteen, including preservation of initial widget position after refresh.
  • Re-initialize ME.js when refreshing Twenty_Fourteen_Ephemera_Widget.

See #27355.
Fixes #35855.

#100 @westonruter
12 months ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core by mike. View the logs.


9 months ago

#102 @westonruter
33 hours ago

In 40316:

Customize: Fix selective refresh when customizing the 404 template.

Overrides the 404 status during partial refresh requests to serve back 200 so that the request is not deemed a failure and invoke the fallback behavior (full refresh).

See #27355.
Fixes #40018.

Note: See TracTickets for help on using tickets.