Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#20161 closed enhancement (fixed)

Move Toolbar rendering in the admin to the header

Reported by: nacin's profile nacin Owned by: nacin's profile nacin
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Toolbar Keywords:
Focuses: Cc:

Description

Some admin pages take a while to load. Examples include:

  • themes.php page with a boatload of themes
  • edit.php with a large posts-per-page setting
  • an update which does not operate in an iframe

There's nothing truly wrong with a page that takes a long time to fully paint (150 themes is a lot of screenshots to load) unless we need DOM ready to fire quickly, but it does look broken when you see the empty space at the top.

The in_admin_header seems like a good candidate hook. At worst, we add a new hook.

The Debug Bar will still simply wait until admin_footer to spit out its panel information. That's the only contraindication I could think of for not doing this.

Change History (7)

#1 @nacin
13 years ago

This works beautifully.

#2 @scribu
13 years ago

  • Type changed from defect (bug) to enhancement

#3 @nacin
13 years ago

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

In [20099]:

In the admin, render the Toolbar immediately, rather than waiting until the footer. When intensive pages take a while to load, you aren't stuck without a working admin header. fixes #20161.

#4 follow-up: @johnbillion
13 years ago

Bah. This breaks a debugging plugin of mine which outputs page generation time/queries/etc in a toolbar menu.

#5 in reply to: ↑ 4 ; follow-up: @nacin
13 years ago

Replying to johnbillion:

Bah. This breaks a debugging plugin of mine which outputs page generation time/queries/etc in a toolbar menu.

That data is going to be inaccurate anyway, as the pageload wasn't allowed to finish before collecting that data.

Locally, I use this:

add_action( 'shutdown', function() {
	if ( ! did_action( 'admin_footer' ) )
		return;
	$text = round( memory_get_peak_usage() / 1024 / 1024, 3 ) . ' MB / ' . timer_stop() . ' sec';
	echo "\n<script>(function($){ $('#wp-admin-bar-debug-bar').find('div').text('$text'); })(jQuery);</script>\n";
}, 1 );

http://plugins.trac.wordpress.org/changeset/525389/debug-bar/

#6 in reply to: ↑ 5 ; follow-up: @johnbillion
13 years ago

Replying to nacin:

That data is going to be inaccurate anyway, as the pageload wasn't allowed to finish before collecting that data.

This is actually intentional. The plugin hooks onto wp_footer() and admin_footer() at a priority of 999 so it gathers its data and adds its menus just before the admin bar is rendered (at priority 1000). This way the data it displays is exclusive of the overhead of the admin bar and more representative of a page load for non-logged-in users (which is what I'm interested in seeing).

Anyway, this changeset isn't a problem now because I've implemented a JS-based solution similar to yours.

#7 in reply to: ↑ 6 @nacin
13 years ago

Replying to johnbillion:

Replying to nacin:

That data is going to be inaccurate anyway, as the pageload wasn't allowed to finish before collecting that data.

This is actually intentional. The plugin hooks onto wp_footer() and admin_footer() at a priority of 999 so it gathers its data and adds its menus just before the admin bar is rendered (at priority 1000). This way the data it displays is exclusive of the overhead of the admin bar and more representative of a page load for non-logged-in users (which is what I'm interested in seeing).

Anyway, this changeset isn't a problem now because I've implemented a JS-based solution similar to yours.

I would agree that is intentional behavior.

At the very least, http://plugins.trac.wordpress.org/changeset/525389/debug-bar/ returns the panels to wp_footer and admin_footer at priority 1000. So everything but changing a label in the toolbar should work as normal.

Note: See TracTickets for help on using tickets.