diff --git a/app/components/navigation_component.html.erb b/app/components/navigation_component.html.erb deleted file mode 100644 index 3d192ef2b..000000000 --- a/app/components/navigation_component.html.erb +++ /dev/null @@ -1,17 +0,0 @@ - diff --git a/app/components/navigation_component.rb b/app/components/navigation_component.rb deleted file mode 100644 index e386e429f..000000000 --- a/app/components/navigation_component.rb +++ /dev/null @@ -1,13 +0,0 @@ -class NavigationComponent < ViewComponent::Base - attr_reader :items, :level - - def initialize(items:, level: "primary") - @items = items - @level = level - super - end - - def highlighted_tab?(item, _path) - item[:current] - end -end diff --git a/app/components/primary_navigation_component.html.erb b/app/components/primary_navigation_component.html.erb new file mode 100644 index 000000000..9cad3bc64 --- /dev/null +++ b/app/components/primary_navigation_component.html.erb @@ -0,0 +1,17 @@ + diff --git a/app/components/primary_navigation_component.rb b/app/components/primary_navigation_component.rb new file mode 100644 index 000000000..0389d30b3 --- /dev/null +++ b/app/components/primary_navigation_component.rb @@ -0,0 +1,12 @@ +class PrimaryNavigationComponent < ViewComponent::Base + attr_reader :items + + def initialize(items:) + @items = items + super + end + + def highlighted_item?(item, _path) + item[:current] + end +end diff --git a/app/components/sub_navigation_component.html.erb b/app/components/sub_navigation_component.html.erb new file mode 100644 index 000000000..b9790d917 --- /dev/null +++ b/app/components/sub_navigation_component.html.erb @@ -0,0 +1,15 @@ + diff --git a/app/components/sub_navigation_component.rb b/app/components/sub_navigation_component.rb new file mode 100644 index 000000000..e4dd0654a --- /dev/null +++ b/app/components/sub_navigation_component.rb @@ -0,0 +1,12 @@ +class SubNavigationComponent < ViewComponent::Base + attr_reader :items + + def initialize(items:) + @items = items + super + end + + def highlighted_item?(item, _path) + item[:current] + end +end diff --git a/app/frontend/styles/_primary-navigation.scss b/app/frontend/styles/_primary-navigation.scss index 7409962ea..e6597ea2b 100644 --- a/app/frontend/styles/_primary-navigation.scss +++ b/app/frontend/styles/_primary-navigation.scss @@ -1,20 +1,14 @@ -.app-primary-navigation, -.app-sub-navigation { - @include govuk-font(19, $weight: bold); - border-bottom: 1px solid $govuk-border-colour; -} - .app-primary-navigation { + @include govuk-font(19, $weight: bold); background-color: govuk-colour("light-grey"); + border-bottom: 1px solid $govuk-border-colour; } -.govuk-phase-banner + .app-primary-navigation, -.app-sub-navigation { +.govuk-phase-banner + .app-primary-navigation { margin-top: -1px; } -.app-primary-navigation__list, -.app-sub-navigation__list { +.app-primary-navigation__list { @include govuk-clearfix; left: govuk-spacing(-3); list-style: none; @@ -25,8 +19,7 @@ width: calc(100% + #{govuk-spacing(6)}); } -.app-primary-navigation__item, -.app-sub-navigation__item { +.app-primary-navigation__item { box-sizing: border-box; display: block; float: left; @@ -36,8 +29,7 @@ position: relative; } -.app-primary-navigation__item--current, -.app-sub-navigation__item--current { +.app-primary-navigation__item--current { border-bottom: $govuk-border-width-narrow solid $govuk-link-colour; &:hover { @@ -49,15 +41,13 @@ } } -.app-primary-navigation__item--align-right, -.app-sub-navigation__item--align-right { +.app-primary-navigation__item--align-right { @include govuk-media-query($from: tablet) { float: right; } } -.app-primary-navigation__link, -.app-sub-navigation__link { +.app-primary-navigation__link { @include govuk-link-common; @include govuk-link-style-no-visited-state; @include govuk-link-style-no-underline; @@ -74,7 +64,6 @@ } } -.app-primary-navigation__item--current .app-primary-navigation__link:hover, -.app-sub-navigation__item--current .app-sub-navigation__link:hover { +.app-primary-navigation__item--current .app-primary-navigation__link:hover { text-decoration: none; } diff --git a/app/frontend/styles/_sub-navigation.scss b/app/frontend/styles/_sub-navigation.scss new file mode 100644 index 000000000..7940e4814 --- /dev/null +++ b/app/frontend/styles/_sub-navigation.scss @@ -0,0 +1,81 @@ +.app-sub-navigation { + @include govuk-font(19, $weight: bold); + @include govuk-responsive-margin(6, "bottom"); +} + +.app-sub-navigation__list { + @include govuk-clearfix; + left: govuk-spacing(-3); + list-style: none; + margin: 0; + padding: 0; + position: relative; + right: govuk-spacing(-3); + width: calc(100% + #{govuk-spacing(6)}); + + @include govuk-media-query($from: tablet) { + box-shadow: inset 0 -1px 0 $govuk-border-colour; + } +} + +.app-sub-navigation__item { + box-sizing: border-box; + display: block; + line-height: 40px; + height: 40px; + padding: 0 govuk-spacing(3); + + @include govuk-media-query($from: tablet) { + box-shadow: none; + display: block; + float: left; + line-height: 50px; + height: 50px; + padding: 0 govuk-spacing(3); + position: relative; + } +} + +.app-sub-navigation__item--current { + @include govuk-media-query($until: tablet) { + border-left: $govuk-border-width-narrow solid $govuk-link-colour; + padding-left: 11px; + } + + @include govuk-media-query($from: tablet) { + border-bottom: $govuk-border-width-narrow solid $govuk-link-colour; + padding-left: govuk-spacing(3); + } + + &:hover { + border-color: $govuk-link-hover-colour; + } + + &:active { + border-color: $govuk-link-active-colour; + } +} + +.app-sub-navigation__link { + @include govuk-link-common; + @include govuk-link-style-no-visited-state; + @include govuk-link-style-no-underline; + @include govuk-typography-weight-bold; + position: relative; + + // Extend the touch area of the link to the list + &:after { + bottom: 0; + content: ""; + left: 0; + position: absolute; + right: 0; + top: 0; + } +} + +.app-sub-navigation__item--current .app-sub-navigation__link { + &:hover { + text-decoration: none; + } +} diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss index 883ea5e63..487f2d893 100644 --- a/app/frontend/styles/application.scss +++ b/app/frontend/styles/application.scss @@ -40,6 +40,7 @@ $govuk-breakpoints: ( @import "panel"; @import "primary-navigation"; @import "search"; +@import "sub-navigation"; // App utilities .app-\!-colour-muted { diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb index 376424c7e..bc3e10f86 100644 --- a/app/helpers/tab_nav_helper.rb +++ b/app/helpers/tab_nav_helper.rb @@ -3,11 +3,11 @@ module TabNavHelper def user_cell(user) link_text = user.name.presence || user.email - [govuk_link_to(link_text, user, class: "govuk-!-font-weight-bold"), user.email].join("\n") + [govuk_link_to(link_text, user), "#{user.email}"].join("\n") end def org_cell(user) - role = "#{user.role.to_s.humanize}" + role = "#{user.role.to_s.humanize}" [user.organisation.name, role].join("\n") end diff --git a/app/views/case_logs/_log_list.html.erb b/app/views/case_logs/_log_list.html.erb index bf027896c..87c5c1ed7 100644 --- a/app/views/case_logs/_log_list.html.erb +++ b/app/views/case_logs/_log_list.html.erb @@ -1,58 +1,70 @@
- - - - - - - - - - + <% end %> + <%= table.head do |head| %> + <%= head.row do |row| %> + <% row.cell(header: true, text: "Log", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Tenant", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Property", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Tenancy starts", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Log created", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Log Status", html_attributes: { + scope: "col", + }) %> <% if current_user.support? %> - - + <% row.cell(header: true, text: "Owning organisation", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Managing organisation", html_attributes: { + scope: "col", + }) %> <% end %> - - - + <% end %> + <% end %> + <%= table.body do |body| %> <% case_logs.map do |log| %> - - - - - - - - <% if current_user.support? %> - - <% end %> - <% end %> - -
+ <%= govuk_table do |table| %> + <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular], id: title.dasherize) do |caption| %> <%= pagy.count %> total <%= title.downcase %> <%= govuk_link_to "Download (CSV)", "/logs.csv", type: "text/csv" %> -
LogTenantPropertyTenancy startsLog createdStatusOwning organisationManaging organisation
- <%= govuk_link_to case_log_path(log) do %> - Log <%= log.id %> + <%= body.row do |row| %> + <% row.cell(header: true, html_attributes: { + scope: "row", + }) do %> + <%= govuk_link_to case_log_path(log) do %> + Log <%= log.id %> + <% end %> + <% end %> + <% row.cell( + text: log.tenant_code? ? log.tenant_code : "–", + classes: "app-!-font-tabular", + ) %> + <% row.cell( + text: log.propcode? ? log.propcode : "–", + classes: "app-!-font-tabular", + ) %> + <% row.cell(text: log.startdate.present? ? log.startdate.to_formatted_s(:govuk_date) : "–") %> + <% row.cell(text: log.created_at.to_formatted_s(:govuk_date)) %> + <% row.cell do %> + <%= status_tag(log.status) %> + <% end %> + <% if current_user.support? %> + <% row.cell(text: log.owning_organisation.name) %> + <% row.cell(text: log.managing_organisation.name) %> <% end %> - - <%= log.tenant_code? ? log.tenant_code : "–" %> - - <%= log.propcode? ? log.propcode : "–" %> - - <%= log.startdate.present? ? log.startdate.to_formatted_s(:govuk_date) : "–" %> - - <%= log.created_at.to_formatted_s(:govuk_date) %> - - <%= status_tag(log.status) %> - - <%= log.owning_organisation.name %> - - <%= log.managing_organisation.name %> -
+ <% end %> + <% end %>
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index d1a2c0dd6..7a3e85d89 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -65,9 +65,8 @@ ) %> <% if !current_user.nil? %> - <%= render NavigationComponent.new( + <%= render PrimaryNavigationComponent.new( items: primary_items(request.path, current_user), - level: "primary", ) %> <% end %> diff --git a/app/views/organisations/_organisation_list.html.erb b/app/views/organisations/_organisation_list.html.erb index 8e6e443b1..a027718fa 100644 --- a/app/views/organisations/_organisation_list.html.erb +++ b/app/views/organisations/_organisation_list.html.erb @@ -1,23 +1,29 @@ <% content_for :title, title %> <%= govuk_table do |table| %> - <%= table.caption(size: "s", classes: %w[govuk-!-text-align-left govuk-!-margin-top-4 govuk-!-margin-bottom-4]) do |caption| %> - - <%= @pagy.count %> total <%= title.downcase %> - + <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> + <%= @pagy.count %> total <%= title.downcase %>. <% end %> <%= table.head do |head| %> <%= head.row do |row| %> - <% row.cell(header: true, text: "Name") %> - <% row.cell(header: true, text: "Registration number") %> - <% row.cell(header: true, text: "Type") %> + <% row.cell(header: true, text: "Name", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Registration number", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Type", html_attributes: { + scope: "col", + }) %> <% end %> <% end %> <% @organisations.each do |organisation| %> <%= table.body do |body| %> <%= body.row do |row| %> - <% row.cell do %> - <%= govuk_link_to(organisation.name, "organisations/#{organisation.id}/logs", class: "govuk-!-font-weight-bold") %> + <% row.cell(header: true, html_attributes: { + scope: "row", + }) do %> + <%= govuk_link_to(organisation.name, "organisations/#{organisation.id}/logs") %> <% end %> <% row.cell(text: organisation.housing_registration_no) %> <% row.cell(text: display_provider_type(organisation.provider_type)) %> diff --git a/app/views/organisations/logs.erb b/app/views/organisations/logs.html.erb similarity index 77% rename from app/views/organisations/logs.erb rename to app/views/organisations/logs.html.erb index cb72cd15d..a12287530 100644 --- a/app/views/organisations/logs.erb +++ b/app/views/organisations/logs.html.erb @@ -4,14 +4,12 @@ <%= @organisation.name %> <%= content_for(:title) %> -
- <%= render NavigationComponent.new( - items: secondary_items(request.path, @organisation.id), - level: "sub" - ) %> -
-
+<%= render SubNavigationComponent.new( + items: secondary_items(request.path, @organisation.id), +) %> + +
<%= render partial: "case_logs/log_filters" %> <% if @case_logs.present? %>
diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index dae556d97..ecd36da73 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -4,16 +4,13 @@ <%= "Details" %> <% end %> -
- <% if current_user.support? %> -
- <%= render NavigationComponent.new( - items: secondary_items(request.path, @organisation.id), - level: "sub", - ) %> -
+ <%= render SubNavigationComponent.new( + items: secondary_items(request.path, @organisation.id), + ) %> <% end %> + +
<%= govuk_summary_list do |summary_list| %> <% @organisation.display_attributes.each do |attr| %> diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 44f401515..e9bd62065 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -11,12 +11,12 @@ <%= render SearchComponent.new(current_user:, label: "Search by name or email address") %> <%= govuk_table do |table| %> - <%= table.caption(size: "s", classes: %w[govuk-!-text-align-left govuk-!-margin-top-4 govuk-!-margin-bottom-4]) do |caption| %> + <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> <% if @searched %> - Matches <%= @pagy.count %> of <%= User.count %> total users + Matches <%= @pagy.count %> of <%= User.count %> total users. <% else %> - <%= @pagy.count %> total users + <%= @pagy.count %> total users. <% end %> <% if current_user.support? %> @@ -25,15 +25,23 @@ <% end %> <%= table.head do |head| %> <%= head.row do |row| %> - <% row.cell(header: true, text: "Name and email adress") %> - <% row.cell(header: true, text: "Organisation and role") %> - <% row.cell(header: true, text: "Last logged in") %> + <% row.cell(header: true, text: "Name and email adress", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Organisation and role", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Last logged in", html_attributes: { + scope: "col", + }) %> <% end %> <% end %> <% @users.each do |user| %> <%= table.body do |body| %> <%= body.row do |row| %> - <% row.cell do %> + <% row.cell(header: true, html_attributes: { + scope: "row", + }) do %> <%= simple_format(user_cell(user), {}, wrapper_tag: "span") %> <% if user.is_data_protection_officer? || user.is_key_contact? %>
diff --git a/spec/components/primary_navigation_component_spec.rb b/spec/components/primary_navigation_component_spec.rb index e37f8320b..0de1c58d1 100644 --- a/spec/components/primary_navigation_component_spec.rb +++ b/spec/components/primary_navigation_component_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe NavigationComponent, type: :component do +RSpec.describe PrimaryNavigationComponent, type: :component do let(:items) do [ NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), @@ -9,8 +9,8 @@ RSpec.describe NavigationComponent, type: :component do ] end - context "when the item is 'current' in nav tabs" do - it "then that tab appears as selected" do + context "when the item is 'current' in nav items" do + it "then that item appears as selected" do result = render_inline(described_class.new(items:)) expect(result.css('.app-primary-navigation__link[aria-current="page"]').text).to include("Organisations") @@ -18,17 +18,17 @@ RSpec.describe NavigationComponent, type: :component do end context "when the current page is sub-page" do - it "highlights the correct tab" do + it "highlights the correct item" do navigation_panel = described_class.new(items:) - expect(navigation_panel).to be_highlighted_tab(items[0], "/something-else") - expect(navigation_panel).not_to be_highlighted_tab(items[1], "/something-else") - expect(navigation_panel).not_to be_highlighted_tab(items[2], "/something-else") + expect(navigation_panel).to be_highlighted_item(items[0], "/something-else") + expect(navigation_panel).not_to be_highlighted_item(items[1], "/something-else") + expect(navigation_panel).not_to be_highlighted_item(items[2], "/something-else") end end - context "when rendering tabs" do - it "all of the nav tabs specified in the items hash are passed to it" do + context "when rendering items" do + it "all of the nav items specified in the items hash are passed to it" do result = render_inline(described_class.new(items:)) expect(result.text).to include("Organisations") diff --git a/spec/components/sub_navigation_component_spec.rb b/spec/components/sub_navigation_component_spec.rb new file mode 100644 index 000000000..fcfa2d75d --- /dev/null +++ b/spec/components/sub_navigation_component_spec.rb @@ -0,0 +1,39 @@ +require "rails_helper" + +RSpec.describe SubNavigationComponent, type: :component do + let(:items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Logs ", "/logs", false), + ] + end + + context "when the item is 'current' in nav items" do + it "then that tab appears as selected" do + result = render_inline(described_class.new(items:)) + + expect(result.css('.app-sub-navigation__link[aria-current="page"]').text).to include("Organisations") + end + end + + context "when the current page is sub-page" do + it "highlights the correct tab" do + navigation_panel = described_class.new(items:) + + expect(navigation_panel).to be_highlighted_item(items[0], "/something-else") + expect(navigation_panel).not_to be_highlighted_item(items[1], "/something-else") + expect(navigation_panel).not_to be_highlighted_item(items[2], "/something-else") + end + end + + context "when rendering items" do + it "all of the nav items specified in the items hash are passed to it" do + result = render_inline(described_class.new(items:)) + + expect(result.text).to include("Organisations") + expect(result.text).to include("Users") + expect(result.text).to include("Logs") + end + end +end diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb index f8931de8e..4930d33b5 100644 --- a/spec/helpers/tab_nav_helper_spec.rb +++ b/spec/helpers/tab_nav_helper_spec.rb @@ -6,14 +6,14 @@ RSpec.describe TabNavHelper do describe "#user_cell" do it "returns user link and email separated by a newline character" do - expected_html = "Danny Rojas\n#{user.email}" + expected_html = "Danny Rojas\n#{user.email}" expect(user_cell(user)).to match(expected_html) end end describe "#org_cell" do it "returns the users org name and role separated by a newline character" do - expected_html = "DLUHC\nData provider" + expected_html = "DLUHC\nData provider" expect(org_cell(user)).to match(expected_html) end end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 342c6f36c..94f928a6b 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -460,7 +460,7 @@ RSpec.describe OrganisationsController, type: :request do end it "shows the total organisations count" do - expect(CGI.unescape_html(response.body)).to match("#{total_organisations_count} total organisations") + expect(CGI.unescape_html(response.body)).to match("#{total_organisations_count} total organisations.") end it "has pagination links" do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index beb81a993..10821961c 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -381,7 +381,7 @@ RSpec.describe UsersController, type: :request do end it "updates the table caption" do - expect(page).to have_content("Matches 1 of 5 total users") + expect(page).to have_content("Matches 1 of 5 total users.") end end @@ -417,7 +417,7 @@ RSpec.describe UsersController, type: :request do end it "updates the table caption" do - expect(page).to have_content("Matches 2 of 5 total users") + expect(page).to have_content("Matches 2 of 5 total users.") end end end @@ -842,7 +842,7 @@ RSpec.describe UsersController, type: :request do end it "updates the table caption" do - expect(page).to have_content("Matches 1 of 4 total users") + expect(page).to have_content("Matches 1 of 4 total users.") end end @@ -881,7 +881,7 @@ RSpec.describe UsersController, type: :request do end it "updates the table caption" do - expect(page).to have_content("Matches 2 of 4 total users") + expect(page).to have_content("Matches 2 of 4 total users.") end end end