Skip to content

Commit

Permalink
Remove 'placeholder' image alt texts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
selfthinker committed Jan 19, 2018
1 parent ab4acbd commit 5910b68
Show file tree
Hide file tree
Showing 10 changed files with 12 additions and 10 deletions.
3 changes: 2 additions & 1 deletion app/models/classification_featuring.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion app/models/feature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down
2 changes: 1 addition & 1 deletion app/models/image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/models/take_part_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/lead_image_presenter_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def lead_image_alt_text
if images.first
images.first.alt_text.squish
else
'placeholder'
''
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/sync_checker/formats/news_article_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/unit/lead_image_presenter_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down

0 comments on commit 5910b68

Please # to comment.