Skip to content

Commit

Permalink
Merge pull request #3426 from alphagov/stop-guessing-image-urls
Browse files Browse the repository at this point in the history
Stop guessing urls for Topical Event image srcset attribute
  • Loading branch information
Tuomas Nylund authored Nov 1, 2023
2 parents a30feec + 3506297 commit f7e5ac4
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 15 deletions.
28 changes: 14 additions & 14 deletions app/models/topical_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,23 @@ def image_url
@content_item.content_item_data.dig("details", "image", "url")
end

def image_alt_text
@content_item.content_item_data.dig("details", "image", "alt_text")
def image_medium_resolution_url
@content_item.content_item_data.dig("details", "image", "medium_resolution_url")
end

def image_high_resolution_url
@content_item.content_item_data.dig("details", "image", "high_resolution_url")
end

def image_srcset
[
{ url: sized_image_url(960), size: "960w" },
{ url: sized_image_url(712), size: "712w" },
{ url: sized_image_url(630), size: "630w" },
{ url: sized_image_url(465), size: "465w" },
{ url: sized_image_url(300), size: "300w" },
{ url: sized_image_url(216), size: "216w" },
]
end

def sized_image_url(width)
image_url.gsub("/s300_", "/s#{width}_")
set = {}
set[image_medium_resolution_url] = "2x" if image_medium_resolution_url
set[image_high_resolution_url] = "3x" if image_high_resolution_url
set
end

def image_alt_text
@content_item.content_item_data.dig("details", "image", "alt_text")
end

def body
Expand Down
8 changes: 7 additions & 1 deletion app/views/topical_events/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,13 @@

<% if @topical_event.image_url %>
<div class="topical-events__image">
<%= image_tag(@topical_event.image_url, alt: @topical_event.image_alt_text, srcset: { @topical_event.image_srcset[2][:url] => "2x", @topical_event.image_srcset[0][:url] => "3x",}) %>
<%=
if @topical_event.image_srcset.any?
image_tag(@topical_event.image_url, alt: @topical_event.image_alt_text, srcset: @topical_event.image_srcset)
else
image_tag(@topical_event.image_url, alt: @topical_event.image_alt_text)
end
%>
</div>
<% end %>

Expand Down
17 changes: 17 additions & 0 deletions spec/features/topical_event_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,23 @@
expect(page).to have_css("img[src='https://www.gov.uk/some-image.png'][alt='Text describing the image']")
end

it "includes image srcset for various image sizes" do
visit base_path
expect(page).to have_css("img[srcset='https://www.gov.uk/some-medium-image.png 2x, https://www.gov.uk/some-high-image.png 3x']")
end

context "when the image has no variants" do
before do
content_item["details"]["image"].delete("medium_resolution_url")
content_item["details"]["image"].delete("high_resolution_url")
stub_content_store_has_item(base_path, content_item)
end
it "omits srcset if there are no higher res image sizes" do
visit base_path
expect(page).not_to have_css("img[srcset]")
end
end

it "sets the body text" do
visit base_path
expect(page).to have_text(content_item.dig("details", "body"))
Expand Down
2 changes: 2 additions & 0 deletions spec/fixtures/content_store/topical_event.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
"body": "This is a very important topical event.",
"image": {
"url": "https://www.gov.uk/some-image.png",
"medium_resolution_url": "https://www.gov.uk/some-medium-image.png",
"high_resolution_url": "https://www.gov.uk/some-high-image.png",
"alt_text": "Text describing the image"
},
"end_date": "2016-04-28T00:00:00+00:00",
Expand Down

0 comments on commit f7e5ac4

Please sign in to comment.