-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue #839: Add wp-cli script to create test widgets #859
Conversation
Adds an instance of every default WordPress widget to the first registered sidebar. First, it checks if a widget with the same settings exists in the sidebar. Then, it conditionally creates the widget. And adds all of them to the sidebar. Mainly copies amp_get_widget_id() from import_theme_starter_content(). Props @westonruter.
Use get_attachment_url() to get the image's URL. Also, access the height and width with array index notation, Not with object operators. Also, pass array() as a second argument to get_option(). This will ensure that the default return is array().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @kienstra 👍 I pushed a quick fix and added documentation for the script, could you kindly review my changes.
bin/add-test-widgets-to-sidebar.php
Outdated
$menus = wp_get_nav_menus(); | ||
$minimum_count = 4; | ||
foreach ( $menus as $menu ) { | ||
if ( $menu->count > $minimum_count ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually requires 5 menu items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed here.
bin/add-test-widgets-to-sidebar.php
Outdated
*/ | ||
function amp_widget_already_in_sidebar( $widget, $sidebar ) { | ||
$sidebars = wp_get_sidebars_widgets(); | ||
$widgets_in_sidebar = $sidebars[ $sidebar ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure this isset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed here.
contributing.md
Outdated
1. `ssh` into an environment like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV) | ||
2. `cd` to the root of this plugin | ||
3. run `wp eval-file bin/add-test-widgets-to-sidebar.php` | ||
4. visit a page with the sidebar: Blog Sidebar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ThierryA,
Thanks for adding this documentation. This step 4.
will often be accurate. But the step below would apply no matter what theme is active:
4. There will be a message indicating which sidebar has the widgets. Please visit a page with that sidebar.
Thanks For Reviewing And Improving Hi @ThierryA, |
Thanks @kienstra, I pushed a commit with your suggestion 👍 |
Request For Code Review
Hi @westonruter or @ThierryA,
Could you please review this pull request for #839? This builds on @kopepasah's work to create a widget test script.
Using Koop's function, it finds the first registered sidebar. Then, it adds an instance of every WordPress widget that's registered in a "vanilla" instance, per Joshua's screenshot.
But only if an instance with the same settings doesn't already exist in that sidebar, As @westonruter requested in #839.
Some widgets have multiple instances, with different settings. Like 'Audio,' 'Archives,'Categories,' 'Tag Cloud,' and 'Video.'