[wp-trac] Re: [WordPress Trac] #10013: poorly documented widget args are confusing

WordPress Trac wp-trac at lists.automattic.com
Wed Jun 3 07:10:21 GMT 2009


#10013: poorly documented widget args are confusing
-------------------------------+--------------------------------------------
 Reporter:  Denis-de-Bernardy  |       Owner:  azaozz      
     Type:  defect (bug)       |      Status:  new         
 Priority:  normal             |   Milestone:  2.8         
Component:  Widgets            |     Version:              
 Severity:  normal             |    Keywords:  dev-feedback
-------------------------------+--------------------------------------------

Comment(by Denis-de-Bernardy):

 Replying to [comment:6 azaozz]:
 > > this line: if ( 'noform' !== $return ) ... is very questionable. it
 means you cannot add extra fields to a form that has no fields in the
 first place.
 >
 > Yes, you cannot add form fields to a widget that doesn't have a form...
 This is only used to show (or hide) the Save button on the widgets screen.
 >
 > > widget_update_callback hook has inconsistent args.
 >
 > Again this is not meant to allow replacing of the callback, it is there
 to allow extra processing of the widget's settings before saving them.

 Here's what I used to do, and what I'm looking into doing instead, so you
 get a better picture... The plugin adds context handling to widgets.

 Until now, I'd change the two/three wp_registered_widgets arrays directly,
 and replace their callbacks with my own. In my display callback, I'd check
 the context, and proceed with the widget's normal callback if it was
 valid. In the form callback, I'd output the widget's callback, then my
 own. In the update callback, I'd grab my own variables, and pass it on to
 the widget.

 It works fine until users start dropping widgets. The contexts array where
 I store it all then grows, and grows, and grows. It would seem a lot more
 elegant to leave the internal arrays untouched, and used the three new
 filters to do the same, storing the contexts in the widget's option
 directly.


 > I'm actually wondering how to discourage plugins that access widgets
 internal functions directly. Each widget is like a small (or not so small)
 plugin and is meant to be a separate entity that uses the API. Using (or
 changing) internal widget functionality is not a good practice.

 That would depend on what your plugin tries to do... ;-) Frankly, I fail
 to see the point in these hooks, if they don't allow to do what I'm using
 them for.

 Anyway, this one:

 {{{
 $instance = apply_filters('widget_update_callback', $instance,
 $new_instance, $this);
 }}}

 Should almost certainly be:

 {{{
 $instance = apply_filters('widget_update_callback', $instance,
 $new_instance, $old_instance, $this);
 }}}

 ... so as to allow to save options added to the form.

 And this one:

 {{{
 if ( 'noform' !== $return )
         do_action_ref_array( 'in_widget_form', array(&$this) ); // add
 extra fields in the widget form
 }}}

 Should be something like:

 {{{
 // add extra fields in the widget form, unset return if you add any to a
 widget that has none
 do_action_ref_array( 'in_widget_form', array(&$this, &$return,
 $sidebar_args) );
 }}}

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/10013#comment:7>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list