From 34a49c8288d9409d5e42ec582cbe141e0a5fa9f6 Mon Sep 17 00:00:00 2001 From: Paul Robert Lloyd Date: Thu, 28 Jul 2022 09:17:12 +0100 Subject: [PATCH] Design review (#789) --- app/views/locations/index.html.erb | 2 + app/views/organisations/index.html.erb | 2 - app/views/organisations/logs.html.erb | 11 +- app/views/organisations/schemes.html.erb | 9 +- app/views/organisations/show.html.erb | 3 +- app/views/organisations/users.html.erb | 11 +- app/views/schemes/check_answers.html.erb | 130 +++++++++--------- app/views/schemes/index.html.erb | 2 - app/views/schemes/show.html.erb | 20 +-- app/views/users/index.html.erb | 4 +- config/locales/en.yml | 2 +- .../requests/organisations_controller_spec.rb | 5 - spec/requests/schemes_controller_spec.rb | 5 - 13 files changed, 99 insertions(+), 107 deletions(-) diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb index 8d7b927b2..04e4c4bf6 100644 --- a/app/views/locations/index.html.erb +++ b/app/views/locations/index.html.erb @@ -13,6 +13,8 @@ <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %> +

Locations

+ <%= render SearchComponent.new(current_user:, search_label: "Search by location name or postcode", value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb index 967c2d0bf..b81922c5e 100644 --- a/app/views/organisations/index.html.erb +++ b/app/views/organisations/index.html.erb @@ -5,8 +5,6 @@ <%= render partial: "organisations/headings", locals: request.path == "/organisations" ? { main: "Organisations", sub: nil } : { main: @organisation.name, sub: "Organisations" } %> -

Organisations

- <% if current_user.support? %> <%= govuk_button_link_to "Create a new organisation", new_organisation_path, html: { method: :get } %> <% end %> diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb index 9d8e978dc..10b686f56 100644 --- a/app/views/organisations/logs.html.erb +++ b/app/views/organisations/logs.html.erb @@ -5,11 +5,12 @@ <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %> -<%= render SubNavigationComponent.new( - items: secondary_items(request.path, @organisation.id), -) %> - -

Logs

+<% if current_user.support? %> + <%= render SubNavigationComponent.new( + items: secondary_items(request.path, @organisation.id), + ) %> +

Logs

+<% end %>
diff --git a/app/views/organisations/schemes.html.erb b/app/views/organisations/schemes.html.erb index 4c59601e6..06d65477a 100644 --- a/app/views/organisations/schemes.html.erb +++ b/app/views/organisations/schemes.html.erb @@ -3,17 +3,22 @@ <% content_for :title, title %> -<%= render partial: "organisations/headings", locals: current_user.support? ? { main: @organisation.name, sub: nil } : { main: "Schemes", sub: current_user.organisation.name } %> +<%= render partial: "organisations/headings", locals: current_user.support? ? { main: @organisation.name, sub: nil } : { main: "Supported housing schemes", sub: current_user.organisation.name } %> <% if current_user.support? %> <%= render SubNavigationComponent.new( items: secondary_items(request.path, @organisation.id), ) %> +

Supported housing schemes

<% end %> <%= govuk_button_link_to "Create a new supported housing scheme", new_scheme_path, html: { method: :post } %> -

Supported housing schemes

+<%= govuk_details( + classes: "govuk-!-width-two-thirds", + summary_text: "What is a supported housing scheme?", + text: "A supported housing scheme (also known as a ‘supported housing service’) provides shared or self-contained housing for a particular client group, for example younger or vulnerable people. A single scheme can contain multiple units, for example bedrooms in shared houses or a bungalow with 3 bedrooms.", +) %> <%= render SearchComponent.new(current_user:, search_label: "Search by scheme name, code or postcode", value: @searched) %> diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 5b82aedc7..3227e7b88 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -8,10 +8,9 @@ <%= render SubNavigationComponent.new( items: secondary_items(request.path, @organisation.id), ) %> +

About this organisation

<% end %> -

About this organisation

-
<%= govuk_summary_list do |summary_list| %> diff --git a/app/views/organisations/users.html.erb b/app/views/organisations/users.html.erb index e8dc383ed..c604a1c9f 100644 --- a/app/views/organisations/users.html.erb +++ b/app/views/organisations/users.html.erb @@ -5,11 +5,12 @@ <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %> -<%= render SubNavigationComponent.new( - items: secondary_items(request.path, @organisation.id), -) %> - -

Users

+<% if current_user.support? %> + <%= render SubNavigationComponent.new( + items: secondary_items(request.path, @organisation.id), + ) %> +

Users

+<% end %> <% if current_user.data_coordinator? || current_user.support? %> <%= govuk_button_link_to "Invite user", new_user_path(organisation_id: @organisation.id), html: { method: :get } %> diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb index 561fca9c9..a9a6033c9 100644 --- a/app/views/schemes/check_answers.html.erb +++ b/app/views/schemes/check_answers.html.erb @@ -2,76 +2,76 @@ <%= render partial: "organisations/headings", locals: { main: "Check your changes before creating this scheme", sub: @scheme.service_name } %> <%= form_for(@scheme, as: :scheme, method: :patch) do |f| %> -
- <%= f.govuk_error_summary %> - <%= govuk_tabs(title: "Check your answers before creating this scheme") do |component| %> - <% component.tab(label: "Scheme") do %> -
- <% @scheme.check_details_attributes.each do |attr| %> - <% next if current_user.data_coordinator? && attr[:name] == ("owned by") %> - <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_details_path(@scheme, check_answers: true) } %> - <% end %> - <% if !@scheme.arrangement_type_same? %> - <% @scheme.check_support_services_provider_attributes.each do |attr| %> - <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_services_provider_path(@scheme, check_answers: true) } %> - <% end %> - <% end %> - <% @scheme.check_primary_client_attributes.each do |attr| %> - <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_primary_client_group_path(@scheme, check_answers: true) } %> - <% end %> - <% @scheme.check_secondary_client_confirmation_attributes.each do |attr| %> - <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_confirm_secondary_client_group_path(@scheme, check_answers: true) } %> - <% end %> - <% if @scheme.has_other_client_group == "Yes" %> - <% @scheme.check_secondary_client_attributes.each do |attr| %> - <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_secondary_client_group_path(@scheme, check_answers: true) } %> - <% end %> - <% end %> - <% @scheme.check_support_attributes.each do |attr| %> - <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_path(@scheme, check_answers: true) } %> - <% end %> -
+ <%= f.govuk_error_summary %> + <%= govuk_tabs(title: "Check your answers before creating this scheme") do |component| %> + <% component.tab(label: "Scheme") do %> +

Scheme

+
+ <% @scheme.check_details_attributes.each do |attr| %> + <% next if current_user.data_coordinator? && attr[:name] == ("owned by") %> + <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_details_path(@scheme, check_answers: true) } %> <% end %> - <% component.tab(label: "Locations") do %> - <%= govuk_table do |table| %> - <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= @scheme.locations.count %> <%= @scheme.locations.count.eql?(1) ? "location" : "locations" %> - <% end %> - <%= table.head do |head| %> - <%= head.row do |row| %> - <% row.cell(header: true, text: "Code", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Postcode", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Units", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Common unit type", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Available from", html_attributes: { - scope: "col", - }) %> - <% end %> - <% end %> - <% @scheme.locations.each do |location| %> - <%= table.body do |body| %> - <%= body.row do |row| %> - <% row.cell(text: location.id) %> - <% row.cell(text: simple_format(location_cell(location, "/schemes/#{@scheme.id}/locations/#{location.id}/edit"), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> - <% row.cell(text: location.units) %> - <% row.cell(text: simple_format("#{location.type_of_unit}")) %> - <% row.cell(text: location.startdate&.to_formatted_s(:govuk_date)) %> - <% end %> + <% if !@scheme.arrangement_type_same? %> + <% @scheme.check_support_services_provider_attributes.each do |attr| %> + <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_services_provider_path(@scheme, check_answers: true) } %> + <% end %> + <% end %> + <% @scheme.check_primary_client_attributes.each do |attr| %> + <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_primary_client_group_path(@scheme, check_answers: true) } %> + <% end %> + <% @scheme.check_secondary_client_confirmation_attributes.each do |attr| %> + <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_confirm_secondary_client_group_path(@scheme, check_answers: true) } %> + <% end %> + <% if @scheme.has_other_client_group == "Yes" %> + <% @scheme.check_secondary_client_attributes.each do |attr| %> + <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_secondary_client_group_path(@scheme, check_answers: true) } %> + <% end %> + <% end %> + <% @scheme.check_support_attributes.each do |attr| %> + <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_support_path(@scheme, check_answers: true) } %> + <% end %> +
+ <% end %> + <% component.tab(label: "Locations") do %> +

Locations

+ <%= govuk_table do |table| %> + <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> + <%= @scheme.locations.count %> <%= @scheme.locations.count.eql?(1) ? "location" : "locations" %> + <% end %> + <%= table.head do |head| %> + <%= head.row do |row| %> + <% row.cell(header: true, text: "Code", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Postcode", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Units", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Common unit type", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Available from", html_attributes: { + scope: "col", + }) %> + <% end %> + <% end %> + <% @scheme.locations.each do |location| %> + <%= table.body do |body| %> + <%= body.row do |row| %> + <% row.cell(text: location.id) %> + <% row.cell(text: simple_format(location_cell(location, "/schemes/#{@scheme.id}/locations/#{location.id}/edit"), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> + <% row.cell(text: location.units) %> + <% row.cell(text: simple_format("#{location.type_of_unit}")) %> + <% row.cell(text: location.startdate&.to_formatted_s(:govuk_date)) %> <% end %> - <% end %> <% end %> - <%= govuk_button_link_to "Add a location", new_location_path(id: @scheme.id), secondary: true %> <% end %> <% end %> -
+ <%= govuk_button_link_to "Add a location", new_location_path(id: @scheme.id), secondary: true %> + <% end %> + <% end %> <%= f.hidden_field :page, value: "check-answers" %> <%= f.hidden_field :confirmed, value: "true" %> <% button_label = @scheme.confirmed? ? "Save" : "Create scheme" %> diff --git a/app/views/schemes/index.html.erb b/app/views/schemes/index.html.erb index 190d29d3d..ca00be067 100644 --- a/app/views/schemes/index.html.erb +++ b/app/views/schemes/index.html.erb @@ -5,8 +5,6 @@ <%= render partial: "organisations/headings", locals: current_user.support? ? { main: "Supported housing schemes", sub: nil } : { main: "Supported housing schemes", sub: current_user.organisation.name } %> -

Supported housing schemes

- <%= govuk_button_link_to "Create a new supported housing scheme", new_scheme_path, html: { method: :post } %> <%= render SearchComponent.new(current_user:, search_label: "Search by scheme name, code or postcode", value: @searched) %> diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb index 2c700fd14..75cb533f7 100644 --- a/app/views/schemes/show.html.erb +++ b/app/views/schemes/show.html.erb @@ -12,15 +12,15 @@ <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %> -
- <%= govuk_summary_list do |summary_list| %> - <% @scheme.display_attributes.each do |attr| %> - <% next if current_user.data_coordinator? && attr[:name] == ("Housing stock owned by") %> - <%= summary_list.row do |row| %> - <% row.key { attr[:name].eql?("Registered under Care Standards Act 2000") ? "Registered under Care Standards Act 2000" : attr[:name].to_s.humanize } %> - <% row.value { details_html(attr) } %> - <% row.action(text: "Change", href: scheme_edit_name_path(scheme_id: @scheme.id)) if attr[:edit] %> - <% end %> +

Scheme

+ +<%= govuk_summary_list do |summary_list| %> + <% @scheme.display_attributes.each do |attr| %> + <% next if current_user.data_coordinator? && attr[:name] == ("Housing stock owned by") %> + <%= summary_list.row do |row| %> + <% row.key { attr[:name].eql?("Registered under Care Standards Act 2000") ? "Registered under Care Standards Act 2000" : attr[:name].to_s.humanize } %> + <% row.value { details_html(attr) } %> + <% row.action(text: "Change", href: scheme_edit_name_path(scheme_id: @scheme.id)) if attr[:edit] %> <% end %> <% end %> -
+<% end %> diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index da05e2905..b9b9975be 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -3,9 +3,7 @@ <% content_for :title, title %> -<%= render partial: "organisations/headings", locals: current_user.support? ? { main: "Users", sub: nil } : { main: "User", sub: current_user.organisation.name } %> - -

Users

+<%= render partial: "organisations/headings", locals: current_user.support? ? { main: "Users", sub: nil } : { main: "Users", sub: current_user.organisation.name } %> <% if current_user.data_coordinator? || current_user.support? %> <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 14af424b8..c139d9c31 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -325,7 +325,7 @@ en: name: "Location name (optional)" units: "Total number of units at this location" type_of_unit: "What is the most common type of unit at this location?" - startdate: "When did the first property in this location become available under this scheme?" + startdate: "When did the first property in this location become available under this scheme? (optional)" add_another_location: "Do you want to add another location?" mobility_type: "What are the mobility standards for the majority of units in this location?" descriptions: diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 357dbb4ba..3d388ee0f 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -116,11 +116,6 @@ RSpec.describe OrganisationsController, type: :request do expect(page).to have_field("search", type: "search") end - it "has hidden accessibility field with description" do - expected_field = "

Supported housing schemes

" - expect(CGI.unescape_html(response.body)).to include(expected_field) - end - it "shows only schemes belonging to the same organisation" do expect(page).to have_content(same_org_scheme.id_to_display) schemes.each do |scheme| diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 8549d79f0..ecf244761 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -93,11 +93,6 @@ RSpec.describe SchemesController, type: :request do expect(CGI.unescape_html(response.body)).to match("#{schemes.count} total schemes.") end - it "has hidden accebility field with description" do - expected_field = "

Supported housing schemes

" - expect(CGI.unescape_html(response.body)).to include(expected_field) - end - context "when params scheme_id is present" do it "shows a success banner" do get "/schemes", params: { scheme_id: schemes.first.id }