[wp-trac] [WordPress Trac] #34596: WP_Customize_Manager::add_*() methods should return the added instance

WordPress Trac noreply at wordpress.org
Sun Nov 8 02:31:40 UTC 2015


#34596: WP_Customize_Manager::add_*() methods should return the added instance
-------------------------------------------------+-------------------------
 Reporter:  westonruter                          |       Owner:
     Type:  enhancement                          |      Status:  new
 Priority:  normal                               |   Milestone:  Future
Component:  Customize                            |  Release
 Severity:  normal                               |     Version:  3.4
 Keywords:  good-first-bug has-patch has-unit-   |  Resolution:
  tests                                          |     Focuses:
-------------------------------------------------+-------------------------

Comment (by westonruter):

 @fusillicode good work! Two pieces of feedback:

 1) Instead of returning the value of an assignment:

 {{{#!php
 <?php
 return $this->settings[ $setting->id ] = $setting;
 }}}

 Let's break these up onto separate lines:

 {{{#!php
 <?php
 $this->settings[ $setting->id ] = $setting;
 return $setting;
 }}}

 2) I think these are harder to test, but I don't think that these tests
 are necessarily testing the right thing:

 *
 `test_add_control_returns_a_new_control_instance_created_with_the_arguments_passed_in()`
 *
 `test_add_section_returns_a_new_section_instance_created_with_the_arguments_passed_in()`
 *
 `test_add_panel_returns_a_new_panel_instance_created_with_the_arguments_passed_in()`

 So instead of:

 {{{#!php
 <?php
 $panel = new WP_Customize_Panel( $this->manager, 'foo' );
 $inside_panel = $this->manager->add_panel( 'foo' );
 $this->assertEquals( $panel->id, $inside_panel->id );
 }}}

 I think assertions would make more sense like:

 {{{#!php
 <?php
 $panel = $this->manager->add_panel( 'foo' )
 $this->assertInstanceOf( 'WP_Customize_Panel', $panel );
 $this->assertEquals( 'foo', $panel->id );
 }}}

--
Ticket URL: <https://core.trac.wordpress.org/ticket/34596#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list