[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