Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ window.GOVUK.Modules = window.GOVUK.Modules || {}

updateImageStatus(replacementEl) {
if (replacementEl) {
this.$root.replaceChild(replacementEl, this.imageStatus)
this.imageStatus.replaceWith(replacementEl)
} else {
this.imageStatus.remove()
}
Expand Down
43 changes: 28 additions & 15 deletions app/components/admin/edition_images/image_card_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,15 +1,28 @@
<%= render "govuk_publishing_components/components/summary_card", {
id: "uploaded_#{image_usage.key}_image_card",
summary_card_actions: summary_card_actions,
title: image_usage.title.upcase_first,
rows: [
{
key: "Image",
value: thumbnail,
},
image_usage.caption_enabled? ? {
key: "Caption",
value: caption,
} : nil,
].compact,
} %>
<li
data-module="image-processing-checker"
data-variant="s960"
<%= "data-image-link=#{admin_edition_image_path(edition, image)}" if image.present? %>>
<template class="js-image-preview">
<img src="" alt="" class="app-view-edition-resource__preview">
</template>

<%= render "govuk_publishing_components/components/summary_card", {
id: "uploaded_#{image_usage.key}_image_card",
summary_card_actions: summary_card_actions,
title: image_usage.title.upcase_first,
rows: [
{
key: "Image",
value: thumbnail,
},
image_usage.caption_enabled? ? {
key: "Caption",
value: caption,
} : nil,
image_usage.embeddable? ? {
key: "Markdown code",
value: markdown_code,
} : nil,
].compact,
} %>
</li>
18 changes: 17 additions & 1 deletion app/components/admin/edition_images/image_card_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,26 @@ def caption
def thumbnail
return "Not set" if image.blank?

return "<span class=\"govuk-tag govuk-tag--green\">Processing</span>".html_safe unless image.image_data&.original_uploaded? && image.thumbnail
return "<div class=\"js-image-processing-status\"><span class=\"govuk-tag govuk-tag--green\">Processing</span></div>".html_safe unless image.image_data&.original_uploaded? && image.thumbnail

return "<span class=\"govuk-tag govuk-tag--red\">Requires cropping</span>".html_safe if image.requires_crop?

sanitize("<img style=\"width: 100%;\" src=\"#{image.thumbnail}\" alt=\"\" class=\"app-view-edition-resource__preview\">")
end

def markdown_code
render("govuk_publishing_components/components/copy_to_clipboard", {
label: tag.span("Markdown code:", class: "govuk-visually-hidden"),
copyable_content: image_markdown,
button_text: "Copy Markdown",
})
end

def image_markdown
edition.images_have_unique_filenames? ? "[Image: #{image.filename}]" : "!!#{find_image_index + 1}"
end

def find_image_index
edition.images.find_index(image)
end
end
36 changes: 11 additions & 25 deletions app/components/admin/edition_images/image_upload_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,32 +29,18 @@
} %>

<%= hidden_field_tag("usage", image_usage.key) %>
<%= hidden_field_tag("image_kind", image_kind.name) %>

<% if image_usage.kinds.size > 1 %>
<%= render "govuk_publishing_components/components/radio", {
heading: "What kind of image is this?",
name: "image_kind",
items: image_usage.kinds.map do |image_kind|
{
value: image_kind.name,
text: image_kind.display_name,
}
end,
} %>
<% else %>
<%= hidden_field_tag("image_kind", image_kind.name) %>

<% if image_kind.allowed_formats.include?("svg") %>
<%= render "govuk_publishing_components/components/details", {
title: "You must use an SVG for charts and diagrams",
} do %>
SVGs allow users to magnify images without losing quality.
Find out <%= link_to "how to create an SVG file (opens in new tab)",
"https://guidance.publishing.service.gov.uk/formatting-content/images-videos/formatting-images/#how-to-create-an-svg-file",
class: "govuk-link",
target: "_blank",
rel: "noopener" %>.
<% end %>
<% if image_kind.allowed_formats.include?("svg") %>
<%= render "govuk_publishing_components/components/details", {
title: "You must use an SVG for charts and diagrams",
} do %>
SVGs allow users to magnify images without losing quality.
Find out <%= link_to "how to create an SVG file (opens in new tab)",
"https://guidance.publishing.service.gov.uk/formatting-content/images-videos/formatting-images/#how-to-create-an-svg-file",
class: "govuk-link",
target: "_blank",
rel: "noopener" %>.
<% end %>
<% end %>

Expand Down
2 changes: 1 addition & 1 deletion app/models/image_usage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def lead?
end

def title
return "image" if embeddable?
return "Embeddable image" if embeddable?

label || "#{key} image"
end
Expand Down
6 changes: 0 additions & 6 deletions app/uploaders/image_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ class ImageUploader < WhitehallUploader
config.validate_integrity = true
end

def downloader
# this overloads the downloader from Carrierwave::Uploader::Base
# so that `download!` can be used in development and test environments
WhitehallDownloader
end

def extension_allowlist
model.image_kind_config.allowed_formats
end
Expand Down
5 changes: 0 additions & 5 deletions app/uploaders/whitehall_downloader.rb

This file was deleted.

29 changes: 5 additions & 24 deletions app/views/admin/edition_images/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<%= image_usage.lead? ? render(Admin::EditionImages::LeadImageCardComponent.new(edition: @edition, image:, image_usage:)) : render(Admin::EditionImages::ImageCardComponent.new(edition: @edition, image:, image_usage:)) %>
<% end %>

<% @edition.permitted_image_usages.select { |usage| usage.multiple? }.each_with_index do |usage, multiple_upload_section_index| %>
<% @edition.permitted_image_usages.select { |usage| usage.multiple? }.each do |usage| %>
<% if @edition.editable? %>
<%= render Admin::EditionImages::ImageUploadComponent.new(edition: @edition, failed_images: @failed_images, image_usage: usage) %>
<% end %>
Expand All @@ -50,33 +50,14 @@
} %>
<% end %>

<div id="uploaded_<%= usage.label || usage.key %>_image_list" class="govuk-list">
<% current_images.group_by(&:image_kind).each do |kind, images| %>
<%= render "govuk_publishing_components/components/heading", {
text: Whitehall.image_kinds.fetch(kind).display_name_without_dimensions,
heading_level: 3,
font_size: "m",
padding: true,
} if usage.kinds.size > 1 %>
<ul id="uploaded_<%= usage.label || usage.key %>_<%= kind %>_image_list" class="govuk-list">
<% images.each_with_index do |image, idx| %>
<%= render Admin::EditionImages::ImageComponent.new(edition: @edition, image: image, image_usage: usage) %>
<% unless (idx + 1) == images.size %>
<li aria-hidden="true">
<hr class="app-view-edition-resource__section-break govuk-section-break govuk-section-break--visible">
</li>
<% end %>
<% end %>
</ul>
<ul id="uploaded_embeddable_image_list" class="govuk-list">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you had a document type config with

"usages": {
  "example 1": {
    "multiple": true,
  },
  "example 2": {
    "multiple": true,
  }
}

would it result in two divs ala


<div id="uploaded_embeddable_image_list">
</div>

...

<div id="uploaded_embeddable_image_list">
</div>

I don't think we have any config types writtenlike that currently, but could be an issue in future?

<% current_images.each do |image| %>
<%= render(Admin::EditionImages::ImageCardComponent.new(edition: @edition, image:, image_usage: usage)) %>
<% end %>
</div>
</ul>
<% else %>
<p class="govuk-body">No images uploaded</p>
<% end %>

<% unless multiple_upload_section_index == @edition.permitted_image_usages.select { |usage| usage.multiple? }.count - 1 %>
<hr aria-hidden="true" class="app-view-edition-resource__section-break govuk-section-break govuk-section-break--visible govuk-!-margin-top-6 govuk-!-margin-bottom-6">
<% end %>
<% end %>
</div>
</div>
16 changes: 12 additions & 4 deletions features/step_definitions/image_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def upload_file(width = nil, height = nil, image_usage_key = "govspeak_embed", f
end

Then(/^I should see a list with (\d+) image/) do |count|
expect(page).to have_selector("ul .app-view-edition-resource__preview", count:)
expect(page).to have_selector("#uploaded_embeddable_image_list .gem-c-summary-card", count:)
end

Then(/^I should see a list with (\d+) (.*) image/) do |_count, image_usage_key|
Expand Down Expand Up @@ -113,20 +113,28 @@ def upload_file(width = nil, height = nil, image_usage_key = "govspeak_embed", f
end

When("I click to delete an image") do
first("a", text: "Delete image").click
within "#uploaded_embeddable_image_list" do
first("a", text: "Delete").click
end
end

When("I click to edit the details of an image") do
io_object = fixture_file_upload(jpg_image, "image/jpeg").tempfile.to_io

stub_request(:get, %r{.*/media/.*/*.jpg}).to_return(status: 200, body: io_object, headers: {})
first("a", text: "Edit details").click

within "#uploaded_embeddable_image_list" do
first("a", text: "Edit").click
end
end

When("I click to edit the details of the image that needs to be cropped") do
io_object = fixture_file_upload(Rails.root.join("test/fixtures/images/960x960_jpeg.jpg"), "image/jpeg").tempfile.to_io
stub_request(:get, %r{.*/media/.*/960x960_jpeg.jpg}).to_return(status: 200, body: io_object, headers: {})
find_all("a", text: "Edit details").last.click

within "#uploaded_embeddable_image_list" do
find_all("a", text: "Edit").last.click
end
end

Then("I should I see the processing status tag") do
Expand Down
4 changes: 0 additions & 4 deletions lib/whitehall/image_kinds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ def deconstruct_keys(_keys)
def version_names
versions.map(&:prefixed_name)
end

def display_name_without_dimensions
display_name.split("(").first.strip
end
end

class ImageKinds
Expand Down
46 changes: 43 additions & 3 deletions test/components/admin/edition_images/image_card_component_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ class Admin::EditionImages::ImageCardComponentTest < ViewComponent::TestCase
assert_selector ".govuk-summary-list__row:has(.govuk-summary-list__key:contains(\"Caption\")) .govuk-summary-list__value", text: "Not set"
end

test "renders 'Not set' when image is missing" do
ConfigurableDocumentType.setup_test_types(build_configurable_document_type("test_type"))
edition = build_stubbed(:standard_edition, images: [])

render_inline(Admin::EditionImages::ImageCardComponent.new(edition:, image: nil, image_usage: ImageUsage.new(key: "test_usage", label: "Test usage")))

assert_selector ".govuk-summary-list__row:has(.govuk-summary-list__key:contains(\"Image\")) .govuk-summary-list__value", text: "Not set"
end

test "renders the 'PROCESSING' tag when image has missing assets" do
image_data = build_stubbed(:image_data_with_no_assets, image_kind: "default")
image = build_stubbed(:image, image_data:)
Expand All @@ -66,7 +75,7 @@ class Admin::EditionImages::ImageCardComponentTest < ViewComponent::TestCase

render_inline(Admin::EditionImages::ImageCardComponent.new(edition:, image:, image_usage: ImageUsage.new(key: "test_usage", label: "Test usage")))

assert_selector ".govuk-summary-list__row:has(.govuk-summary-list__key:contains(\"Image\")) .govuk-summary-list__value", text: "Processing"
assert_selector ".govuk-summary-list__row:has(.govuk-summary-list__key:contains(\"Image\")) .govuk-summary-list__value .js-image-processing-status .govuk-tag--green", text: "Processing"
end

test "renders the 'Requires crop' tag when the image needs cropping" do
Expand All @@ -77,7 +86,7 @@ class Admin::EditionImages::ImageCardComponentTest < ViewComponent::TestCase

render_inline(Admin::EditionImages::ImageCardComponent.new(edition:, image:, image_usage: ImageUsage.new(key: "test_usage", label: "Test usage")))

assert_selector ".govuk-summary-list__row:has(.govuk-summary-list__key:contains(\"Image\")) .govuk-summary-list__value", text: "Requires crop"
assert_selector ".govuk-summary-list__row:has(.govuk-summary-list__key:contains(\"Image\")) .govuk-summary-list__value .govuk-tag--red", text: "Requires cropping"
end

test "does not render caption row when caption_enabled is false" do
Expand All @@ -98,6 +107,37 @@ class Admin::EditionImages::ImageCardComponentTest < ViewComponent::TestCase

render_inline(Admin::EditionImages::ImageCardComponent.new(edition:, image:, image_usage: ImageUsage.new(key: "test_usage", label: "Test usage")))

assert_selector "img[src='#{image.thumbnail}']"
assert_selector ".govuk-summary-list__row:has(.govuk-summary-list__key:contains(\"Image\")) .govuk-summary-list__value img.app-view-edition-resource__preview[src='#{image.thumbnail}'][alt='']"
end

test "renders the markdown code for embeddable usages" do
jpeg = upload_fixture("images/960x640_jpeg.jpg")
gif = upload_fixture("images/960x640_gif.gif")
jpeg_image_data = build_stubbed(:image_data, file: jpeg)
gif_image_data = build_stubbed(:image_data, file: gif)
images = [build_stubbed(:image, image_data: jpeg_image_data), build_stubbed(:image, image_data: gif_image_data)]
ConfigurableDocumentType.setup_test_types(build_configurable_document_type("test_type"))
edition = build_stubbed(:standard_edition, images: images)
render_inline(Admin::EditionImages::ImageCardComponent.new(edition:, image: images.first, image_usage: ImageUsage.new(key: "govspeak_embed")))

assert_selector "input[value='[Image: 960x640_jpeg.jpg]']"
end

test "image index markdown used when edition has duplicate image filenames" do
images = [build_stubbed(:image), build_stubbed(:image)]
ConfigurableDocumentType.setup_test_types(build_configurable_document_type("test_type"))
edition = build_stubbed(:standard_edition, images: images)
render_inline(Admin::EditionImages::ImageCardComponent.new(edition:, image: images.first, image_usage: ImageUsage.new(key: "govspeak_embed")))

assert_selector "input[value='!!1']"
end

test "image index markdown handles a lead image being present correctly" do
images = [build_stubbed(:image), build_stubbed(:image), build_stubbed(:image)]
ConfigurableDocumentType.setup_test_types(build_configurable_document_type("test_type"))
edition = build_stubbed(:standard_edition, images: images)
render_inline(Admin::EditionImages::ImageCardComponent.new(edition:, image: images.third, image_usage: ImageUsage.new(key: "govspeak_embed")))

assert_selector "input[value='!!3']"
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,14 @@ class Admin::EditionImages::ImageUploadComponentTest < ViewComponent::TestCase
assert_selector "input[name=\"usage\"][value=\"test_usage\"]", visible: false
end

test "renders hidden image data kind input for single usage" do
test "renders hidden image data kind input" do
edition = build_stubbed(:draft_publication)
usage = ImageUsage.new(key: "test_usage", kinds: [Whitehall.image_kinds.fetch("default")], multiple: false, label: "test")
render_inline(Admin::EditionImages::ImageUploadComponent.new(edition:, image_usage: usage))

assert_selector "input[name=\"image_kind\"][value=\"default\"]", visible: false
end

test "renders radio button inputs for multiple usage" do
edition = build_stubbed(:draft_publication)
image_kinds = [Whitehall.image_kinds.fetch("default"), Whitehall.image_kinds.fetch("topical_event_header")]
usage = ImageUsage.new(key: "test_usage", kinds: image_kinds, multiple: true, label: "test")
render_inline(Admin::EditionImages::ImageUploadComponent.new(edition:, image_usage: usage))

image_kinds.each do |kind|
assert_selector "input[name=\"image_kind\"][value=\"#{kind.name}\"]"
end
end

test "renders inline errors for failed images" do
edition = build_stubbed(:draft_publication)
usage = ImageUsage.new(key: "test_usage", kinds: [Whitehall.image_kinds.fetch("default")], multiple: false, label: "test")
Expand Down
4 changes: 3 additions & 1 deletion test/integration/image_deletion_integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ class ImageDeletionIntegrationTest < ActionDispatch::IntegrationTest
before do
visit admin_edition_path(edition)
click_link "Edit images"
click_link "Delete image"
within "#uploaded_embeddable_image_list" do
click_link "Delete"
end
click_button "Delete image"
assert_text "minister-of-funk.960x640.jpg has been deleted"
visit admin_edition_path(edition)
Expand Down
4 changes: 2 additions & 2 deletions test/unit/app/models/image_usage_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class ImageUsageTest < ActiveSupport::TestCase
assert_equal "header logo", usage.title
end

test "title is `image` if embeddable (default) image usage" do
test "title is `Embeddable image` if embeddable (default) image usage" do
usage = ImageUsage.new(key: "govspeak_embed", kinds: [], multiple: false, caption_enabled: true)
assert_equal "image", usage.title
assert_equal "Embeddable image", usage.title
end
end
14 changes: 0 additions & 14 deletions test/unit/lib/image_kinds_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,6 @@ class ImageKindsTest < ActiveSupport::TestCase
end
end

test "#display_name_without_dimensions returns display name without the dimensions" do
result = Whitehall::ImageKinds.build_image_kinds(
"test_kind" => {
"display_name" => "Test Kind (300x200)",
"valid_width" => 300,
"valid_height" => 200,
"allowed_formats" => %w[jpg jpeg gif png svg],
"versions" => [],
},
)

assert_equal "Test Kind", result["test_kind"].display_name_without_dimensions
end

test "prefixes 'name' and 'from_version' when version_prefix config true" do
result = Whitehall::ImageKinds.build_image_kinds(
"topical_event_logo" => {
Expand Down