WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#25368 closed task (blessed) (fixed)

Add temp hooks for Widgets UI Refresh plugin-as-feature

Reported by: westonruter Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.0
Component: Widgets Keywords: has-patch commit
Focuses: Cc:

Description

In the Widgets UI Refresh feature-as-plugin group, one of the things we've discussed is using the Customizer to manage the widgets: to add them, edit them, re-arrange them, and remove them. In order for a widget to be draggable into the Customizer preview to drop onto a sidebar, there must first be a sidebar container element to serve as the drop zone. Unfortunately, the dynamic_sidebar action only fires if there are widgets populating that sidebar (and in fact, it fires once per widget as they are looped over). If a sidebar is empty you cannot introspect where the sidebar is located in the template. Similarly to dynamic_sidebar is is_active_sidebar, which theme authors may use to wrap an element containing the dynamic_sidebar, but this function has no filter at all.

So we need a total of 4 temp hooks added:

  • temp_is_active_sidebar
  • temp_dynamic_sidebar_did_one — filter to make sure dynamic_sidebar returns true, as otherwise fallback content may be rendered.
  • temp_before_dynamic_sidebar
  • temp_after_dynamic_sidebar

With these hooks in place, it is then possible for a plugin to force a sidebar to appear and then to create a widget drop-zone:

add_action( 'customize_preview_init', function () {

	add_filter( 'temp_is_active_sidebar', '__return_true' );
	add_filter( 'temp_dynamic_sidebar_did_one', '__return_true' );

	add_action( 'temp_before_dynamic_sidebar', function ( $index ) {
		printf( '<div class="%s" data-index="%s">', 'widget-drop-zone', esc_attr( $index ) );
	} );

	add_action( 'temp_after_dynamic_sidebar', function ( $index ) {
		printf( '</div><!-- /.widget-drop-zone.%s -->', $index );
	} );

} );

Patch has been attached to this ticket, and commits for this feature have also been pushed to GitHub: https://github.com/x-team/WordPress/compare/widgets-ui-refresh-temp-hooks?w=1

/cc @shaunandrews

Attachments (5)

widgets.php.patch (3.7 KB) - added by westonruter 4 years ago.
widgets.php.2.patch (4.2 KB) - added by westonruter 4 years ago.
Remove temp_ prefixes for dynamic_sidebar_before, dynamic_sidebar_after, dynamic_sidebar_has_widgets, and is_active_sidebar hooks. Add inline docs for these new hooks and for old dynamic_sidebar action and dynamic_sidebar_params filter.
25368.patch (7.0 KB) - added by DrewAPicture 4 years ago.
Second pass of the hook docs
25368.2.patch (7.0 KB) - added by DrewAPicture 4 years ago.
3rd pass
25368.3.patch (10.8 KB) - added by westonruter 4 years ago.
4th pass. Remove temp_ prefix from is_active_sidebar and dynamic_sidebar_has_widgets hooks; eliminate obsolete use of hacky dynamic_sidebar hook. Also on GitHub: https://github.com/x-team/wordpress-develop/compare/trac-25368

Download all attachments as: .zip

Change History (23)

#1 @westonruter
4 years ago

The patch is not nearly as large as the diff indicates. See with whitespace differences ignored: https://github.com/x-team/WordPress/compare/widgets-ui-refresh-temp-hooks?w=1

#2 @nacin
4 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 25580:

Temporary hooks for the widgets feature team in dynamic_sidebar() and is_active_sidebar().

props westonruter.
fixes #25368.

#3 @nacin
4 years ago

  • Milestone changed from Awaiting Review to 3.7

#4 @strangerstudios
4 years ago

These hooks would be useful outside of the overhaul. Any chance they lose the "temp" prefix in a future version?

#5 @westonruter
4 years ago

I'd love for them to become permanent. These temp hooks are the only way for code to detect whether or not an emptied sidebar is present on the current template.

#6 @westonruter
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Per discussion in wordpress-ui today: https://irclogs.wordpress.org/chanlog.php?channel=wordpress-ui&day=2013-12-16&sort=asc#m144623

I'm re-opening this ticket with the request to strip the temp_ prefixes from these hooks.

#7 @SergeyBiryukov
4 years ago

  • Milestone changed from 3.7 to 3.9

@westonruter
4 years ago

Remove temp_ prefixes for dynamic_sidebar_before, dynamic_sidebar_after, dynamic_sidebar_has_widgets, and is_active_sidebar hooks. Add inline docs for these new hooks and for old dynamic_sidebar action and dynamic_sidebar_params filter.

#8 @SergeyBiryukov
4 years ago

  • Keywords docs-feedback added

#9 follow-up: @DrewAPicture
4 years ago

  • Keywords needs-docs added

Yeah, the hook docs in widgets.php.2.patch are going to need some work.

#10 in reply to: ↑ 9 ; follow-up: @westonruter
4 years ago

Replying to DrewAPicture:

Yeah, the hook docs in widgets.php.2.patch are going to need some work.

What can we do to improve the hook docs?

#11 in reply to: ↑ 10 @DrewAPicture
4 years ago

Replying to westonruter:

Replying to DrewAPicture:

Yeah, the hook docs in widgets.php.2.patch are going to need some work.

What can we do to improve the hook docs?

I'll work up an improved patch for you this afternoon.

@DrewAPicture
4 years ago

Second pass of the hook docs

#12 follow-up: @DrewAPicture
4 years ago

  • Keywords needs-docs removed

25368.patch is a comprehensive second pass of the hook docs. Things started to get a little murky with the dynamic_sidebar hook so feel free to comment with any changes.

#13 in reply to: ↑ 12 @westonruter
4 years ago

Replying to DrewAPicture:

25368.patch is a comprehensive second pass of the hook docs. Things started to get a little murky with the dynamic_sidebar hook so feel free to comment with any changes.

Regarding @type array|string $callback should the type just be callable? It could be a closure as well.

And then the $params is actually wrapped in another array, so I think the docs should be something like:

@type array   $params { 
        Array of arrays to pass into the widget callback, as the second argument to call_user_func_array

        @type array { 
                An array of multi-widget arguments. 
 
                @type int $number Number increment used for multiples of the same widget. 
        } 
}

Here's a dump of an example registered widget:

Array
(
    [name] => Text
    [id] => text-54
    [callback] => Array
        (
            [0] => WP_Widget_Text Object
                (
                    [id_base] => text
                    [name] => Text
                    [widget_options] => Array
                        (
                            [classname] => widget_text
                            [description] => Arbitrary text or HTML.
                        )

                    [control_options] => Array
                        (
                            [id_base] => text
                            [width] => 400
                            [height] => 350
                        )

                    [number] => 62
                    [id] => text-62
                    [updated] =>
                    [option_name] => widget_text
                )

            [1] => display_callback
        )

    [params] => Array
        (
            [0] => Array
                (
                    [number] => 54
                )

        )

    [classname] => widget_text
    [description] => Arbitrary text or HTML.
)

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


4 years ago

#15 @helen
4 years ago

  • Type changed from enhancement to task (blessed)

Need to rename these hooks as well as all instances of using them.

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


4 years ago

@DrewAPicture
4 years ago

3rd pass

#17 @DrewAPicture
4 years ago

  • Keywords commit added; docs-feedback removed

In 25368.2.patch, I opted to pass a type of array|callback for the $callback argument, as well as simply define $param as an associative array of multi-widget arguments instead of get into the minutia of the sub-arrays.

The inline docs in this patch look good to go.

@westonruter
4 years ago

4th pass. Remove temp_ prefix from is_active_sidebar and dynamic_sidebar_has_widgets hooks; eliminate obsolete use of hacky dynamic_sidebar hook. Also on GitHub: https://github.com/x-team/wordpress-develop/compare/trac-25368

#18 @ocean90
4 years ago

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

In 27543:

Widget Customizer: Make temp hooks final and add inline docs.

New hooks are dynamic_sidebar_before, dynamic_sidebar_after, dynamic_sidebar_has_widgets and is_active_sidebar.
Remove obsolete use of hacky dynamic_sidebar hook.

props westonruter, DrewAPicture.
fixes #25368.

Note: See TracTickets for help on using tickets.