WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#33552 closed enhancement (fixed)

Facilitate plugins to override Customizer features

Reported by: westonruter Owned by: westonruter
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.9
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

This is splitting out from #33411, which was to fix an issue where removing the nav_menus panel in the Customizer would result in a JS error on init.

Nevertheless, removing the nav_menus panel on its own actually would not have the desired effect. All of the Menu Customizer settings, sections, and controls would still be loaded needlessly. In 4.3, the only way to completely disable the nav menus in the Customizer is to unhook the actions/filters that are added by WP_Customize_Nav_Menus::__construct(), for instance with this plugin:

<?php
add_action( 'customize_register', function( $wp_customize ) {
        /** @var WP_Customize_Manager $wp_customize */
        remove_action( 'customize_controls_enqueue_scripts', array( $wp_customize->nav_menus, 'enqueue_scripts' ) );
        remove_action( 'customize_register', array( $wp_customize->nav_menus, 'customize_register' ), 11 );
        remove_filter( 'customize_dynamic_setting_args', array( $wp_customize->nav_menus, 'filter_dynamic_setting_args' ) );
        remove_filter( 'customize_dynamic_setting_class', array( $wp_customize->nav_menus, 'filter_dynamic_setting_class' ) );
        remove_action( 'customize_controls_print_footer_scripts', array( $wp_customize->nav_menus, 'print_templates' ) );
        remove_action( 'customize_controls_print_footer_scripts', array( $wp_customize->nav_menus, 'available_items_template' ) );
        remove_action( 'customize_preview_init', array( $wp_customize->nav_menus, 'customize_preview_init' ) );
}, 10 );

This is not developer friendly.

To truly remove the Nav Menus entirely, another extension mechanism is needed to hook in earlier to be able to override which Customizer features get loaded. This is useful for disabling features, but it is perhaps even more useful for plugins to introduce new features, and even for feature plugins to swap out Customizer components altogether. With that in mind, we could have a customize_manager_construct action that fires at the top of WP_Customize_Manager::__construct(). A plugin could then set the $wp_customize->widgets and $wp_customize->nav_menus up front, and then when the constructor continues it could prevent assigning these if they were already assigned.

Sou could disable menus in the Customizer just with this plugin:

<?php
add_action( 'customize_manager_construct', function( $wp_customize ) {
        $wp_customize->nav_menus = false;
} );

Attachments (3)

33552.diff (1.5 KB) - added by westonruter 4 years ago.
33552.2.diff (1.7 KB) - added by DrewAPicture 4 years ago.
Whitelist approach
33552.3.diff (5.7 KB) - added by westonruter 4 years ago.

Download all attachments as: .zip

Change History (19)

@westonruter
4 years ago

#1 @westonruter
4 years ago

  • Keywords has-patch added

@DrewAPicture commented on 33411:

Very interesting. I wonder what if any potential downsides there might be in placing such a general hook this high up in the stack.

I'd almost be tempted use a dynamic hook with a whitelist based on the nav_menus and widgets properties to sort of lock it down to this specific context.

The whitelist could be expanded in the future without opening the door wide to the scenario that any general-purpose hook brings: having to support any number of unexpected use-cases down the road.

Last edited 4 years ago by DrewAPicture (previous) (diff)

#2 @valendesigns
4 years ago

I really hate that people want to remove Menus from the Customizer, but I get it. Change is hard. Generally though this is a nice addition that extends well beyond Menus, which could prove to be very useful for the right use case.

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


4 years ago

#4 in reply to: ↑ 3 @DrewAPicture
4 years ago

  • Keywords dev-feedback added

Replying to slackbot:

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

33552.2.diff contains the whitelist approach discussed in Slack earlier and in comment:1. As mentioned, this approach has the benefit of control over the whitelist and a graceful fallback if anything unexpected is passed. It could also be expanded as necessary in the future.

@DrewAPicture
4 years ago

Whitelist approach

#5 @DrewAPicture
4 years ago

@westonruter Looking for feedback on 33552.2.diff.

#6 @westonruter
4 years ago

@DrewAPicture first of all, the $components would need to be iterated over in a foreach loop, right?

To me it seems the primary use case of this filter is to remove components from being added. For instance:

<?php
add_filter( 'customize_components_to_load', function( $components ) {
    return array_filter( $components, function ( $component ) { 
         return 'nav_menus' !== $component; 
    } );
} );

But what about the other non-removal use cases, where new components might be added or existing ones replaced?

If I want to introduce a new component for “posts” then it seems I would need to do basically hack the filter to serve as an action:

<?php
add_filter( 'customize_components_to_load', function( $components, $wp_customize ) {
    $wp_customize->posts = new My_Customize_Posts( $wp_customize );
    return $components;
}, 10, 2 );

If I want to replace the current widgets implementation with a different one, I would have to do something like:

<?php
add_filter( 'customize_components_to_load', function( $components, $wp_customize ) {
    // First remove widgets entirely so that it won't get constructed.
    $components = array_filter( $components, function ( $component ) { 
         return 'widgets' !== $component; 
    } );

    // Now instantiate the the custom implementation now that the default widgets one won't be. 
    $wp_customize->widgets = new My_Customize_Widgets( $wp_customize );
    return $components;
}, 10, 2 );

So I'm not sure.

Maybe it would work better if customize_components_to_load was filtering a mapping of component name to class name. For instance:

<?php
$components = array(
    'widgets' => 'WP_Customize_Widgets',
    'nav_menus' => 'WP_Customize_Nav_Menus',
);
$components = apply_filters( 'customize_components_to_load', $components, $this ); 

And then it could do:

<?php
foreach ( $components as $component => $class ) {
    $this->$component = new $class( $this );
}

This would, however, not play nicely with the requireing of the class file, so those should require_once statements should remain where they are at the top, or else changed to be something lower down like:

<?php
if ( isset( $components['widgets'] ) ) {
    require_once( ABSPATH . WPINC . '/class-wp-customize-widgets.php' );
}
if ( isset( $components['nav_menus'] ) ) {
    require_once( ABSPATH . WPINC . '/class-wp-customize-nav-menus.php' );
}

What this implies is that \WP_Customize_Widgets and \WP_Customize_Nav_Menus can never be pluggable to be replaced with any other classes. If you want to use a different class for managing menus, then you'd have to do:

<?php
add_filter( 'customize_components_to_load', function( $components) {
    require_once MY_PLUGIN_DIR . '/class-my-customize-nav-menus.php';
    $components['nav_menus'] = 'My_Customize_Nav_Menus';
    return $components;
} );

But both of the My_Customize_Nav_Menus and WP_Customize_Nav_Menus would both be loaded regardless, even though the latter would never be used.

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

#7 @wonderboymusic
4 years ago

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

#8 @westonruter
4 years ago

With 33552.diff, all the use cases I can image would be achievable:

Remove a component from being loaded:

<?php
add_action( 'customize_manager_construct', function ( $manager ) {
    $manager->nav_menus = false;
});

To replace a component with another implementation:

<?php
add_action( 'customize_manager_construct', function ( $manager ) {
    require_once( __DIR__ . '/class-my-customize-widgets.php' );
    $manager->widgets = new My_Customize_Widgets( $manager );
});

And to add an entirely new component, similar to the previous:

<?php
add_action( 'customize_manager_construct', function ( $manager ) {
    require_once( __DIR__ . '/class-my-customize-comments.php' );
    $manager->comments = new My_Customize_Comments( $manager );
});

Maybe customize_construct would be better than customize_manager_construct for an action name.

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


4 years ago

@westonruter
4 years ago

#10 @westonruter
4 years ago

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

In 35307:

Customizer: Introduce customize_loaded_components filter to allow core components to be disabled.

Also move style rule from customize-nav-menus.css to customize-controls.css so that widgets button is properly styled when nav_menus component is excluded from loading. See [35304]. See #33327.

Props westonruter, DrewAPicture.
Fixes #33552.

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


4 years ago

#12 @westonruter
4 years ago

  • Keywords dev-feedback removed

With [35307], nav menus can be removed from the Customizer via:

<?php
add_filter( 'customize_loaded_components', function ( $components ) {
    return array_filter( $components, function ( $component ) {
        return 'nav_menus' !== $component;
    } );
} );

Similarly, widgets can be removed from the Customizer via:

<?php
add_filter( 'customize_loaded_components', function ( $components ) {
    return array_filter( $components, function ( $component ) {
        return 'widgets' !== $component;
    } );
} );

#13 @westonruter
4 years ago

In 36216:

Customizer: Call _doing_it_wrong() if widgets or nav_menus are manually removed via WP_Customize_Manager::remove_panel().

Advise that the customize_loaded_components filter should be used instead.

Props voldemortensen.
See #33552.
Fixes #35242.

#14 @westonruter
4 years ago

In 36262:

Customizer: Re-use list of components to eliminate code duplication.

Introduces protected WP_Customize_Manager::$components to store list of loaded core components filtered by customize_loaded_components.

Props DrewAPicture.
See #35242
See #33552.
Fixes #35354.

#15 @westonruter
4 years ago

In 36645:

Docs: Use markdown instead of HTML for code formatting.

Fixes phpdoc usage in [36622], [36608], [35724], [35307].

See #35898.
See #35869.
See #34738.
See #33552.

#16 @westonruter
3 years ago

See also #38967. We should honor the widgets and nav_menus theme support in these components.

Note: See TracTickets for help on using tickets.