From b3039d418633159fb57120ad0bdebd6948aa4b8c Mon Sep 17 00:00:00 2001 From: Mark Taylor <138604938+mtaylorgds@users.noreply.github.com> Date: Wed, 30 Aug 2023 14:14:53 +0100 Subject: [PATCH] Migrate the manuals show view to the design system Update the layout of the "show" view from using the legacy layout to use the design system one. This change significantly alters the layout of the page, but without changing any of the underlying functionality. --- app/assets/javascripts/application.js | 1 + .../stylesheets/_broken-links-report.scss | 48 +--- app/assets/stylesheets/_header.scss | 18 -- .../stylesheets/application-legacy.scss | 1 - app/assets/stylesheets/application.scss | 17 ++ app/assets/stylesheets/views/manuals.scss | 40 +++ app/controllers/manuals_controller.rb | 1 + app/helpers/application_helper.rb | 123 ++++++++ .../admin/link_check_reports/_form.html.erb | 5 +- .../_link_check_report.html.erb | 38 +-- app/views/layouts/design_system.html.erb | 26 ++ app/views/manuals/_title.html.erb | 13 - app/views/manuals/show.html.erb | 210 +++++++------- .../shared/_summary_card_component.html.erb | 41 +++ features/step_definitions/manual_steps.rb | 10 +- features/support/manual_helpers.rb | 10 +- spec/features/manual_urls_spec.rb | 2 +- spec/helpers/application_helper_spec.rb | 268 ++++++++++++++++++ .../link_check_reports/_form.html.erb.spec.rb | 12 + spec/views/manuals/show.html.erb_spec.rb | 21 ++ 20 files changed, 714 insertions(+), 191 deletions(-) create mode 100644 app/assets/stylesheets/views/manuals.scss delete mode 100644 app/views/manuals/_title.html.erb create mode 100644 app/views/shared/_summary_card_component.html.erb create mode 100644 spec/views/admin/link_check_reports/_form.html.erb.spec.rb diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 760ea533f..1275150d2 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -1,6 +1,7 @@ //= require govuk_publishing_components/dependencies //= require govuk_publishing_components/lib //= require govuk_publishing_components/components/button +//= require govuk_publishing_components/components/details //= require govuk_publishing_components/components/layout-header //= require govuk_publishing_components/components/skip-link //= require govuk_publishing_components/components/table diff --git a/app/assets/stylesheets/_broken-links-report.scss b/app/assets/stylesheets/_broken-links-report.scss index 5f9d89127..ed709c006 100644 --- a/app/assets/stylesheets/_broken-links-report.scss +++ b/app/assets/stylesheets/_broken-links-report.scss @@ -1,41 +1,21 @@ -.broken-links-report { - .issue-summary { - font-weight: normal; - } - - .issue-list { - list-style: none; - padding-left: 0; - - li { - margin-top: 5px; - } - } +.app-inset-prompt { + @include govuk-text-colour; + @include govuk-responsive-padding(4); + @include govuk-responsive-margin(6, "bottom"); - .issue-status-description { - margin-top: 10px; - } + border-left: $govuk-border-width-narrow solid govuk-colour("dark-grey"); + background-color: govuk-colour("light-grey"); - .issue-list + .issue-status-description { - margin: 15px 0; + @include govuk-media-query($from: tablet) { + border-left: $govuk-border-width solid govuk-colour("dark-grey"); } - .display-issue-details { - display: block; - font-weight: bold; - padding-top: 5px; - } - - .display-issue-details::-webkit-details-marker { - display: none; - } - - .display-issue-details:before { - content: "\25B6"; - padding-right: 3px; + &:focus { + outline: $govuk-focus-width solid $govuk-focus-colour; } +} - details[open] > .display-issue-details:before { - content: "\25BC"; - } +.app-inset-prompt--error { + border-color: $govuk-error-colour; + background-color: govuk-tint($govuk-error-colour, 90%); } diff --git a/app/assets/stylesheets/_header.scss b/app/assets/stylesheets/_header.scss index b432fdd15..96f5a71b3 100644 --- a/app/assets/stylesheets/_header.scss +++ b/app/assets/stylesheets/_header.scss @@ -1,21 +1,3 @@ .page-header { border-bottom: none; - - .document-slug { - color: $text-muted; - font-size: $font-size-large; - display: block; - margin-top: 5px; - font-weight: normal; - padding: 0; - } - - .document-slug li { - display: inline; - list-style: none; - } - - .document-slug li + li:before { - content: "– "; - } } diff --git a/app/assets/stylesheets/application-legacy.scss b/app/assets/stylesheets/application-legacy.scss index 2bfd9fae3..d40cd81e5 100644 --- a/app/assets/stylesheets/application-legacy.scss +++ b/app/assets/stylesheets/application-legacy.scss @@ -14,7 +14,6 @@ @import "navbar"; @import "header"; @import "ordered_lists"; -@import "broken-links-report"; p.no-content-message { // stylelint-disable-line selector-no-qualifying-type @include core-19; diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 30d221521..d7ec13d2c 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -1,12 +1,29 @@ +$govuk-page-width: 1140px; + @import "govuk_publishing_components/govuk_frontend_support"; @import "govuk_publishing_components/component_support"; @import "govuk_publishing_components/components/breadcrumbs"; @import "govuk_publishing_components/components/button"; +@import "govuk_publishing_components/components/details"; +@import "govuk_publishing_components/components/error-alert"; +@import "govuk_publishing_components/components/error-message"; +@import "govuk_publishing_components/components/hint"; +@import "govuk_publishing_components/components/input"; +@import "govuk_publishing_components/components/label"; @import "govuk_publishing_components/components/layout-footer"; +@import "govuk_publishing_components/components/layout-for-admin"; @import "govuk_publishing_components/components/layout-header"; +@import "govuk_publishing_components/components/notice"; +@import "govuk_publishing_components/components/search"; @import "govuk_publishing_components/components/skip-link"; +@import "govuk_publishing_components/components/success-alert"; +@import "govuk_publishing_components/components/summary-list"; @import "govuk_publishing_components/components/table"; @import "govuk_publishing_components/components/title"; +@import "govuk_publishing_components/components/warning-text"; +@import "views/manuals"; @import "views/whats_new"; + +@import "broken-links-report"; diff --git a/app/assets/stylesheets/views/manuals.scss b/app/assets/stylesheets/views/manuals.scss new file mode 100644 index 000000000..5a078f0b5 --- /dev/null +++ b/app/assets/stylesheets/views/manuals.scss @@ -0,0 +1,40 @@ +.app-view-manuals__section { + padding: govuk-spacing(3) 0; +} + +.app-view-manuals__section:first-child { + padding-top: 0; +} + +.app-view-manuals__sidebar-actions { + padding: govuk-spacing(3); + margin-bottom: govuk-spacing(6); + background-color: govuk-colour("light-grey"); + + .gem-c-button { + width: 100%; + } + + .govuk-list { + margin-bottom: 0; + + li:last-child { + margin-bottom: 0; + } + } +} + +.app-view-summary__sidebar-actions .govuk-list--spaced li:last-child div + .broken-links-report { + margin-top: govuk-spacing(6); +} + +.app-flash-container { + .govuk-notification-banner { + margin-top: govuk-spacing(4); + } + + .gem-c-error-alert { + margin-bottom: 0; + margin-top: govuk-spacing(4); + } +} diff --git a/app/controllers/manuals_controller.rb b/app/controllers/manuals_controller.rb index b5ecefec4..022a536ab 100644 --- a/app/controllers/manuals_controller.rb +++ b/app/controllers/manuals_controller.rb @@ -27,6 +27,7 @@ def show render( :show, + layout: "design_system", locals: { manual:, slug_unique:, diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 52f4a5316..aa287beff 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -15,6 +15,129 @@ def state(manual) tag.span(state, class: classes).html_safe end + def state_label(manual) + state_text = manual.publication_state + + if state_text == "published" && manual.draft? + state_text << " with new draft" + end + + classes = "govuk-tag govuk-tag--s" + classes << if manual.draft? + " govuk-tag--blue" + elsif manual.published? + " govuk-tag--green" + else + " govuk-tag--grey" + end + + tag.span(state_text, class: classes).html_safe + end + + def manual_metadata_rows(manual) + rows = [ + { + key: "Status", + value: state_label(manual), + }, + ] + + if current_user_is_gds_editor? + rows << { + key: "From", + value: link_to(manual.organisation_slug, url_for_public_org(manual.organisation_slug)), + } + end + + if manual.originally_published_at.present? + actions = [ + { + label: "Edit", + href: original_publication_date_manual_path(manual), + }, + ] + row = { + key: "Originally published", + value: nice_time_format(manual.originally_published_at), + } + row[:actions] = actions + rows << row + end + + if manual.publish_tasks.any? + rows << { + key: "Last published", + value: publication_task_state(manual.publish_tasks.first), + } + end + rows + end + + def manual_front_page_rows(manual) + rows = [ + { + key: "Slug", + value: manual.slug, + }, + { + key: "Title", + value: sanitize(manual.title), + }, + { + key: "Summary", + value: sanitize(manual.summary), + }, + ] + + if manual.body.present? + rows << { + key: "Body", + value: simple_format(truncate(manual.body, length: 500, class: "govuk-!-margin-top-0")), + } + end + + rows + end + + def manual_sidebar_action_items(manual, slug_unique) + items = [] + + if allow_publish?(manual, slug_unique) + items << render("govuk_publishing_components/components/button", { + text: "Publish", + href: confirm_publish_manual_path(manual), + }) + end + + unless manual.has_ever_been_published? + items << render("govuk_publishing_components/components/button", { + text: "Discard", + destructive: true, + }) + end + + items + end + + def manual_section_rows(manual) + manual.sections.map do |section| + row = {} + + row[:key] = if section.state == "draft" + tag.span("DRAFT", class: "govuk-tag govuk-tag--s govuk-tag--blue") << + tag.span(section.title, class: "govuk-!-static-margin-2") + else + tag.span(section.title) + end + row[:value] = last_updated_text(section) + row[:actions] = [{ + label: "View", + href: manual_section_path(manual, section), + }] + row + end + end + def show_preview?(item) if item.respond_to?(:sections) item.draft? || item.sections.any?(&:draft?) diff --git a/app/views/admin/link_check_reports/_form.html.erb b/app/views/admin/link_check_reports/_form.html.erb index 557c47571..3f2a41f78 100644 --- a/app/views/admin/link_check_reports/_form.html.erb +++ b/app/views/admin/link_check_reports/_form.html.erb @@ -1,3 +1,6 @@ <%= form_tag link_check_reports_path(link_reportable: reportable), remote: true do %> - <%= submit_tag button_text, class: "btn btn-default add-top-margin remove-bottom-margin" %> + <%= render "govuk_publishing_components/components/button", { + text: button_text, + secondary_quiet: true + } %> <% end %> diff --git a/app/views/admin/link_check_reports/_link_check_report.html.erb b/app/views/admin/link_check_reports/_link_check_report.html.erb index 095a37cb6..c287ee7e1 100644 --- a/app/views/admin/link_check_reports/_link_check_report.html.erb +++ b/app/views/admin/link_check_reports/_link_check_report.html.erb @@ -1,34 +1,36 @@
Check this document for broken links. The report will take a few moments to complete.
+Check this document for broken links. The report will take a few moments to complete.
<%= render 'admin/link_check_reports/form', reportable: reportable, button_text: 'Check for broken links' %>Broken link report in progress.
+Please wait. <%= link_to "Refresh", link_check_report_path(report.id), class: 'js-broken-links-refresh js-hidden', - remote: true %> + remote: true %>
<%= t "broken_links.#{status}.subheading" %>
-<%= t "broken_links.#{status}.subheading" %>
+<%= link.problem_summary %>
+ <%= link_to link.uri.truncate(50), link.uri, title: link.uri, class: 'govuk-link' %> + <%= render "govuk_publishing_components/components/details", { + title: "See more details about this link" + } do %> +<%= link.problem_summary %>
<% if link.suggested_fix %> -Suggested fix: <%= link.suggested_fix %>
- <% end %> -Suggested fix: <%= link.suggested_fix %>
+ <% end %> + <% end %>This document contains no broken links.
+This document contains no broken links.
<%= render 'admin/link_check_reports/form', reportable: reportable, button_text: 'Check again' %>Warning: There are duplicate section slugs in this manual.
-<%= manual.summary %>
-<%= manual.body %>-
+ <%= link_to "Preview on website (opens in new tab)", content_preview_url(manual), target: "_blank" %>
+This is a new draft of a document that has already been published.
++ <%= link_to "Go to published edition", url_for_public_manual(manual), target: "_blank", class: "govuk-link" %>
+a manual body
" }, + ) + end + + it "does not return a row for the body when it is empty" do + manual = FactoryBot.build_stubbed(:manual, body: "") + + rows = manual_front_page_rows(manual) + + expect(rows).to_not include(include(key: "Body")) + end + end + + describe "#manual_sidebar_action_items" do + before do + allow_any_instance_of(ApplicationHelper).to receive(:allow_publish?).and_return(true) + end + + it "returns a 'publish' button when the manual is allowed to be published" do + manual = FactoryBot.build_stubbed(:manual) + confirm_publish_path = "/manual/blah/confirm_publish" + allow_any_instance_of(ApplicationHelper).to receive(:allow_publish?).and_return(true) + allow_any_instance_of(ApplicationHelper).to receive(:confirm_publish_manual_path).and_return(confirm_publish_path) + + items = manual_sidebar_action_items(manual, true) + + expect(items).to include(include("Publish")) + expect(items).to include(include(confirm_publish_path)) + end + + it "does not return a 'publish' button when the manual is not allowed to be published" do + manual = FactoryBot.build_stubbed(:manual) + allow_any_instance_of(ApplicationHelper).to receive(:allow_publish?).and_return(false) + + items = manual_sidebar_action_items(manual, true) + + expect(items).not_to include(include("Publish")) + end + + it "returns a 'discard' button when the manual has never been published" do + manual = FactoryBot.build_stubbed(:manual, ever_been_published: false) + + items = manual_sidebar_action_items(manual, true) + + expect(items).to include(include("Discard")) + end + + it "does not return a 'discard' button when the manual has been published" do + manual = FactoryBot.build_stubbed(:manual, ever_been_published: true) + + items = manual_sidebar_action_items(manual, true) + + expect(items).not_to include(include("Discard")) + end + end + + describe "#manual_section_rows" do + before do + allow_any_instance_of(ApplicationHelper).to receive(:last_updated_text).and_return("") + end + + it "returns a row for each section in the manual" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({}) + manual.build_section({}) + + rows = manual_section_rows(manual) + + expect(rows.length).to be(2) + end + + it "adds a 'DRAFT' tag to the key when the section is in the 'draft' state" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({ state: "draft", title: "test title" }) + + rows = manual_section_rows(manual) + + expect(rows).to include(include(key: include(">DRAFT"))) + expect(rows).to include(include(key: include(">test title"))) + end + + it "does not add a 'DRAFT' tag to the key when the section is in the 'published' state" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({ state: "published", title: "test title" }) + + rows = manual_section_rows(manual) + + expect(rows).not_to include(include(key: include(">DRAFT"))) + expect(rows).to include(include(key: include(">test title"))) + end + + it "uses the section's title for the key when the section is in the 'published state" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({ state: "published", title: "test title" }) + + rows = manual_section_rows(manual) + + expect(rows).to include(include(key: "test title")) + end + end end diff --git a/spec/views/admin/link_check_reports/_form.html.erb.spec.rb b/spec/views/admin/link_check_reports/_form.html.erb.spec.rb new file mode 100644 index 000000000..c6c584147 --- /dev/null +++ b/spec/views/admin/link_check_reports/_form.html.erb.spec.rb @@ -0,0 +1,12 @@ +require "spec_helper" + +describe "admin/link_check_reports/_form", type: :view do + it "renders a button with the specified text" do + allow(view).to receive(:reportable).and_return({}) + allow(view).to receive(:button_text).and_return("Do the thing") + + render + + expect(rendered).to have_css("button", exact_text: "Do the thing") + end +end diff --git a/spec/views/manuals/show.html.erb_spec.rb b/spec/views/manuals/show.html.erb_spec.rb index c35a8c597..9923e41c5 100644 --- a/spec/views/manuals/show.html.erb_spec.rb +++ b/spec/views/manuals/show.html.erb_spec.rb @@ -23,4 +23,25 @@ expect(rendered).to match(/Discard draft/) end + + [ + [:notice, ".govuk-notification-banner .govuk-notification-banner__content"], + [:success, ".govuk-notification-banner .govuk-notification-banner__content"], + [:error, ".gem-c-error-alert .gem-c-error-alert__message"], + ].each do |flash_type, msg_css_selector| + it "renders a #{flash_type} flash message" do + manual = FactoryBot.build_stubbed(:manual) + manual.publish_tasks = [] + msg = "A flash message" + flash[flash_type] = msg + allow(view).to receive(:flash).and_return(flash) + allow(view).to receive(:current_user).and_return(FactoryBot.build_stubbed(:user)) + + render template: "manuals/show", + layout: "layouts/design_system", + locals: { manual:, slug_unique: true, clashing_sections: [] } + + expect(rendered).to have_css(msg_css_selector, text: msg) + end + end end