Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#35855 closed task (blessed) (fixed)

Let selective refresh component be required but be opt-in for sidebars/widgets (for now)

Reported by: drewapicture's profile DrewAPicture Owned by: westonruter's profile westonruter
Milestone: 4.5 Priority: normal
Severity: blocker Version: 4.6
Component: Customize Keywords: has-patch needs-testing
Focuses: Cc:

Description (last modified by westonruter)

Instead of adding a method to determine whether Selective Refresh (#27355) is active, it can instead be always active but specific components can opt-in for support as required: specifically widgets.

Currently isset( $this->manager->selective_refresh ) is used to choose between the 'postMessage' and 'refresh' transports in a bunch of different places.

Instead, there can be specific theme support and widget opt-in for selective refresh. See code in the original Widget Customizer plugin for reference: https://github.com/xwp/wp-widget-customizer/compare/without-partial-previews


For details, see Make Core post: Implementing Selective Refresh Support for Widgets

Attachments (7)

35855.diff (7.4 KB) - added by DrewAPicture 9 years ago.
35855.0.diff (59.9 KB) - added by westonruter 9 years ago.
https://github.com/xwp/wordpress-develop/pull/144
35855.1.diff (60.0 KB) - added by westonruter 9 years ago.
Refresh patch
35855.2.diff (71.7 KB) - added by westonruter 9 years ago.
35855.3.diff (71.4 KB) - added by westonruter 9 years ago.
35855.4.diff (71.4 KB) - added by westonruter 9 years ago.
35855.5.diff (78.3 KB) - added by westonruter 9 years ago.
https://github.com/xwp/wordpress-develop/compare/2916ef2...36d0bd9

Download all attachments as: .zip

Change History (64)

#1 @westonruter
9 years ago

Yeah, this or maybe WP_Customize_Manager::has_selective_refresh() as a shorthand for isset( $wp_customize->selective_refresh ).

@DrewAPicture
9 years ago

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


9 years ago

#3 @ocean90
9 years ago

  • Owner set to DrewAPicture
  • Status changed from new to assigned

#4 @westonruter
9 years ago

I suggest this is not an enhancement but blessed if it is to implement the facility for themes and plugins to opt-in to widget support for selective refresh. This was talked about being enabled near RC.

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


9 years ago

#6 @westonruter
9 years ago

  • Description modified (diff)
  • Summary changed from DRY up checking for selective refresh support in WP_Customize_Manager to Let selective refresh component be required but be opt-in for sidebars/widgets (for now)
  • Version set to trunk

#7 @westonruter
9 years ago

  • Type changed from enhancement to task (blessed)

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


9 years ago

#9 @DrewAPicture
9 years ago

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

#10 @westonruter
9 years ago

  • Description modified (diff)
  • Keywords needs-patch added; dev-feedback removed
  • Status changed from reviewing to accepted

#11 @westonruter
9 years ago

  • Keywords has-patch added; needs-patch removed

@DrewAPicture take a look at 35855.0.diff to see what I have in mind.

Themes add support for selective refresh of sidebars via:

<?php
add_theme_support( 'customize-selective-refresh-widgets' );

Then individual widgets opt-in for selective refresh via a new customize_selective_refresh widget option:

<?php
class Foo_Widget extends WP_Widget {
        /* ... */
        
        public function __construct() {
                $widget_ops = array(
                        'classname' => 'widget_foo',
                        'description' => __( 'So much foo[d].' ),
                        'customize_selective_refresh' => true,
                );
                parent::__construct( 'foo', __( 'Foo[d]' ), $widget_ops );
        }

Alternatively, a plugin can opt-in a theme and widget for selective refresh via:

<?php
add_theme_support( 'customize-selective-refresh-widgets', 'foo' );

The second argument is the widget id_base that support is being added for.

Thoughts?

Twenty Sixteen support via PR: https://github.com/WordPress/twentysixteen/pull/430
I didn't yet add support for Twenty_Eleven_Ephemera_Widget or Twenty_Fourteen_Ephemera_Widget.

Last edited 9 years ago by westonruter (previous) (diff)

#13 follow-up: @DrewAPicture
9 years ago

Help me understand the downside(s) of simply turning it on for all widgets by default? I guess I don't see why we'd want to implement selective refresh for some things and not other things.

It's like introducing a new feature and turning it off by default – which usually begs the question of why we're adding it in the first place.

#14 in reply to: ↑ 13 @westonruter
9 years ago

Replying to DrewAPicture:

Help me understand the downside(s) of simply turning it on for all widgets by default? I guess I don't see why we'd want to implement selective refresh for some things and not other things.

It's like introducing a new feature and turning it off by default – which usually begs the question of why we're adding it in the first place.

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

#15 @DrewAPicture
9 years ago

Yeah, OK.

In that case, I think theme support is the way to go, though I'd probably break it down a little bit in the name of forward compatibility, i.e. assuming there might be a future need to selectively selective refresh.

In that vein, adding post formats support comes to mind, with a twist.

What would you think about something along the lines of this:

<?php
// Single component (global on).
add_theme_support( 'customize-selective-refresh', array( 'widgets' ) );

// Multiple components (global on).
add_theme_support( 'customize-selective-refresh', array( 'widgets', 'other-thing' ) );


// Specific widgets by `id_base`.
add_theme_support( 'customize-selective-refresh', array(
        'widgets' => array( 'foo', 'bar' )
) );

// Specific widgets by `id_base` plus global 'other-thing'.
add_theme_support( 'customize-selective-refresh', array(
        'widgets' => array( 'foo', 'bar' ),
        'other-thing'
) );

Either way, the 'customize_selective_refresh' argument could independently override the "global on or off" via theme support, right?

The big thing here is that I'm not sure we want to introduce a theme support for a specific component for a specific feature ('selective-refresh'). This approach leaves the door open if something else comes along later.

Thoughts?

#16 follow-up: @westonruter
9 years ago

@DrewAPicture I think that makes sense. But nav-menus currently don't require theme support. Should theme support for them be on by default, with a theme needing to then explicitly remove_theme_support( 'customize-selective-refresh', 'nav-menus' )? the problem with this is that you can't specify sub-features when removing theme support, so the above would blow away not only nav-menus but also widgets and everything else. That's partially why I think the current Theme Support API kinda requires that the component be part of the theme support feature.

The other thing to consider here is that we talked in our core dev meeting about removing the opt-in for widgets in the future, making them all selective refreshed once authors have a chance to add support as required. In that case, we'd just remove the customize-selective-refresh-widgets theme support and be done with it.

#17 @westonruter
9 years ago

@mikeschroder thoughts on this?

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


9 years ago

@westonruter
9 years ago

Refresh patch

#19 @westonruter
9 years ago

The patch has a lot of whitepace diffs due to indentation changes. Here is the patch with whitespace differences suppressed: https://github.com/xwp/wordpress-develop/pull/144/files?w=1

#20 @westonruter
9 years ago

Here's a PR for implementing selective refresh for Jetpack's widgets for Twitter Timeline, Facebook Like Box, and Contact Info: https://github.com/Automattic/jetpack/pull/3543/files

Note that this PR includes some refactoring as well, to let the the shortcodes and widgets re-use the same codebase. The key changes to ensure selective refresh compatibility are:

  1. Enqueue any JS and CSS dependencies for the widget if is_customize_preview(), not only if is_active_widget() in a sidebar. If this is not done, then any JS dependencies would need lazy-loaded when the first widget is added (which is, in fact, what the Customize Partial Refresh feature plugin's Jetpack compat script did).
  2. Add a handler for partial-content-rendered selective refresh events to rebuild a widget's dynamic elements when it is re-rendered. See example.
  3. Add a handler for partial-content-moved selective refresh events to refresh partials if any dynamic elements involve iframes that are have dynamically-written documents (such as the Twitter Timeline widget). See example. (Adding this event handler should normally not be required.)

As seen from adding selective refresh support for some of Jetpack's widgets, I believe this demonstrates the need for selective refresh of widgets to be opt-in. If the above steps are not taken, then when a widget is added or updated, it will appear broken (as if JS is turned off).

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


9 years ago

#22 in reply to: ↑ 16 @obenland
9 years ago

Have you considered making it a WP_Customize_Manager property that themes can enable? Or have them enable it by changing the transport method, like default themes do with site title and description?

#23 @westonruter
9 years ago

@obenland changing the transport would work well if there was a single setting to flip over to postMessage. The problem is it's a whole range of settings that need to change the default transport for. As for a WP_Customize_Manager flag… perhaps, but it could get tricky to add the hook early enough, like:

<?php
function twentytwenty_customize_widgets_selective_refresh_optin( $wp_customize ) {
    if ( isset( $wp_customize->widgets ) ) {
        $wp_customize->widgets->selective_refresh_support = true;
    }
}
add_action( 'customize_register', 'twentytwenty_customize_widgets_selective_refresh_optin' );

Something like that? Maybe this would better in terms of encapsulation and reducing globals, but it seems an undue burden for themers vs:

<?php
add_theme_support( 'customize-selective-refresh-widgets' );

#24 follow-up: @obenland
9 years ago

I'm not a fan of the add_theme_support() route for a variety of reasons:

  1. Introducing a new call means starting from 0 (okay, 5) in terms of theme adoption. 5 is a lot less than all the themes. Have we exhausted all options to enable it by default and be clever about maybe disabling it for themes that do funky things with their sidebars? While the theme support route is very convenient and makes things very easy in terms of backwards compatibility, we pay a high price with waiving adoption.
  2. Like @DrewAPicture pointed out in 15, the proposed name is not really future proof. It actually makes it more like for the influx of more calls to be a problem because other Customizer features that might require a support declaration would add their own.
  3. The model is not really sustainable in the long run. Default themes come with at least three features they need to add support for (this would be the fourth), there are themes like Confit that already have 9 (!) calls.
  4. The name is not very elegant, to a degree where I think it might even affect adoption.

To be a bit more constructive: If adding a flag on customize_register is not an option (is it really not?) and we do decide to go the theme support route, may I suggest the following syntax:

<?php
// General support.
add_theme_support( 'customizer', 'selective-refresh' );

// Specific widgets by `id_base`.
add_theme_support( 'customizer', array(
        'selective-refresh' => array( 'foo', 'bar' ),
) );

#25 in reply to: ↑ 24 @westonruter
9 years ago

Thanks for the thoughtful feedback!

Replying to obenland:

Have we exhausted all options to enable it by default and be clever about maybe disabling it for themes that do funky things with their sidebars? While the theme support route is very convenient and makes things very easy in terms of backwards compatibility, we pay a high price with waiving adoption.

I'd love to find a solution here. The only thing that comes to mind is sniffing for whether widgets in a sidebar have inline style attributes attached to them. This could be a signal that a theme is doing something funky?

To be a bit more constructive: If adding a flag on customize_register is not an option (is it really not?)
[…]
The name is not very elegant, to a degree where I think it might even affect adoption.

It is an option. But it just seems 10× less elegant than adding a new theme support feature, where a theme support is much easier to add if it is needed by all themes.

Like @DrewAPicture pointed out in 15, the proposed name is not really future proof. It actually makes it more like for the influx of more calls to be a problem because other Customizer features that might require a support declaration would add their own. ¶ The model is not really sustainable in the long run. Default themes come with at least three features they need to add support for (this would be the fourth), there are themes like Confit that already have 9 (!) calls.

Part of the thinking for a specific customize-selective-refresh-widgets feature name is the idea that the need for themes adding support would be temporary. This specific feature name could be added in 4.5 with the explicit advisory that the feature will be enabled by default in future releases, to give theme and plugin authors time to ensure they're compatible.

and we do decide to go the theme support route, may I suggest the following syntax:

<?php
// General support.
add_theme_support( 'customizer', 'selective-refresh' );

// Specific widgets by `id_base`.
add_theme_support( 'customizer', array(
        'selective-refresh' => array( 'foo', 'bar' ),
) );

Widgets are the only thing that seem to need theme support. This is why I shy away from breaking down the name into separate parts like customizer, selective-refresh, and widgets. Nav menu selective refresh is currently enabled for all themes, as is selective refresh for custom logos. If the theme support feature flag is removed/deprecated in a future release, then this would leave a customizer root theme option without any children.

#26 @MH Themes
9 years ago

Hi,

selective refresh for widgets is quite a major change as this possibly will break thousands of themes out there, at least in the customizer. Of course it isn't much of an issue for the default themes, but as you know, there are plenty of much more complex themes out there with JS based widgets like sliders, social widgets or else.

As it is now in beta 3, selective refresh for widgets is enabled by default and that leads to a situation where all widgets with JS functionality don't work anymore in the customizer out of the box. There is a pretty good chance that many theme authors won't have this change on their radar, at least in short term for WP 4.5 and enabling this by default could cause some confusing and also lots of support requests from users who wonder why their widgets don't appear anymore in the customizer after updating to WP 4.5.

I think the best route to take would be to not enable selective refresh for widgets out of the box and then give theme authors a way to enable it for their widgets that are compatible with selective refresh. That would ensure a smooth transition as eventually most theme authors will add compatibility over time to keep their themes competitive and ensure that their themes work well with the customizer. Enabling it by default could be quite an adventure - at least short term. :-)

#27 @westonruter
9 years ago

Here is another example for how to implement selective refresh for a widget, here for Jetpack's Contact Info widget:

wp.customize.selectiveRefresh.bind( 'partial-content-rendered', function( placement ) {
        if ( placement.partial.widgetId && /^widget_contact_info-\d+$/.test( placement.partial.widgetId ) ) {
                setupContactMaps( placement.container );
        }
} );

Note the setupContactMaps function here gets called at jQuery ready and sets up the entire document.body, but then here you can see the plugin code allows the root element to be passed in to scope the re-initialization logic.

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


9 years ago

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


9 years ago

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


9 years ago

#31 follow-up: @karmatosed
9 years ago

I will fully admit to not liking the path we seem to be going down a bit of 'put it in the theme'. Any amazing feature should be amazing enough to not require themes to add code to support it. If it's truly good for all then it should be. I get that is a rather idealistic perspective though.

The facts are this feature is there and without add_theme_support we are potentially going to break so much. I really am torn. The only way I can be slightly ok with this would be for it to revert from opt in to opt out in a few releases or so as @westonruter you mentioned in Slack. If we had more time I'd be suggested the other way around and giving themers time to implement. We unfortunately don't have that option.

I assume we need to patch at least one default theme because of this too.

#32 @kirasong
9 years ago

  • Severity changed from normal to blocker

#33 in reply to: ↑ 31 ; follow-up: @westonruter
9 years ago

Replying to karmatosed:

I will fully admit to not liking the path we seem to be going down a bit of 'put it in the theme'. Any amazing feature should be amazing enough to not require themes to add code to support it. If it's truly good for all then it should be. I get that is a rather idealistic perspective though.

Note that postMessage transport previewing of changes in the Customizer is also an amazing feature, but themes all have to add explicit support for that as well. So it is a similar situation.

I assume we need to patch at least one default theme because of this too.

Yes. This is included in 35855.1.diff and Twenty Sixteen via https://github.com/WordPress/twentysixteen/pull/430

#34 in reply to: ↑ 33 ; follow-up: @obenland
9 years ago

Replying to westonruter:

Replying to karmatosed:
Note that postMessage transport previewing of changes in the Customizer is also an amazing feature, but themes all have to add explicit support for that as well. So it is a similar situation.

That sounds like an argument to let themes add selective refresh support the same way they add postMessage support. Which might not be the worst compromise.

#35 in reply to: ↑ 34 @westonruter
9 years ago

Replying to obenland:

That sounds like an argument to let themes add selective refresh support the same way they add postMessage support. Which might not be the worst compromise.

I meant in the same what they that have to do “something” to add support, not specifically the via the mechanics of adding an additional ...->transport = 'postMessage' line to their theme code. That's not applicable because opt-in for selective refresh of widgets would add postMessage transport to _all_ widget-related settings, as well as being the default transport for new widgets added. But since individual widgets also have to indicate support to be eligible for selective refresh, the transport can only added once both requirements are satisfied. So this is why a more meta-level flag is needed, like a theme support feature.

#36 @davidakennedy
9 years ago

I have to say that I'm not a huge fan of either of these options. Those are:

  1. Opt in : Themes aren't potentially broken but we have yet another add_theme_support().
  2. Opt out: Themes are potentially borked but we don't have another option.

Neither is elegant and worry-free, but I think I understand where we're at right now. I don't think it's fair to potentially break a lot of themes when there hasn't been much warning or visibility on that potential breakage. So I would side for an opt in with the add_theme_support(). I dislike it, not just for the reasons Obenland mentioned in ticket:35855#comment:24, but also:

  • It adds another option themers need to think about.
  • It's not something that seems very black and white, and easy to grasp why you may need it, unless you're familiar with the Customizer already. So it may be misunderstood or misused by some.

I do like the idea of making this default after a reasonable time period so themers can adjust accordingly.

#37 follow-up: @grapplerulrich
9 years ago

I read the explanation on the make post.

If I understand it correctly we are defining if that the widget is selective refresh ready using $widget_ops in the WP_Widget class. Could we not use register_sidebar() to define if the sidebar is selective refresh ready?

#38 in reply to: ↑ 37 @westonruter
9 years ago

Replying to grapplerulrich:

If I understand it correctly we are defining if that the widget is selective refresh ready using $widget_ops in the WP_Widget class. Could we not use register_sidebar() to define if the sidebar is selective refresh ready?

There are two separate supports flags needed:

  1. A theme needs to indicate it supports selective refresh for the sidebars that it defines. Consider Twenty Thirteen which uses Masonry for the footer sidebar. For this I am proposing the add_theme_support() call. It's true that this could be defined at the individual sidebar level as a new argument passed into register_sidebar(), but I should think that “selectively” implementing selective refresh for one sidebar but not another wouldn't be something we want to encourage?
  2. A widget needs to indicate it supports selective refresh in the common case for widgets that have dynamic elements that have to be initialized with JavaScript, such as the Twitter Timeline, Facebook Like Box, and Contact Info widgets in Jetpack. This is what the $widget_options argument for WP_Widget is proposed for.

So selective refresh would be available for previewing a specific widget change only if these two conditions are met.

#39 @celloexpressions
9 years ago

It could actually work to do selective refresh opt-in on a per-sidebar level. For example, if a theme with multiple sidebars can easily support selective refresh for one, but needs more work for another, they could add support just for the one sidebar for now.

I almost lean towards making it opt-out and making a big push to educate theme developers about how to address any issues, like with nav menus. There were some issues there, as there will be here, but I'm concerned about getting adequate adoption unless there's some aspect of forcing themes to adjust. There are plenty of reasons not to do that as well, though.

#40 @MH Themes
9 years ago

I think making it opt-out (at least in case of WP 4.5) is a no-go and would cause quite a mess as this will potentially break thousands of themes and possibly many theme authors won't have this on their radar at the moment and implementing also takes time. If this is being considered, then it would probably make sense to at least wait until 4.6 and ensure that this is extensively announced (and educated) so that theme developers have time to dig into this and make necessary changes.

Especially in case of widgets this can have quite an impact. Widgets can basically do anything and there are lots of scenarios where widgets won't be compatible anymore out of the box. I think this is different to the previous nav menus implementation, as widgets can be much more complex.

#41 @westonruter
9 years ago

There is a certain elegance to letting the theme opt-in be an argument on register_sidebar() as it would be more closely aligned with how a WP_Widget is proposed to be defined with support:

  class WP_Widget_Pages extends WP_Widget {
  
        public function __construct() {
                $widget_ops = array(
                        'classname' => 'widget_pages',
                        'description' => __( 'A list of your site&#8217;s Pages.' ),
+                       'customize_selective_refresh' => true,
                );
                parent::__construct( 'pages', __('Pages'), $widget_ops );
        }
  }

...

  register_sidebar( array(
        'name' => __( 'Widget Area', 'twentyfifteen' ),
        'id' => 'sidebar-1',
        'description' => __( 'Add widgets here to appear in your sidebar.', 'twentyfifteen' ),
        'before_widget' => '<aside id="%1$s" class="widget %2$s">',
        'after_widget' => '</aside>',
        'before_title' => '<h2 class="widget-title">',
        'after_title' => '</h2>',
+       'customize_selective_refresh' => true,
  ) );

So there is this option using register_sidebar(), and then the option for using add_theme_support():

add_theme_support( 'customize-selective-refresh-widgets' );

Although, the identifier customize-selective-refresh-widgets vs customize_selective_refresh isn't set in stone either.

#42 @westonruter
9 years ago

I'd like to commit the consensus approach tomorrow (Friday) so that I can publish a Make Core post with field guide notes on how to implement selective refresh for themes and widgets.

#43 @westonruter
9 years ago

@karmatosed thoughts on the two approaches for indicating sidebar support in a theme?

#44 @celloexpressions
9 years ago

41 (with per-widget and per-sidebar opt-in) sounds like the best option to me for now.

#45 @karmatosed
9 years ago

As discussed in Slack - thanks for taking time to explain @westonruter - either way has the same hinderance to a themer. If having the register option in sidebar means people have potentially more ease then I guess that should be the route. I still would love to see this become opt out at some point.

@westonruter
9 years ago

#46 @westonruter
9 years ago

  • Keywords needs-testing added

Work in progress on implementing the per-sidebar opt-in: 35855.2.diff

It's proving to be much more difficult than I expected to go the route of per-sidebar opt-in, as there are a lot more edge cases compared with a global on/off. The one known issue not yet addressed in this patch is properly previewing the move of a widget from a non-selective-refreshed sidebar to a selective-refreshed sidebar. The selective refresh is getting initiated in the an iframe that is being destroyed, and so when the new iframe is loaded it loads without the widget placed in the new sidebar.

If it is really a toss-up between the add_theme_support() and the register_sidebar() route, it would simplify things a lot if we just went with the former.

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


9 years ago

@westonruter
9 years ago

#48 @westonruter
9 years ago

Based on my comment above and the most recent conversation on Slack, I've returned to using a single:

add_theme_support( 'customize-selective-refresh-widgets' )

instead of (multiple) call(s) to:

register_sidebar( array( /*...*/ 'customize_selective_refresh' => true ) )

Widgets will still have the customize_selective_refresh widget option to specify to indicate support at the widget level.

I've also updated my Twenty Sixteen PR to use the theme support feature: https://github.com/WordPress/twentysixteen/pull/430

The hope is that with a critical mass adoption, these supports flags can be added by default so that themes and widgets have to opt-out instead of opt-in. For now, however, there would be a large number of breakages for widgets in the Customizer preview if this is enabled by default.

For patch review, please see diff with whitespace changes suppressed (since as part of this the selective refresh component is no longer included among those which can be filtered off): https://github.com/xwp/wordpress-develop/pull/144/files?w=1

I hope to commit this on Sunday (PDT) and then on Monday I plan to follow up with a post on Make Core and cross-post on Make Themes to document how theme and widget authors can add support and why they should do so.

@MH-Themes please review the patch and check for compatibility with your code if no support flags are included.

Last edited 9 years ago by westonruter (previous) (diff)

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


9 years ago

@westonruter
9 years ago

#50 @westonruter
9 years ago

If anyone could help test the patch in the next few hours, I'd be grateful. I want to commit it this evening (PDT).

#51 @ryankienstra
9 years ago

Tested With 3 Themes
Theme-Level Opt-In Works As Expected

The opt-in of the "Selective Refresh" feature works as @westonruter described it. The WordPress core themes now have an opt-in function call for "Selective Refresh." And all of the core widgets refresh properly with this:

"Archives," "Calendar," "Categories," "Custom Menu," "Meta," "Pages," "Recent Comments," "Recent Posts," "RSS," "Search," "Tag Cloud," and "Text."

While refreshing, they quickly became lighter, and update with the new content. They have no effect on the surrounding widgets or content. The rest of the page is exactly the same.

When commenting out the theme-level opt-in code, "Selective Refresh" was disabled. The widgets reloaded as before, with a full-page refresh.

I also tested disabling the widget-level opt-in for core widgets. For example, on in class-wp-widget-archives.php, I commented out 'customize_selective_refresh' => true,. This had the intended effect of refreshing the entire page on an "Archives" widget change. But the rest of the widgets still refreshed the same as before.

And the same results occurred for this test on the wp-widget-pages.php function with the theme Twenty Sixteen.

I tested this with @westonruter's commits to WordPress Core and Twenty Sixteen. Using these commits, I tested the themes Twenty Fourteen, Twenty Fifteen, and Twenty Sixteen.

Last edited 9 years ago by ryankienstra (previous) (diff)

#52 @westonruter
9 years ago

In testing some more, I was surprised to learn that Twenty Fourteen also has a sidebar that renders widgets with Masonry. What's more is that both Twenty Eleven and Twenty Fourteen also include widgets which needed support. The Twenty Eleven ephemera widget just needed the customize_selective_refresh flag added to the $widget_options. The Twenty Fourteen ephemera widget, however, also needed support for re-initializing ME.js for the refreshed partials since they can contain video and audio elements.

Also I realized I was reflowing Masonry incorrectly, and I found how to ensure that a refreshed widget retains the initial position as the previous container it replaces.

See changes in 35855.5.diff (diff), and PR.

#53 @westonruter
9 years ago

Drafted commit message:

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.

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


9 years ago

#55 @westonruter
9 years ago

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

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.

#56 @westonruter
9 years ago

  • Description modified (diff)
  • Resolution fixed deleted
  • Status changed from closed to reopened

[37040] failed to update the terminology from site logo to custom logo, resulting in the preview no longer working as noted in https://core.trac.wordpress.org/ticket/36255#comment:37

#57 @westonruter
9 years ago

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

In 37066:

Customize: Replace site logo with custom logo terminology, fixing failure to preview logo changes.

Fixes regression introduced in [37040] which was from a patch that did not include the terminology change.

See #36255.
Fixes #35855.

Note: See TracTickets for help on using tickets.