Add readability comment to $title variable used in admin-header

[ravipatel]

Add a readability comment above each instance of $title = __( '' ) in each admin panel's file. This $title is used to populate the <title> element which is built in the reusable wp-admin/admin-header.php file.

The comment will avoid confusion on panel file's where the translated content reads the same for the <title> and <h1>. For example, the confusion can occur in the wp-admin/privacy.php:

$title = __( 'Privacy' );
...
require_once ABSPATH . 'wp-admin/admin-header.php';
...
<div class="about__header-title">
        <h1>
                <?php _e( 'Privacy' ); ?>
        </h1>
</div>

[ravipatel]

Code Re-usablity

[audrasjb]

Hello, thanks for the ticket and the patch.

_e() function is meant to display thing, so you shouldn't write $title = _e( 'Privacy' );.

Plus, if the $title variable is never used, shouldn't we just get rid of it?

[SergeyBiryukov]

Hi there, thanks for the patch!

It might not be immediately clear, but the $title variable is used in wp-admin/admin-header.php.

This is a common pattern used in many admin files, not just in wp-admin/privacy.php:

$title = __( ... );
...
require_once ABSPATH . 'wp-admin/admin-header.php';

Looking at the About page and related pages:

• wp-admin/about.php:

  $title = _x( 'About', 'page title' );
  ...
  require_once ABSPATH . 'wp-admin/admin-header.php';
  ...
  <div class="about__header-title">
  	<h1>
  		<?php _e( 'WordPress' ); ?>
  		<?php echo $display_version; ?>
  	</h1>
  </div>
  

• wp-admin/credits.php:

  $title = __( 'Credits' );
  ...
  require_once ABSPATH . 'wp-admin/admin-header.php';
  ...
  <div class="about__header-title">
  	<h1>
  		<?php _e( 'Contributors' ); ?>
  	</h1>
  </div>
  

• wp-admin/freedoms.php:

  $title = __( 'Freedoms' );
  ...
  require_once ABSPATH . 'wp-admin/admin-header.php';
  ...
  <div class="about__header-title">
  	<h1>
  		<?php _e( 'The Four Freedoms' ); ?>
  	</h1>
  </div>
  

• wp-admin/privacy.php:

  $title = __( 'Privacy' );
  ...
  require_once ABSPATH . 'wp-admin/admin-header.php';
  ...
  <div class="about__header-title">
  	<h1>
  		<?php _e( 'Privacy' ); ?>
  	</h1>
  </div>
  

So the Privacy page is the only one where $title is the same as the <h1> tag. We could technically reuse the $title variable for the <h1> tag here, but that would make it a bit inconsistent with the other pages.

The existing code seems good to me as is.

[sabernhardt]

The About page includes a translator comment that helps give context when reading the file, too.

/* translators: Page title of the About WordPress page in the admin. */
$title = _x( 'About', 'page title' );

To explain the variable in each of the three related files, perhaps we could add a regular comment as simple as:

// Used in HTML title tag.
$title = __( 'Privacy' );

[hellofromTonya]

The $title variable is for populating the <title> tag and not for the panel's <h1>. The <title> is built in wp-admin/admin-header.php. Why is this variable in each panel's file and not assigned in the admin header file? For reusability. It allows each panel to assign its title for use in building the <title>.

Should this variable be used to also populate the <h1>?

No, I don't think so. I agree with @SergeyBiryukov. Why?

As Sergey notes, the <h1> is different than the <title> on many of the panels. Using it for dual purpose on some panels but not others will be confusing to contributors as it breaks the pattern.

At the same time, I can also see confusion when reading it on the Privacy panel and it's the same. It looks like duplicate code. Right? I definitely get why this ticket was created.

What could we do to help with the readability and avoid this type of confusion?

Add an inline comment as suggestions:

To explain the variable in each of the three related files, perhaps we could add a regular comment as simple as:

// Used in HTML title tag.
$title = __( 'Privacy' );

Patch Notes

Also note @ravipatel in 53729.patch​, the title variable would need to be echoed. Thought I'd note that as a learning opportunity for anyone reading this ticket.

Next Steps

Add comments above each $title = ... for readability. @ravipatel would you like to create the patch for this?

[ravipatel]

@hellofromTonya yes i will do patch for this.

[hellofromTonya]

w00t!

Replying to ravipatel:

@hellofromTonya yes i will do patch for this.

Updated the ticket's title and description to match the new scope of this ticket (helps in discoverability).

Also moving into the 5.9 milestone.

[ravipatel]

Added new patch with comment as per suggested.

[hellofromTonya]

Great job @ravipatel! 53729.2.patch​ looks good to me. Marking for commit.

[SergeyBiryukov]

It looks like 53729.2.patch​ is a good start but only patches a portion of the files.

53729.3.diff​ makes the change consistently for all the files in wp-admin.

[SergeyBiryukov]

In 51475:

Docs: Add a comment about the $title global usage in various admin files.

This should make it clear that the variable is used as part of the HTML <title> tag on admin screens.

Props ravipatel, hellofromTonya, sabernhardt, audrasjb, SergeyBiryukov.
Fixes #53729.

[desrosj]

In 51499:

Coding Standards: Apply some alignment fixes from composer format.

Follow up to [51475].

See #53729.