Problem/Motivation

Spotted while reviewing #3023638: Remove unused 'options' from SystemManager::getAdminBlock, template_preprocess_admin_block_content() has this docblock:

 * @param $variables
 *   An associative array containing:
 *   - content: An array containing information about the block. Each element
 *     of the array represents an administrative menu item, and must at least
 *     contain the keys 'title', 'link_path', and 'localized_options', which are
 *     passed to l(). A 'description' key may also be provided.

This is out of date. l() doesn't even exist any more and some of the other array key names are wrong.

Similarly this is carried through to templates, system.module's admin-block-content.html.twig mentions:

 * Available variables:
 * - content: A list containing information about the block. Each element
 *   of the array represents an administrative menu item, and must at least
 *   contain the keys 'title', 'link_path', and 'localized_options', which are
 *   passed to l(). A 'description' key may also be provided.

stable and stable9 also have the same docblock, all of these are out of date.

Proposed resolution

Discover the available variables. Fix the preprocess and template documentation to match.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 3174832-7.patch4.28 KBRamya Balasubramanian
#4 3174832-applied_patch.png40.01 KBAbhijith S
#3 3174832-3.patch4.12 KBRamya Balasubramanian

Issue fork drupal-3174832

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Ramya Balasubramanian’s picture

Ramya Balasubramanian CreditAttribution: Ramya Balasubramanian at Srijan | A Material+ Company for Drupal India Association commented
Status: Active » Needs review
FileSize
3174832-3.patch4.12 KB

Hi @longwave,

Uploaded the patch. Please have a look and let me know if there are any issues.

Abhijith S’s picture

Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commented
FileSize
3174832-applied_patch.png40.01 KB

Applied patch #3 and it works fine.The deprecated l() and its description is removed from those templates.There is no white-space found, also the new description given is meaningful in the scenario.

after

I'm moving it to RTBC.

Abhijith S’s picture

Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/system.admin.inc
@@ -18,10 +18,10 @@
+ *   - content: List of administrative menu items. Each menu item contains:
+ *      - url: Path to the admin section.
+ *      - title: Short name of the section.
+ *      - description: Description of the administrative menu item.

I think we're missing options looking at \Drupal\system\SystemManager::getAdminBlock()

Ramya Balasubramanian’s picture

Ramya Balasubramanian CreditAttribution: Ramya Balasubramanian at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3174832-7.patch4.28 KB

Hi @alexpott,

Updated the options. Please have a look.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented
Status: Needs review » Needs work

The documentation is not fully clear:

+ * - options: The array of options.

This doesn't explain what these options are used for. They are in fact not used in the theme layer right now, but we can derive the meaning from MenuLinkInterface. How about this?

+ * - options: Options to use when creating the URL. See \Drupal\Core\Url::fromUri() for details.

pfrenssen opened merge request !383

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented
Status: Needs work » Needs review
pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented

I had a look in the theme layer which context variables are actually available using Twig debugging. The formatted link is there in addition to the url (actually this is a URI not a URL but we cannot change this without breaking B/C) and options components. Updated documentation accordingly.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. RTBC if bot agrees, as this is docs changes only I don't see why it wouldn't.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented

Thanks for the quick review!

  • catch committed 70a3c2c on 9.2.x 
    Issue #3174832 by pfrenssen, Abhijith S, longwave, alexpott: admin-block...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

@Abhijith S there's no need to upload a screenshot to show the patch applying since d.o's testing infrastructure tells us this information already.

Committed f6205a6 and pushed to 9.2.x. Thanks!

  • catch committed 4129e3d on 9.2.x 
    Revert "Issue #3174832 by pfrenssen, Abhijith S, longwave, alexpott:...
  • catch committed f6205a6 on 9.2.x
    Issue #3174832 by pfrenssen, Ramya Balasubramanian, longwave, alexpott:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.