In working on #2766491: Update status should indicate whether installed contributed projects receive security coverage, I used the admin_block theme, which comes with a light grey background. For that issue, more-flexible background options are needed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drumm created an issue. See original summary.

drumm’s picture

Status: Active » Needs review
FileSize
2.74 KB

This patch adds attributes array processing with template_preprocess_admin_block and moves colors.css later in the cascade so its background takes precedence.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

drumm’s picture

This currently applies cleanly to 8.5.x.

star-szr’s picture

Status: Needs review » Needs work

Thanks @drumm, overall looks very good!

  1. +++ b/core/modules/system/system.admin.inc
    @@ -10,6 +10,21 @@
    + * @param $variables
    

    Minor: missing data type.

  2. +++ b/core/modules/system/templates/admin-block.html.twig
    @@ -14,7 +14,7 @@
    -<div class="panel">
    +<div{{ block.attributes }}>
    
    +++ b/core/themes/stable/templates/admin/admin-block.html.twig
    @@ -12,7 +12,7 @@
    -<div class="panel">
    +<div{{ block.attributes }}>
    

    For these we can use the pattern established in Classy like so:

    {%
      set classes = [
        'panel',
      ]
    %}
    <div{{ block.attributes.addClass(classes) }}>
    
  3. +++ b/core/themes/seven/seven.libraries.yml
    @@ -10,7 +10,6 @@ global-styling:
    -      css/components/colors.css: {}
    
    @@ -23,6 +22,7 @@ global-styling:
    +      css/components/colors.css: {}
    

    Maybe move this to the 'theme' group for the added specificity?

star-szr’s picture

+++ b/core/modules/system/system.admin.inc
@@ -10,6 +10,21 @@
+  $variables['block']['attributes'] = new Attribute($variables['block']['attributes']);

On second thought, we could probably just do $variables['attributes'] here, then we wouldn't need the new Attribute() part and the template would just print attributes.

The calling code would then move the attributes up to the top level in its own hash key.

To do this, I believe we'd need to update the hook_theme definition to add attributes => [] to variables.

drumm’s picture

Status: Needs work » Needs review
FileSize
3.01 KB

The attached patch fixes all the suggestions and is working well. Thanks for the review!

star-szr’s picture

Title: Allow color-* to be used in admin blocks » Allow attributes to be passed to admin blocks (admin_block theme hook)
Assigned: drumm » Unassigned
Status: Needs review » Reviewed & tested by the community
Issue tags: +Twig

Looks great, and I tested along with the latest patch in #2766491: Update status should indicate whether installed contributed projects receive security coverage and it works well. The markup for existing uses of this template remains the same so this is a safe change to make in the Stable theme. Thanks!

larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed aaa668a and pushed to 8.5.x. Thanks!

  • larowlan committed aaa668a on 8.5.x
    Issue #2887860 by drumm, Cottser: Allow attributes to be passed to admin...

Status: Fixed » Closed (fixed)

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