Add WP admin pointer for view-only dashboard access "Site Kit" menu item

[felixarntz]

A WordPress admin pointer should be added that informs users that have received view-only access to the dashboard about the new "Site Kit" admin menu item available to them.

---

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

Acceptance criteria

• A new WP admin pointer should be added that points to the "Site Kit" menu if all of the following circumstances apply:
  • The user is currently on the WordPress dashboard (index.php).
  • The user has only shared / view-only access to Site Kit (i.e. cannot authenticate).
  • The user has dismissed neither that WP admin pointer nor the shared splash dismissal.
• The wording for the pointer should be as follows:
  • Title: You now have access to Site Kit
  • Content: Check Site Kit's dashboard to find out how much traffic your site is getting, your most popular pages, top keywords people use to find your site on Search, and more.
• When the pointer is dismissed, it should be dismissed permanently, i.e. no transient or so.

Implementation Brief

A good reference and starting point would be e.g. https://github.com/WordPress/performance/blob/1.2.0/admin/load.php#L342-L418, copy this code into a more Site Kit-appropriate class structure and tweak it based on the ACs (e.g. add the Site Kit-specific checks).

• The idea is to create a Admin Pointer system in similar way to how Notices are handled in Site Kit source code. Most of the inspiration were drawn from the following classes. Feel free to refer to these classes when something seems unclear.
  • Google\Site_Kit\Core\Admin\Notice
  • Google\Site_Kit\Core\Admin\Notices
  • Google\Site_Kit\Core\Util\Activation_Notice
• Create a new class Google\Site_Kit\Core\Admin\Pointer that represents a Pointer.
  • Have slug and args as private property.
    • slug is the unique Pointer slug.
    • args should contain the following:
      • title: pointer title.
      • content pointer content.
      • target_id: the id where the pointer will be attached. ie. a menu item.
      • active_callback: function to determine whether the pointer should be active. ie. shown.
  • Initialize slug and args in the constructor.
    • Use wp_parse_args to parse the args.
  • Add a public method is_active that:
    • Takes hook_suffix as an argument.
    • Return false if either title or content is empty.
    • Return true if active_callback is not set. ie. null or falsy.
    • Otherwise, return the boolean result of calling avtive_callback with hook_suffix as an argument using the call_user_func.
  • Add a public method enqueue_script:
    • that adds print_script to admin_print_footer_scripts action using add_action.
  • Add print_script method:
    • Get heading by escaping title with esc_js.
    • Get content by:
      • If content is callable, call it using call_user_func.
      • Otherwise, wrap it in a p tag and escape it using wp_kses.
    • Print out the pointer inline script using the BC_Functions::wp_print_inline_script_tag method.
      • Get the script from the link above.
      • Print it as a formatted string using sprintf and interpolate heading, content, slug and target_id accordingly.
• Create a new class Google\Site_Kit\Core\Admin\Pointers which will register and manage all the pointers.
  • Create a public method register:
    • That adds enqueue_pointers to to admin_enqueue_scripts action using add_action.
  • Create enqueue_pointers method:
    • It takes one argument, hook_suffix.
    • Return early if hook_suffix is empty.
    • Get all the pointers using $this->get_pointers function.
    • Return early if there is no pointers.
    • Filter out active_pointers by calling is_active on every pointer and passing hook_suffix to it.
    • Return early if there is no active_pointers.
    • Otherwise, enqueue wp-pointer script and style using wp_enqueue_script and wp_enqueue_style respectively.
    • Call enqueue_script on all active pointers.
  • Create get_pointers method:
    • Get all the registered pointers by calling apply_filters with googlesitekit_admin_pointers filter with an empty array as the default value.
    • Only return pointers that are an instance of Pointer.
• Create Google\Site_Kit\Core\Util\View_Only_Pointer class that will register the View Only Pointer.
  • create a SLUG constant with the value googlesitekit-view-only-pointer.
  • Have dismissed_items and initialize them in the constructor.
  • Create a public register method:
    • Add the view only pointer to the googlesitekit_admin_pointers using add_filter hook.
    • Use get_view_only_pointer to get the Pointer.
  • Create get_view_only_pointer method:
    • It will return a new Pointer with the following:
    • slug => self::SLUG.
    • args =>
      • title => See AC. It should be translated.
      • content => See AC. It should be translated.
      • target_id => toplevel_page_googlesitekit-dashboard
      • active_callback => Should be a function that takes hook_suffix as a parameter and:
        • Returns false if either of the following is true
          • hook_suffix is not index.php.
          • dashboardSharing is not enabled.
          • current user can Authenticate.
          • current user can not view splash.
          • shared_dashboard_splash is dismissed.
        • Get dismissed_wp_pointers using get_user_meta.
        • Return true if dismissed_wp_pointers is empty.
        • Otherwise, explode the dismissed_wp_pointers with , to create an array.
        • Return false if self::slug is in the array.
        • Otherwise, return true.
• In Google\Site_Kit\Plugin class:
  • Instantiate and register the newly added Pointers and View_Only_Pointer class.

Test Coverage

• Add Unit Test for Pointer.
  • Check the enqueued inline script.
• Add Unit Test for Pointers.
  • Register a pointer and see if the inline script and the wp-pointer scripts are added to the admin screens.
• Add a Unit Test for the active_callback function of View_Only_Pointer.

QA Brief

• Enable dashboardSharing.
• Share some modules with non-admin role to give them shared access.
• Log in to a account of aforementioned role.
• You should see a Admin Pointer pointing to the Site Kit menu.
• Match title and content with the AC.

Changelog entry

[felixarntz]

@eclarke1 @FlicHollis Would be great if we could get this added to the upcoming sprint.

@marrrmarrr still needs to add wording to the ACs, but the IB could technically already be written at this point.

[eclarke1]

Thanks @felixarntz I have moved this over to IB so the team know it can be picked up, but will keep assigned to @marrrmarrr as well to finalise the wording there please.

[eclarke1]

Following comms with @marrrmarrr confirming this is a 'Nice to Have' for launch, so dropping down to a P1

[marrrmarrr]

@eclarke1 I've updated the text here, so this can move forward.

[eclarke1]

Just a polite ping on this one please @aaemnnosttv as it's for this sprint 82 and has been in IBR 8 days. Thanks so much. cc @FlicHollis

[aaemnnosttv]

Thanks @eclarke1 – apologies for the delay here. I hadn't forgotten about it 👍

@kuasha420, nice work here – I like the idea to model this after Notices, that should work well I think.

• Get dismissed_wp_pointers using user_options->get.

@kuasha420, while I think using User_Options would work for this, this is not technically a user option and so we should use user meta directly like core does.

Otherwise, the IB looks good, but we should make sure to add test cases for Pointer as well, not just the class for all pointers.

[kuasha420]

@aaemnnosttv Thank you for the review. Glad to know that the idea to model it like Notices was a good idea after all! Initially it kind of felt overkill but my rational was that we may use pointers more in the future, specially with having more of site kit in other places of dashboard (Widget areas, editor etc).

I have updated the IB with your suggestions. Please have a look. Cheers. :smile:

[aaemnnosttv]

IB ✅

[mohitwp]

QA Update ⚠️

@kuasha420

• I found that Pointer Title is getting cut in desktop and laptops. Verified on chrome, safari and edge browser.

• Currently Pointer is not showing in all mobile devices - iPad, iPhone and android mobile/tablets. Is this expected ?

[kuasha420]

@mohitwp Hi, I have done some investigation on why the title is cut off. Looks like the WP pointer puts the arrow in the 50% of the height of the Pointer Content. As the Site Kit Menu is situated very top, the title goes under the Admin Bar. Without messing about too much with the CSS, we need to chose either of the following solution.

Put the Pointer under the menu. (Supported in wp-pointer script)

Increase the z-index of Pointer so it shows up over the Admin Bar. (Need custom CSS)

Which one looks better to you?

As for your second question, I think wp-pointer does not ~~work~~ show up ~~on mobile~~ below a certain viewport resolution, so it not appearing on mobile is expected behavior

CC @wpdarren @felixarntz

[mohitwp]

@kuasha420 I like the second design (increase the z-index) but it requires custom CSS work. @felixarntz @wpdarren can you please provide your suggestions here ?

[wpdarren]

@kuasha420 I feel the call needs to come from @felixarntz because he created the ticket, but I agree with Mohit. I'll post a link in the open Slack channel so we can get this moved forward.

[aaemnnosttv]

@kuasha420 let's opt to go with the alternate placement, putting it under the menu instead. I'm sure core probably has its own pointers that could end up below the admin bar and it was probably an intentional choice at some point to keep these below it (although maybe not). I also think increasing the z-index could make part of it unreadable if the content were longer (due to translation perhaps), because the content is centered on the arrow.

[felixarntz]

@kuasha420 +1 to Evan's point, regardless of which UI we visually prefer, I agree we should follow what WP core already offers us here to avoid risk of other "overlap" issues, so let's go with adjusting the position via what is supported by wp-pointer.

[mohitwp]

QA Admin ✅

Verified :

• Verified A new pointer is added and show up in below listed cases-

  • The user is currently on the WordPress dashboard (index.php).
  • The user has only shared / view-only access to Site Kit (i.e. cannot authenticate).
  • The user has dismissed neither that WP admin pointer nor the shared splash dismissal.

• When the pointer is dismissed, it gets dismissed permanently, i.e. no transient or so.

• Verified content and title.