In last 2 days I worked on the code of this plugin. My work covered 3 aspects:
code approach (better explained below)
new features
business logic
readmes update
I ended up to create a candidate V1.3, but before sending a PR, I want to discuss what I've done.
Code approach
Regarding first point main modification is on plugin boot up: instead of using a static init method, the class instantiation happen inside the add_action call: add_action( 'plugins_loaded', array( new Post_Type_Archive_Links, 'init' ) ); so init method is not static but dynamic. After that, all the hooks are moved from constructor to a method enable. This method pairs with another one disable that roll back it: remove hooks and text domain. To call disable we need the class instance, that can be retrieved using a filter hook: "post_type_archive_links".
All this changes are in comit 792c959.
The other are minor changes:
avoid accessing $_POST directly and in 2 different place, but in one single place and using filter_input_array
constant values (metabox id, mebabox list id, nonce) are handled via class constants instead of object properties, however the magic __get method is used for backward compatibility, to map removed variables to constants
the method setup_admin_hooksis removed because redundant: it was a wrapper for metabox_scripts already called on 'admin_enqueue_scripts' hook
New features
Added Italian translation
When no CPTs available, plugin shows "No items." (using core text domain) like core does when no items are available for a specific menu items type.
Composer support
Business logic
Currently the CPTs to be shown in the metabox are retrieved using '_builtin' => false and 'has_archive' => false, however when a CPT is registered with true 'has_archive' but false 'publicly_queryable' it is shown in the metabox, but I think they should not be added. Moreover, I think CPTs registered with 'show_in_nav_menus' set to false (even if core refers to single items), should also be removed from metabox. My code reflects these thoughts, and add a filter "show_{$cpt_slug}_archive_in_nav_menus" to force a CPT being added even if 'publicly_queryable' and/or 'show_in_nav_menus' are set to false (but not if 'has_archive' is false). My code also moves the CPTs retrieval from metabox method to a separate get_cpts method that runs just before metabox (same hook higher priority): this allow to easily show "No items." and avoid js addition if there are no CPTs.
Readmes update
I've updated:
Changelog
FAQ (to explain the why "No items." is shown according to new behavior)
Contributors list adding myself
"Manually" edit markdown readme to use repo-relative images urls for screenshots, because the wp.org urls added via grunt-wp-readme-to-markdown are not shown (they bring to a 404, I don't know why)
Compare
Code explain better than thousands of words (I hope), the full release compare is here
In last 2 days I worked on the code of this plugin. My work covered 3 aspects:
I ended up to create a candidate V1.3, but before sending a PR, I want to discuss what I've done.
Code approach
Regarding first point main modification is on plugin boot up: instead of using a static init method, the class instantiation happen inside the
add_action
call:add_action( 'plugins_loaded', array( new Post_Type_Archive_Links, 'init' ) );
so init method is not static but dynamic. After that, all the hooks are moved from constructor to a methodenable
. This method pairs with another onedisable
that roll back it: remove hooks and text domain. To calldisable
we need the class instance, that can be retrieved using a filter hook:"post_type_archive_links"
. All this changes are in comit 792c959.The other are minor changes:
$_POST
directly and in 2 different place, but in one single place and usingfilter_input_array
__get
method is used for backward compatibility, to map removed variables to constantssetup_admin_hooks
is removed because redundant: it was a wrapper formetabox_scripts
already called on'admin_enqueue_scripts'
hookNew features
Business logic
Currently the CPTs to be shown in the metabox are retrieved using
'_builtin' => false
and'has_archive' => false
, however when a CPT is registered with true'has_archive'
but false'publicly_queryable'
it is shown in the metabox, but I think they should not be added. Moreover, I think CPTs registered with'show_in_nav_menus'
set to false (even if core refers to single items), should also be removed from metabox. My code reflects these thoughts, and add a filter"show_{$cpt_slug}_archive_in_nav_menus"
to force a CPT being added even if'publicly_queryable'
and/or'show_in_nav_menus'
are set to false (but not if'has_archive'
is false). My code also moves the CPTs retrieval frommetabox
method to a separateget_cpts
method that runs just beforemetabox
(same hook higher priority): this allow to easily show "No items." and avoid js addition if there are no CPTs.Readmes update
I've updated:
Compare
Code explain better than thousands of words (I hope), the full release compare is here