Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Page <title> is incomplete on user input screen #6668

Closed
aaemnnosttv opened this issue Mar 2, 2023 · 4 comments
Closed

Page <title> is incomplete on user input screen #6668

aaemnnosttv opened this issue Mar 2, 2023 · 4 comments
Labels
Exp: SP P0 High priority PHP Type: Bug Something isn't working

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Mar 2, 2023

Bug Description

The page title tag does not display properly on the user input screen. This is due to the screen not having a parent page in the admin menu.

Steps to reproduce

  1. Go to user input screen
  2. See page title is incomplete

Screenshots

Dashboard
image

User Input
image

Additional Context

This was actually raised a very long time ago in v1 in #2883 but it got missed and forgotten about because the feature was stalled.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The <title> element on the User Input screen should use the same format as all other Site Kit screens (i.e. if the User Input page was called "Dashboard", it should produce a page title that matches the main SK dashboard)
    • Note this is largely due to the way WP core works with "private" admin screens like this

Implementation Brief

  • In includes/Core/Admin/Screens.php:
    • Update the get_screens() method so that the googlesitekit-user-input Screen entry to the returned array has a non-existent parent_slug in its arguments, e.g. googlesitekit_hidden.

Test Coverage

  • Since the above approach is not a documented and supported WordPress feature, there is a chance this might not work in the future. In order to keep an eye out, please add test cases that cover this approach and ensure the screen shows up as expected (with no menu item but an appropriate <title> tag).

QA Brief

Using the latest version of the plugin build form the main branch, perform the following steps:

  • Enable the userInput feature flag. This can be done via the Tester Settings sub menu item of Google Site Kit when the tester plugin is enabled.
  • Visit the user-input screen page, this can be done manually by navigating to <your testing URL>/wp-admin/admin.php?page=googlesitekit-user-input where <your testing URL> is the URL of whatever local or remote instance you are testing on.
  • Validate the <title> element for the page. This can be done via visual inspection on the browser tab, or (ideally) via the developer tools console under elements. It should no longer contain an empty element, and should be of the following pattern: Site Kit by Google User Input ‹ sitekit.10uplabs.com — WordPress, although your testing instance will likely replace sitekit.10uplabs.com with your testing domain or site name if changed.

The important validate is that the title is no longer broken in the sense that the first set of wording before the first was previously blank. Using the example steps above, in this case the old title element would have been as follows:

<title>  ‹ sitekit.10uplabs.com — WordPress</title>

But following the fix is now the following:

<title>Site Kit by Google User Input  ‹ sitekit.10uplabs.com — WordPress</title>

Changelog entry

  • Correct page titles on screens that don't exist in the admin menu.
@aaemnnosttv aaemnnosttv added Type: Bug Something isn't working P1 Medium priority labels Mar 2, 2023
@aaemnnosttv aaemnnosttv self-assigned this Mar 2, 2023
@kuasha420
Copy link
Collaborator

#5998 is related to this.

@aaemnnosttv aaemnnosttv removed their assignment Mar 27, 2023
@nfmohit nfmohit self-assigned this Apr 4, 2023
@nfmohit
Copy link
Collaborator

nfmohit commented Apr 4, 2023

A simple solution could be using a non-existent parent slug for the hidden menu items, e.g. googlesitekit_hidden. With this approach, WordPress cannot find a parent menu to attach it to, however, the direct link still works and so does its <title>.

Example:

if ( Feature_Flags::enabled( 'userInput' ) ) {
	$screens[] = new Screen(
		self::PREFIX . 'user-input',
		array(
			'title'            => __( 'User Input', 'google-site-kit' ),
			'capability'       => Permissions::MANAGE_OPTIONS,
			'parent_slug'      => Screen::MENU_SLUG . '_hidden',
			'enqueue_callback' => function( Assets $assets ) {
				$assets->enqueue_asset( 'googlesitekit-user-input' );
			},
			'render_callback'  => function( Context $context ) {
				?>

				<div id="js-googlesitekit-user-input" class="googlesitekit-page"></div>

				<?php
			},

		)
	);
}

(draft IB added)

@aaemnnosttv Any thoughts?

@nfmohit nfmohit added the Exp: SP label Apr 4, 2023
@nfmohit nfmohit assigned aaemnnosttv and nfmohit and unassigned nfmohit and aaemnnosttv Apr 5, 2023
@aaemnnosttv
Copy link
Collaborator Author

The estimate looks a bit high at first glance but guessing that's more/less entirely for test coverage which should be more than enough. Thanks for the elegant solution here @nfmohit 👍

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment Apr 6, 2023
@10upsimon 10upsimon self-assigned this May 28, 2023
@10upsimon 10upsimon removed their assignment Jun 5, 2023
@10upsimon 10upsimon assigned aaemnnosttv and unassigned 10upsimon Jun 6, 2023
@aaemnnosttv aaemnnosttv assigned 10upsimon and unassigned aaemnnosttv Jun 6, 2023
@10upsimon 10upsimon assigned 10upsimon and aaemnnosttv and unassigned 10upsimon Jun 6, 2023
@10upsimon 10upsimon removed their assignment Jun 6, 2023
@aaemnnosttv aaemnnosttv removed their assignment Jun 7, 2023
@mohitwp mohitwp self-assigned this Jun 7, 2023
@mohitwp
Copy link
Collaborator

mohitwp commented Jun 7, 2023

QA Update ✅

  • Tested on dev environment.
  • Verified user input screen title changed as per AC.

image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Exp: SP P0 High priority PHP Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants