WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#26495 closed defect (bug) (fixed)

Dashboard "At a Glance" hooks changed

Reported by: nacin Owned by: nacin
Milestone: 3.8 Priority: high
Severity: blocker Version: 3.8
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

+do_action( 'rightnow_list_end' );
+do_action( 'rightnow_list_start' );
-do_action('right_now_content_table_end');
-do_action('right_now_discussion_table_end');
-do_action('right_now_table_end');

We should *never* have had hooks inside a table like that. We could still support these if we wanted. Currently running a search to see if anyone used these.

The new hooks are a bit weird. We should have *one* hook at the bottom, and it should be a filter that implodes into list items, so plugin authors aren't subject to the whims of our markup.

Attachments (2)

26495.diff (1003 bytes) - added by nacin 4 months ago.
I have no idea what to call this hook.
26495.2.diff (939 bytes) - added by nacin 4 months ago.
Fix inline docs typo

Download all attachments as: .zip

Change History (18)

comment:1 nacin4 months ago

I don't really care about compatibility about the old hooks. Having information in the Right Now widget was merely a value-add. And, the widget completely changed. "At a Glance" is a pretty solid instruction for what plugin authors should be putting in this box.

$ ack --php 'right_now_(content|discussion)?table_end'

plugins/bbpress/includes/admin/metaboxes.php
29: * @uses do_action() Calls 'bbp_dashboard_widget_right_now_table_end'
217:	<?php do_action( 'bbp_dashboard_widget_right_now_table_end' ); ?>

plugins/buddy-bbpress-support-topic/includes/bbpress-functions.php
700:add_action( 'bbp_dashboard_widget_right_now_table_end', 'bpbbpst_bbpress_right_now_dashboard_stats' );

plugins/disable-post-fonctionnality/disable-post-fonctionnality.php
142:			do_action('right_now_table_end');

plugins/fv-antispam/fv-antispam.php
45:          add_action( 'right_now_table_end', array( $this, 'admin__show_dashboard_count' ) );

plugins/mailbase/library/wp-framework/Base.php
1196:	public function action_right_now_table_end(){}

plugins/quiz-master/quiz_master.php
69:add_action('right_now_table_end', 'quiz_master_add_dashboard');

plugins/report-comments/report-comments.php
132:add_action('right_now_table_end','rc_dashboard_status');

plugins/revisionary/admin/admin-dashboard_rvy.php
2:add_action ( 'right_now_table_end', 'rvy_right_now_pending' );

plugins/right-now-reloaded/right-now-reloaded.php
290:				do_action( 'right_now_table_end' );

plugins/role-scoper/admin/admin-dashboard_rs.php
3:add_action ( 'right_now_table_end', 'scoper_right_now_pending' );

plugins/user-messages/view/DashboardFeatures.php
23:			add_action('right_now_table_end', array(&$this, 'add_right_now_table_row'));
     
plugins/wordpress-by-circle-tree/includes/class.wp_login_lockdown.php
73:        add_action('right_now_table_end', array($this, 'admin_home_failed_logins'));

plugins/wordpress-ecommerce/marketpress-includes/marketpress-stats.php
14:		add_action( 'right_now_table_end', array(&$this, 'rightnow_dashboard_widget') );

plugins/wp-blackcheck/wp-blackcheck.php
249:	add_action('right_now_table_end', 'wpbc_table_end' );

plugins/wp-deals/wpdeals-admin/admin-dashboard.php
13:	add_action('right_now_table_end', 'wpdeals_right_now');

plugins/wp-reportpost/main.php
295:add_action('right_now_table_end', 'wprp_right_now_table_end');
296:function wprp_right_now_table_end()

plugins/wp-tsina/wp-tsina.php
33:  add_action('right_now_table_end', 'tsina_addto_right_now');

plugins/wplistcal/admin.inc.php
472:add_action("right_now_table_end", "wplc_activity_box");

plugins/wypiekacz/wypiekacz.php
86:					add_action( 'right_now_content_table_end', array( &$this, 'right_now_table_end' ) );
88:					add_action( 'right_now_table_end', array( &$this, 'right_now_table_end' ) );
1174:	function right_now_table_end() {

plugins/zelist/includes/admin-functions.php
489:add_action('right_now_table_end','zelist_activity_report');

plugins/zelist-directory/zelist-directory.php
217:add_action('right_now_table_end','zelist_activity_report');

nacin4 months ago

I have no idea what to call this hook.

comment:2 SergeyBiryukov4 months ago

In 26495.diff, "Include addition elements" looks like a typo. Should it be "additional"?

comment:3 nacin4 months ago

Yeah.

comment:4 GaryJ4 months ago

a) Any benefit in making the filter cover all of the glance items, not just the optional extra ones, so there is more flexibility to remove built-in lines?

b) Could line 224 or 225 get an (array) cast so that if a string is returned from the final filter, it will still try to output correctly?

comment:5 nacin4 months ago

  • Severity changed from normal to blocker

a) Yeah, but it's a bit too late for refactoring the function and figuring out under what parameters will we allow core to be manipulated. This filter is flexible enough that we could just start passing the core ones in 3.9 and all will be fine.

b) It could, but we usually wouldn't here, for two reasons. One, the return value here isn't critical. If a plugin screws up this filter, that's not a big deal. Two, it's better to not suppress developer errors when you can get away with it. Here, the worst that happens is a warning, versus a fatal. It's best to let the developer be told they're doing something wrong.

Setting this ticket to blocker as the current (new) hooks are untenable when it comes to forwards compatibility.

comment:6 toscho4 months ago

  • Cc info@… added

comment:7 follow-up: nacin4 months ago

Any thoughts on the hook name? dashboard_glance_items is the best I could come up with.

comment:8 in reply to: ↑ 7 GaryJ4 months ago

Replying to nacin:

Any thoughts on the hook name? dashboard_glance_items is the best I could come up with.

Where it is, what's it for, and what it handles - dashboard_glance_items seems fine to me.

comment:9 MikeHansenMe4 months ago

  • Keywords has-patch added

nacin4 months ago

Fix inline docs typo

comment:10 nacin4 months ago

  • Keywords commit added

comment:11 nacin4 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 26832:

Introduce dashboard_glance_items filter which replaces 3.7 right_now_* hooks that were tied to table markup.

fixes #26495.

comment:12 follow-ups: mrwweb4 months ago

Appreciate the desire to make it easy to add new elements, but the new filter prevents plugins from adding new headings (and therefore sections) to the At a Glance widget. (I realize that I can still hack out of it with an empty list item but that's gross.)

I'm the author of one of the plugins that was using the old hooks. I had updated for 3.8 compatibility prior to RC2, but found it broken yesterday and tracked down the issue to this change.

The primary list of stuff in "At a Glance" is really just post type counts, so it doesn't make sense to tack on additional random other numbers without some type or organizing or ordering.

If the rightnow_end action preceded the version and privacy setting information, I could hook to that, but it would be weird to stick a list of Post Statuses on after that information rather than immediately after the post counts.

It suspect it's took late to do much about this issue, but I wanted to at least mention this.

comment:13 in reply to: ↑ 12 ; follow-up: mrwweb4 months ago

Replying to mrwweb:

One other issue with current filter is that it doesn't put a class on the list items so all new elements get the default circle dashicon.

comment:14 in reply to: ↑ 13 toscho4 months ago

Replying to mrwweb:

One other issue with current filter is that it doesn't put a class on the list items so all new elements get the default circle dashicon.

Looks really odd. It was much easier before to add custom classes to the new markup.

Is this the correct way? We have to insert a dead empty list item now to get proper CSS classes? Wouldn’t it be better to allow two entries per item: one for the text content, one for additional attributes?

comment:15 in reply to: ↑ 12 ; follow-up: SergeyBiryukov4 months ago

Replying to mrwweb:

It suspect it's took late to do much about this issue, but I wanted to at least mention this.

This ticket was closed on a completed milestone, please open a new one.

comment:16 in reply to: ↑ 15 mrwweb4 months ago

Replying to SergeyBiryukov:

This ticket was closed on a completed milestone, please open a new one.

Opened #26571.

Note: See TracTickets for help on using tickets.