Enable ALT text for Banner Block image field.

Thanks for the patch @owenpm3!

This was previously reported at https://github.com/gareth-fivemile/demo_umami/issues/48

I've closed that Github issue in favour of this one here on d.o

I can't recall what the image showed for the specimen block, or tested the patch, so the alt text it still needs review.

IIRC the theme renders the banner block image as a CSS background-image, so we'll need to find an approach to convey the text-alternative. Let's do the theming in a follow-up, and keep this issue about the profile's config and content.

Hi Andrew and Owen,

Andrew is correct, the image is a background image added via CSS, so although the alt tag is currently in the HTML it is not visible to screen readers since it's hidden behind display: none

Sorry, the rest of my comment seems to have been removed:

Hi Andrew and Owen,

Andrew is correct, the image is a background image added via CSS, so although the alt tag is currently in the HTML it is not visible to screen readers since it's hidden behind display: none as we can see in this screenshot:

I'm not sure what the solution is, but I reckon we need to display the image on the page (probably using a responsive image) and then use position: absolute on the content inside the banner.

The patch itself looks good in that it makes the alt field required and does provide an alt text.

Repeating "Super easy vegetarian pasta bake" three times isn't very useful. If we can provide a text alternative which conveys the image content (and mood), rather than repeat a heading, so much the better.

I wonder if the banner block type is a place we could enable the alt text, but not require it. That would serve to demonstrate how image fields can be configured in different ways. Compare it with the article and recipe content types, where the alt text is required.

At the moment the image field is set to display: none on large screens. If we set that to display: block and set the width/height to 0 that would allow the alt to be read by screenreaders (I think?)

On small screens we are printing the image field so the alt text should be readable there - not withstanding the fact that we don't actually have any alt text there in the first place.

In the mean time, here's a patch that adds alt text to the banner image.

sorry, I forgot this should be part of the content installer module. ignore this patch.

Yes @mark, I was to say that about #11. #2 applies cleanly. I am also not sure the correct approach to follow. As of now, the image that is being shown in screen comes from parent div with background-image CSS property applied to it. The inner field <img> tag is hidden. #2 does provide ALT tag for the inner img tag.

Thanks @owenpm3 for patch !! Not changing the status, Need more reviews.

Thanks for the patch

1.    default_image:
        uuid: null
   -    alt: ''
   +    alt: 'A mouth-watering, easy to prepare, vegetarian pasta bake'

   This sets the phrase as the default for all new Banner Block content entities. If a demo user is learning how the views landing page is put together, and tries to create a brand new Banner Block, it will be pre-populated with an inappropriate alt text.
   Instead, set the the alt text for this specific block entity during content inport, at Drupal\demo_umami_content\InstallHelper::importBlockContent() .

2. The alt text itself: "A mouth-watering, easy to prepare, vegetarian pasta bake".
   • "Easy to prepare" isn't something you can tell by looking at the photo. When writing alt text, be careful not to include information that isn't actually conveyed by the image itself. It's also repeating something that was said in field_summary.
   • "Mouth-watering" is great, let's keep that. It's the author's opinion of the photo, and a very reasonable mood to convey.
   • We could describe the appearance of the food, like the cheese and herbs topping, or the rich tomato sauce. We might also something about it being served in a white casserole dish, or the distressed (rustic?) tabletop, as the setting for the meal.

Writing good alt text is a tricky art sometimes, so this is just my feedback and ideas. There's no right answer though.

@andrewmacpherson I am 100% agreed to you.

Looks good. Thanks @navneet.

We can look at reviewing the specific alt text in Review ALT text for images Umami Demo Content.

Thanks Navneet.

Patch #16 looks good to me, and addresses all concerns form #14.

I tested this manually with an Umami Demo install, and confirmed that the ALT text is correctly imported for the single instance of Banner Block content entity, and the configuration of field_banner_image is correct.

"Mouth watering vegetarian pasta bake with rich tomato sauce and cheese toppings" - great, that sounds good to me. It describes the photo well, and avoids repeating nearby visible text.

@owenpm3 - I'm happy making the ALT text non-required in this case:

• It serves as a demonstration that image fields are configurable for accessibility purposes. Compare with user images, where alt text is not enabled at all, and recipes where it is mandatory.
• We only have a single instance of this block in our specimen content. It's not clear how many possible use-cases such a banner block may have, and I can imagine it being used with a simple decorative pattern image in the background, rather than being expected to have a photo.

Clarifying the title. The scope of this issue is to fix the configuration and content import.

Fixing how the theme renders the ALT text when treating the content as a CSS background image may be tricky, so I'll file a follow-up issue for that.

1. +++ b/core/profiles/demo_umami/config/install/field.field.block_content.banner_block.field_banner_image.yml
   @@ -22,8 +22,8 @@ settings:
   +  alt_field_required: false
   

   I think we should make this true - we're using this profile to demo features of Drupal so should focus on best practices.

   In this instance, we'd advocate that requiring an alt tag here is the best practice right?

2. +++ b/core/profiles/demo_umami/modules/demo_umami_content/src/InstallHelper.php
   @@ -310,6 +310,7 @@ protected function importBlockContent() {
   +          'alt' => 'Mouth watering vegetarian pasta bake with rich tomato sauce and cheese toppings',
   

   mmm

Can we add to the existing text?

We're already asserting the image is there, so should be easy to assert the alt tag in \Drupal\Tests\demo_umami_content\Functional\UninstallDefaultContentTest::assertImportedCustomBlock

This looks good to go @navneet !! resolves mentioned points in #23. Thanks!

re: #23.1

+ alt_field_required: false
I think we should make this true - we're using this profile to demo features of Drupal so should focus on best practices.

In this instance, we'd advocate that requiring an alt tag here is the best practice right?

Best practice is to provide appropriate text alternatives for non-text content. Typically that's done through an alt attribute on an image element, but it's not the only technique. The key word here is appropriate - there are several well-attested uses for omitting a text alternative, and the HTML 5 and WCAG 2.0 recommendations reflect this.

However making a text alternative mandatory for every image field in the CMS is not a best practice in itself. If it were, then Drupal wouldn't need to provide the alt_field_required configuration option at all. We sometimes hear reports from well-informed users who are confounded by having to provide a text alternative for a decorative image which doesn't need one. It's also common to encounter alt attributes that just repeat nearby text, which is overly verbose. The point of the alt_field and alt_field_required configuration options are to give site builders and/or content authors some discretion about how their content model will treat text alternatives. Drupal provides powerful site-builder tools, but accessible content is ultimately down to editorial practice and accessibility awareness.

My thinking in #8 and #19 was that Umami profile could demonstrate all the ways to configure image field text alternatives:

• User images: alt field not enabled. (The theme can output a blank alt attribute, or populate it with an author name token.)
• Recipe content type: alt field enabled and required. (Has a clearly defined purpose - it shows what the dish looks like, and we'd like to convey that to everyone.)
• Banner Block: alt field enabled, but not required.

With the Banner Block type, the use case hasn't been so strongly defined. It seems to be a general purpose banner for special landing pages, but we only have one example so far. There are likely to be fewer instances of these banner blocks than recipes or articles, and they won't always represent recipes (or any particular topic). Making the text alternative optional allows for the use of decorative banner images which don't convey any important information, at the discretion of the content author.

re: #23.2

Can we add to the existing text?

Can you clarify what you mean here?

Andrew,

Should this be set to 'Needs work' or 'Needs review'? You have it set to 'Needs review' but I'm not sure how many more people are going to review it. Both versions of the config (with alt text required, with alt text not required) have been set to RTBC on this, so I think we're at an impasse: do we do more work (set it back to not required, and mark it RTBC) or keep it as required (and mark it as RTBC)?

I think we should go with alt text not required for banner block image.

Are we happy with the patch from #16 so?

No, I think it needs the config + content from patch #16, and the test assertion from patch #24

1. +++ b/core/profiles/demo_umami/config/install/field.field.block_content.banner_block.field_banner_image.yml
   @@ -23,7 +23,7 @@ settings:
   -  alt_field_required: false
   +  alt_field_required: true
   

   Thanks @andrewmacpherson for the detailed comment on why this was set to false originally. @navneet0693 sorry for the misdirection, can you change this back how you had it inline with @andrewmacpherson's comment.

   Thanks!

2. +++ b/core/profiles/demo_umami/modules/demo_umami_content/tests/src/Functional/UninstallDefaultContentTest.php
   @@ -99,6 +99,8 @@ protected function assertImportedCustomBlock(EntityStorageInterface $block_stora
   +    $img_alt_text = $assert->elementExists('xpath', '//*[@id="block-umami-banner-recipes"]/div/div[1]/img')->getAttribute('alt');
   

   This is very specific and therefore likely to be fragile. e.g. If another div is added, this will break.

   Can you instead use $this->elementExists('css', '#block-umami-banner-recipes img') which is less prone to being broken by minor markup changes.

3. +++ b/core/profiles/demo_umami/modules/demo_umami_content/tests/src/Functional/UninstallDefaultContentTest.php
   @@ -99,6 +99,8 @@ protected function assertImportedCustomBlock(EntityStorageInterface $block_stora
   +    $this->assertEquals('Mouth watering vegetarian pasta bake with rich tomato sauce and cheese toppings', $img_alt_text, 'Image alt is matching');
   

   I don't think you need the third argument here, if the test fails you will see 'Image alt is matching' which is not as good as the default behaviour which will be 'failed asserting that `whatever it found that was wrong` equals `Mouth watering vegetarian...`'. I think the default makes it much easier to debug the failure.

Per #26-31, this patch combines..

• the config from patch #16 (alt field enabled, but not required)
• the installer content (pasta bake image alt text)
• the test assertion from patch #24

So it just makes the alt text optional again.

Still TODO: points 2 and 3 from #32

This addresses #32, points 2 and 3 about the test.

I think this should be good now ;-)

Committed as 056e61b and pushed to 8.6.x.