Add Settings link to plugin row actions #12
Conversation
| @@ -46,6 +46,8 @@ class Settings_Page extends Abstract_Admin_Page { | |||
| public function __construct( Script_Registry $script_registry, Style_Registry $style_registry ) { | |||
| parent::__construct(); | |||
|
|
|||
| add_filter( 'plugin_action_links_' . AI_SERVICES_MAIN_FILE, array( $this, 'add_settings_action_link' ) ); | |||
There was a problem hiding this comment.
I realize you probably don't want to add the plugin_action_links_* filter in the constructor, but it doesn't seem like the Abstract_Admin_Page class has the required interface for adding plugin action links.
There was a problem hiding this comment.
I think it makes most sense to implement this in its own small class, e.g. Plugin_Action_Link, because it's not part of the settings page. Also, this way:
- It can take the
Settings_Pageinstance as constructor parameter and read the data from there. - It can also take the
Plugin_Envinstance as constructor parameter and thus call themain_file()method instead of introducing a constant for it.
|
|
||
| $settings_link = sprintf( | ||
| '<a href="%1$s">%2$s</a>', | ||
| esc_url( add_query_arg( 'page', $this->get_slug(), admin_url( 'options-general.php' ) ) ), |
There was a problem hiding this comment.
It would be better if options-general.php was not duplicated here with:
ai-services/includes/Services/Services_Service_Container_Builder.php
Lines 252 to 254 in fbe4916
There was a problem hiding this comment.
For now we may need to go with the duplicate string. Though it would make sense to add a public get_slug() method to the Admin_Menu class (which comes from https://github.com/felixarntz/wp-oop-plugin-lib). This way we could read it from there. That change would need to be made upstream first though.
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks @westonruter for the PR! Some feedback below, I think this will need a minor refactoring in its own small class.
| @@ -46,6 +46,8 @@ class Settings_Page extends Abstract_Admin_Page { | |||
| public function __construct( Script_Registry $script_registry, Style_Registry $style_registry ) { | |||
| parent::__construct(); | |||
|
|
|||
| add_filter( 'plugin_action_links_' . AI_SERVICES_MAIN_FILE, array( $this, 'add_settings_action_link' ) ); | |||
There was a problem hiding this comment.
I think it makes most sense to implement this in its own small class, e.g. Plugin_Action_Link, because it's not part of the settings page. Also, this way:
- It can take the
Settings_Pageinstance as constructor parameter and read the data from there. - It can also take the
Plugin_Envinstance as constructor parameter and thus call themain_file()method instead of introducing a constant for it.
|
|
||
| $settings_link = sprintf( | ||
| '<a href="%1$s">%2$s</a>', | ||
| esc_url( add_query_arg( 'page', $this->get_slug(), admin_url( 'options-general.php' ) ) ), |
There was a problem hiding this comment.
For now we may need to go with the duplicate string. Though it would make sense to add a public get_slug() method to the Admin_Menu class (which comes from https://github.com/felixarntz/wp-oop-plugin-lib). This way we could read it from there. That change would need to be made upstream first though.
| return $links; | ||
| } | ||
|
|
||
| $settings_link = sprintf( |
There was a problem hiding this comment.
Before unconditionally returning this, this needs a capability check for ais_manage_services. Based on my above comment, this capability could also be read from the Settings_Page instance.
…ter/ai-services into westonruter-add/plugin-action-settings-link
…ant classes for the link URL and capability check.
|
@westonruter I have updated the implementation locally to use a new If you're okay with that, could you give me permission to push to your PR branch so that I can update this one? |
There was a problem hiding this comment.
@westonruter I've moved the logic to its own class in 975ba04. Thanks again!
This adds a Settings link to the plugin row actions as is common for plugins to do to avoid having to locate the Settings in the admin menu.