Problem/Motivation

This is annoying because Lighthouse and similar tools are complaining, and yeah images should have width and height attributes.

CommentFileSizeAuthor
#9 site-logo-width-height-3218479-9.patch2.65 KBpivica
#9 interdiff-8-9.txt1.32 KBpivica
#8 site-logo-width-height-3218479-8.patch2.65 KBpivica
#8 interdiff-3218479-7-8.txt799 bytespivica
#7 site-logo-width-height-3218479-7.patch2.4 KBpivica
#7 interdiff-3218479-6-7.txt2.09 KBpivica
#6 site-logo-width-height-3218479-6.patch2.04 KBpivica
#6 interdiff-3218479-4-6.txt1.78 KBpivica
#4 site-logo-width-height-3218479-4.patch1.87 KBpivica
#2 site-logo-width-height-3218479-2.patch1.14 KBpivica

Comments

pivica created an issue. See original summary.

pivica’s picture

pivica commented
StatusFileSize
new site-logo-width-height-3218479-2.patch1.14 KB

Here is a patch that is adding width and height for all image types except SVG.

pivica’s picture

pivica commented
Status: Active » Needs review
pivica’s picture

pivica commented
StatusFileSize
new site-logo-width-height-3218479-4.patch1.87 KB

Forgot to add changes for the related template.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
+++ b/themes/bs_bootstrap/bs_bootstrap.theme
@@ -306,9 +306,17 @@ function bs_bootstrap_preprocess_block(&$variables) {
+          $image = Drupal::service('image.factory')->get(DRUPAL_ROOT . $variables['site_logo']);

I'd add a file exists check to be extra careful.

pivica’s picture

pivica commented
StatusFileSize
new interdiff-3218479-4-6.txt1.78 KB
new site-logo-width-height-3218479-6.patch2.04 KB

> I'd add a file exists check to be extra careful.

Something like this?

Does it make sense maybe to introduce logo_attributes variable instead of manually rendering attributes in template for this part:

 <img src="{{ site_logo }}" alt="{{ site_logo_alt_text }}" class="site-logo" {{ if site_logo_width ? 'width="' ~ site_logo_width ~ '" height="' ~ site_logo_height ~ '"'}}/>
pivica’s picture

pivica commented
StatusFileSize
new interdiff-3218479-6-7.txt2.09 KB
new site-logo-width-height-3218479-7.patch2.4 KB

This manual attribute rendering is quite tricky because of the auto escaping of quotes in twig. I've added the logo_attributes variable instead, as mentioned in the previous comment.

pivica’s picture

pivica commented
StatusFileSize
new interdiff-3218479-7-8.txt799 bytes
new site-logo-width-height-3218479-8.patch2.65 KB

Based on discussion with @berdir we will also keep the old site_logo_alt_text variable so a custom theme that is using this variable in overridden templates will still work.

pivica’s picture

pivica commented
StatusFileSize
new interdiff-8-9.txt1.32 KB
new site-logo-width-height-3218479-9.patch2.65 KB

This changes introduced a bug that needed CSS class `site-logo` is not applied for SVG logo that is not inline which produce layout problems for mobile clients. Should be fixed in new version.

  • pivica committed 5002875 on 8.x-1.x 
    Issue #3218479 by pivica, Berdir: Site logo image is missing width and...
pivica’s picture

pivica commented
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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