From 5910b6821bed8c9efb8cec5ffe744d8177784de4 Mon Sep 17 00:00:00 2001 From: Anika Henke Date: Thu, 18 Jan 2018 18:50:29 +0000 Subject: [PATCH] Remove 'placeholder' image alt texts If no image has been selected for news articles, a placeholder image gets added instead. And when that placeholder image is used, its alt text reads 'placeholder'. That is bad for screen reader users as that content doesn't make sense. It's much better to leave it empty so screen readers can ignore it. --- app/models/classification_featuring.rb | 3 ++- app/models/feature.rb | 3 ++- app/models/image.rb | 2 +- app/models/take_part_page.rb | 2 +- app/presenters/lead_image_presenter_helper.rb | 2 +- lib/sync_checker/formats/news_article_check.rb | 2 +- lib/sync_checker/formats/world_location_news_article_check.rb | 2 +- test/unit/lead_image_presenter_helper_test.rb | 2 +- .../presenters/publishing_api/case_study_presenter_test.rb | 2 +- .../world_location_news_article_presenter_test.rb | 2 +- 10 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/models/classification_featuring.rb b/app/models/classification_featuring.rb index 33a342f9d90..4073458a12f 100644 --- a/app/models/classification_featuring.rb +++ b/app/models/classification_featuring.rb @@ -6,7 +6,8 @@ class ClassificationFeaturing < ApplicationRecord accepts_nested_attributes_for :image, reject_if: :all_blank - validates :image, :alt_text, presence: true + validates :image, presence: true + validates :alt_text, presence: true, allow_blank: true validates :alt_text, length: { maximum: 255 } validates :classification, :ordering, presence: true diff --git a/app/models/feature.rb b/app/models/feature.rb index c9e46836819..054f07f006b 100644 --- a/app/models/feature.rb +++ b/app/models/feature.rb @@ -7,7 +7,8 @@ class Feature < ApplicationRecord mount_uploader :image, ImageUploader, mount_on: :carrierwave_image validates :document, presence: true, unless: ->(feature) { feature.topical_event_id.present? || feature.offsite_link_id.present? } validates :started_at, presence: true - validates :image, :alt_text, presence: true, on: :create + validates :image, presence: true, on: :create + validates :alt_text, presence: true, allow_blank: true, on: :create validates :alt_text, length: { maximum: 255 } validates_with ImageValidator, method: :image, size: [960, 640], if: :image_changed? diff --git a/app/models/image.rb b/app/models/image.rb index 4f71c555bbb..9d50679e5fb 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -2,7 +2,7 @@ class Image < ApplicationRecord belongs_to :image_data belongs_to :edition - validates :alt_text, presence: true, length: { maximum: 255 }, unless: :skip_main_validation? + validates :alt_text, presence: true, allow_blank: true, length: { maximum: 255 }, unless: :skip_main_validation? validates :image_data, presence: { message: 'must be present' } after_destroy :destroy_image_data_if_required diff --git a/app/models/take_part_page.rb b/app/models/take_part_page.rb index 61ddea0e674..6dc521b2d64 100644 --- a/app/models/take_part_page.rb +++ b/app/models/take_part_page.rb @@ -13,7 +13,7 @@ class TakePartPage < ApplicationRecord mount_uploader :image, ImageUploader, mount_on: :carrierwave_image validates :image, presence: true, on: :create - validates :image_alt_text, presence: true, length: { maximum: 255 }, on: :create + validates :image_alt_text, presence: true, allow_blank: true, length: { maximum: 255 }, on: :create validates_with ImageValidator, method: :image, size: [960, 640], if: :image_changed? include Searchable diff --git a/app/presenters/lead_image_presenter_helper.rb b/app/presenters/lead_image_presenter_helper.rb index 97bf1e84e66..a8692340cd6 100644 --- a/app/presenters/lead_image_presenter_helper.rb +++ b/app/presenters/lead_image_presenter_helper.rb @@ -11,7 +11,7 @@ def lead_image_alt_text if images.first images.first.alt_text.squish else - 'placeholder' + '' end end diff --git a/lib/sync_checker/formats/news_article_check.rb b/lib/sync_checker/formats/news_article_check.rb index 59b36b4ed3a..e50043821bd 100644 --- a/lib/sync_checker/formats/news_article_check.rb +++ b/lib/sync_checker/formats/news_article_check.rb @@ -72,7 +72,7 @@ def expected_image(news_article) image_alt_text = if first_image first_image.alt_text.squish else - 'placeholder' + '' end image_caption = first_image.try(:caption).try(:strip).presence diff --git a/lib/sync_checker/formats/world_location_news_article_check.rb b/lib/sync_checker/formats/world_location_news_article_check.rb index a8c88377928..cdf07321256 100644 --- a/lib/sync_checker/formats/world_location_news_article_check.rb +++ b/lib/sync_checker/formats/world_location_news_article_check.rb @@ -67,7 +67,7 @@ def expected_image(world_location_news_article) image_alt_text = if first_image first_image.alt_text.squish else - 'placeholder' + '' end image_caption = first_image.try(:caption).try(:strip).presence diff --git a/test/unit/lead_image_presenter_helper_test.rb b/test/unit/lead_image_presenter_helper_test.rb index 244ce551895..a66965e7fbc 100644 --- a/test/unit/lead_image_presenter_helper_test.rb +++ b/test/unit/lead_image_presenter_helper_test.rb @@ -4,7 +4,7 @@ class LeadImagePresenterHelperTest < ActiveSupport::TestCase test "should use placeholder image if none had been uploaded" do presenter = stub("Target", images: [], lead_organisations: [], organisations: []).extend(LeadImagePresenterHelper) assert_match %r[placeholder], presenter.lead_image_path - assert_equal 'placeholder', presenter.lead_image_alt_text + assert_equal '', presenter.lead_image_alt_text end test "should use first image with version :s300 if an image is present" do diff --git a/test/unit/presenters/publishing_api/case_study_presenter_test.rb b/test/unit/presenters/publishing_api/case_study_presenter_test.rb index 9ea9ca06387..2bec209048a 100644 --- a/test/unit/presenters/publishing_api/case_study_presenter_test.rb +++ b/test/unit/presenters/publishing_api/case_study_presenter_test.rb @@ -106,7 +106,7 @@ def present(edition) expected_hash = { url: (Whitehall.public_asset_host + organisation_image.file.url(:s300)), - alt_text: 'placeholder', + alt_text: '', caption: nil } presented_item = present(case_study) diff --git a/test/unit/presenters/publishing_api/world_location_news_article_presenter_test.rb b/test/unit/presenters/publishing_api/world_location_news_article_presenter_test.rb index 695cd208ecb..3578b3018c7 100644 --- a/test/unit/presenters/publishing_api/world_location_news_article_presenter_test.rb +++ b/test/unit/presenters/publishing_api/world_location_news_article_presenter_test.rb @@ -329,7 +329,7 @@ class PublishingApi::WorldLocationNewsArticlePlaceholderImageTest < ActiveSuppor test "includes a placeholder image when no image is presented" do expected_placeholder_image = { - alt_text: "placeholder", + alt_text: "", caption: nil, url: Whitehall.public_asset_host + "/government/assets/placeholder.jpg" }