wearerequired / WP-Widget-Disable

🖼 A WordPress plugin that allows you to disable Sidebar and Dashboard Widgets.
https://wordpress.org/plugins/wp-widget-disable/
GNU General Public License v2.0
16 stars 6 forks source link

Begin with network admin compatibility #31

Closed swissspidy closed 6 years ago

swissspidy commented 6 years ago

With this patch, the network admin looks like this:

screen shot 2018-05-02 at 20 26 04

To do:

Fixes #29.

ocean90 commented 6 years ago

When the site option doesn't exist yet you get the update message twice, same as in https://github.com/wearerequired/WP-Widget-Disable/issues/11.

Should set_default_options() set the default site option if is_multisite()? Since that's only called by register_activation_hook() what about plugin updates?

swissspidy commented 6 years ago

Yeah, we could totally extend set_default_options() and perhaps always call it on init. However, I'm not a huge fan of just adding an empty options anyway.

I'd much rather like to investigate why this happens.

For instance, we call settings_errors( 'wp-widget-disable' ); on admin_notices. That should be called on multisite as well, so there's no need to call settings_errors() in views/admin.php for multisite

In save_network_options() I used general instead of wp-widget-disable, which is also incorrect:

https://github.com/wearerequired/WP-Widget-Disable/blob/d584d0f218986f77e6a4b2306a4f61c969173d78/classes/class-wp-widget-disable.php#L213-L215

tl;dr:

ocean90 commented 6 years ago

@swissspidy Changed a few things but did you see my mention of #11 and #18 which is basically https://core.trac.wordpress.org/ticket/21989?

swissspidy commented 6 years ago

Yes, and I replied to that. Couldn‘t we solve this by first checking if there are some errors before adding one in the sanitize callback?

ocean90 commented 6 years ago

admin_footer_text() and print_admin_styles() are currently ignored on the network admin screen because $screen->base has -network as a suffix which add_submenu_page() does not add. For print_admin_styles() see #34, do we still want admin_footer_text()?

swissspidy commented 6 years ago

Even though we could just add the suffix for that case, I don‘t think there‘s much value in it really.