Browse Source

Merge pull request #606 from communitiesuk/frontend-tweaks

Table and navigation tweaks
pull/609/head
Paul Robert Lloyd 3 years ago committed by GitHub
parent
commit
42b8da7f87
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 17
      app/components/navigation_component.html.erb
  2. 13
      app/components/navigation_component.rb
  3. 17
      app/components/primary_navigation_component.html.erb
  4. 12
      app/components/primary_navigation_component.rb
  5. 15
      app/components/sub_navigation_component.html.erb
  6. 12
      app/components/sub_navigation_component.rb
  7. 29
      app/frontend/styles/_primary-navigation.scss
  8. 81
      app/frontend/styles/_sub-navigation.scss
  9. 1
      app/frontend/styles/application.scss
  10. 4
      app/helpers/tab_nav_helper.rb
  11. 104
      app/views/case_logs/_log_list.html.erb
  12. 3
      app/views/layouts/application.html.erb
  13. 24
      app/views/organisations/_organisation_list.html.erb
  14. 12
      app/views/organisations/logs.html.erb
  15. 13
      app/views/organisations/show.html.erb
  16. 22
      app/views/users/index.html.erb
  17. 18
      spec/components/primary_navigation_component_spec.rb
  18. 39
      spec/components/sub_navigation_component_spec.rb
  19. 4
      spec/helpers/tab_nav_helper_spec.rb
  20. 2
      spec/requests/organisations_controller_spec.rb
  21. 8
      spec/requests/users_controller_spec.rb

17
app/components/navigation_component.html.erb

@ -1,17 +0,0 @@
<nav class="app-<%= level %>-navigation" aria-label="<%= level %>">
<div class="govuk-width-container">
<ul class="app-<%= level %>-navigation__list">
<% items.each do |item| %>
<% if item.current %>
<li class="app-<%= level %>-navigation__item app-<%= level %>-navigation__item--current">
<%= govuk_link_to item[:text], item[:href], class: "app-#{level}-navigation__link", aria: { current: "page" } %>
</li>
<% else %>
<li class="app-<%= level %>-navigation__item">
<%= govuk_link_to item[:text], item[:href], class: "app-#{level}-navigation__link" %>
</li>
<% end %>
<% end %>
</ul>
</div>
</nav>

13
app/components/navigation_component.rb

@ -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

17
app/components/primary_navigation_component.html.erb

@ -0,0 +1,17 @@
<nav class="app-primary-navigation" aria-label="primary">
<div class="govuk-width-container">
<ul class="app-primary-navigation__list">
<% items.each do |item| %>
<% if item.current %>
<li class="app-primary-navigation__item app-primary-navigation__item--current">
<%= govuk_link_to item[:text], item[:href], class: "app-primary-navigation__link", aria: { current: "page" } %>
</li>
<% else %>
<li class="app-primary-navigation__item">
<%= govuk_link_to item[:text], item[:href], class: "app-primary-navigation__link" %>
</li>
<% end %>
<% end %>
</ul>
</div>
</nav>

12
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

15
app/components/sub_navigation_component.html.erb

@ -0,0 +1,15 @@
<nav class="app-sub-navigation" aria-label="secondary">
<ul class="app-sub-navigation__list">
<% items.each do |item| %>
<% if item.current %>
<li class="app-sub-navigation__item app-sub-navigation__item--current">
<%= govuk_link_to item[:text], item[:href], class: "app-sub-navigation__link", aria: { current: "page" } %>
</li>
<% else %>
<li class="app-sub-navigation__item">
<%= govuk_link_to item[:text], item[:href], class: "app-sub-navigation__link" %>
</li>
<% end %>
<% end %>
</ul>
</nav>

12
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

29
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;
}

81
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;
}
}

1
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 {

4
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), "<span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{user.email}</span>"].join("\n")
end
def org_cell(user)
role = "<span class='app-!-colour-muted'>#{user.role.to_s.humanize}</span>"
role = "<span class=\"app-!-colour-muted\">#{user.role.to_s.humanize}</span>"
[user.organisation.name, role].join("\n")
end

104
app/views/case_logs/_log_list.html.erb

@ -1,58 +1,70 @@
<section class="app-table-group" tabindex="0" aria-labelledby="<%= title.dasherize %>">
<table class="govuk-table">
<caption id="<%= title.dasherize %>" class="govuk-!-text-align-left govuk-!-margin-top-4 govuk-!-margin-bottom-4">
<%= govuk_table do |table| %>
<%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular], id: title.dasherize) do |caption| %>
<span class="govuk-!-margin-right-4">
<strong><%= pagy.count %></strong> total <%= title.downcase %>
</span>
<%= govuk_link_to "Download (CSV)", "/logs.csv", type: "text/csv" %>
</caption>
<thead class="govuk-table__head">
<tr class="govuk-table__row">
<th class="govuk-table__header" scope="col">Log</th>
<th class="govuk-table__header" scope="col">Tenant</th>
<th class="govuk-table__header" scope="col">Property</th>
<th class="govuk-table__header" scope="col">Tenancy starts</th>
<th class="govuk-table__header" scope="col">Log created</th>
<th class="govuk-table__header" scope="col">Status</th>
<% 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? %>
<th class="govuk-table__header" scope="col">Owning organisation</th>
<th class="govuk-table__header" scope="col">Managing organisation</th>
<% row.cell(header: true, text: "Owning organisation", html_attributes: {
scope: "col",
}) %>
<% row.cell(header: true, text: "Managing organisation", html_attributes: {
scope: "col",
}) %>
<% end %>
</tr>
</thead>
<tbody class="govuk-table__body">
<% end %>
<% end %>
<%= table.body do |body| %>
<% case_logs.map do |log| %>
<tr class="govuk-table__row">
<th class="govuk-table__header" scope="row">
<%= govuk_link_to case_log_path(log) do %>
<span class="govuk-visually-hidden">Log </span><%= log.id %>
<%= body.row do |row| %>
<% row.cell(header: true, html_attributes: {
scope: "row",
}) do %>
<%= govuk_link_to case_log_path(log) do %>
<span class="govuk-visually-hidden">Log </span><%= 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 %>
</th>
<td class="govuk-table__cell app-!-font-tabular">
<%= log.tenant_code? ? log.tenant_code : "–" %>
</td>
<td class="govuk-table__cell app-!-font-tabular">
<%= log.propcode? ? log.propcode : "–" %>
</td>
<td class="govuk-table__cell">
<%= log.startdate.present? ? log.startdate.to_formatted_s(:govuk_date) : "–" %>
</td>
<td class="govuk-table__cell">
<%= log.created_at.to_formatted_s(:govuk_date) %>
</td>
<td class="govuk-table__cell">
<%= status_tag(log.status) %>
</td>
<% if current_user.support? %>
<td class="govuk-table__cell">
<%= log.owning_organisation.name %>
</td>
<td class="govuk-table__cell">
<%= log.managing_organisation.name %>
</td>
<% end %>
</tr>
<% end %>
</tbody>
</table>
<% end %>
<% end %>
</section>

3
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 %>

24
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| %>
<span class="govuk-!-margin-right-4">
<strong><%= @pagy.count %></strong><span style="font-weight: normal"> total <%= title.downcase %></span>
</span>
<%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %>
<strong><%= @pagy.count %></strong> 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)) %>

12
app/views/organisations/logs.erb → app/views/organisations/logs.html.erb

@ -4,14 +4,12 @@
<span class="govuk-caption-l"><%= @organisation.name %></span>
<%= content_for(:title) %>
</h1>
<div class="govuk-!-margin-bottom-4">
<%= render NavigationComponent.new(
items: secondary_items(request.path, @organisation.id),
level: "sub"
) %>
</div>
<div class="app-filter-layout" data-controller="filter-layout">
<%= render SubNavigationComponent.new(
items: secondary_items(request.path, @organisation.id),
) %>
<div class="app-filter-layout" data-controller="filter-layout">
<%= render partial: "case_logs/log_filters" %>
<% if @case_logs.present? %>
<div class="app-filter-layout__content">

13
app/views/organisations/show.html.erb

@ -4,16 +4,13 @@
<%= "Details" %>
<% end %>
<div class="govuk-grid-row">
<% if current_user.support? %>
<div class="govuk-!-margin-bottom-4">
<%= render NavigationComponent.new(
items: secondary_items(request.path, @organisation.id),
level: "sub",
) %>
</div>
<%= render SubNavigationComponent.new(
items: secondary_items(request.path, @organisation.id),
) %>
<% end %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop">
<%= govuk_summary_list do |summary_list| %>
<% @organisation.display_attributes.each do |attr| %>

22
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| %>
<span class="govuk-!-margin-right-4">
<% if @searched %>
<strong><span style="font-weight: normal"> Matches </span><%= @pagy.count %><span style="font-weight: normal"> of </span><%= User.count %> <span style="font-weight: normal"> total users</strong></span>
Matches <strong><%= @pagy.count %></strong> of <strong><%= User.count %></strong> total users.
<% else %>
<strong><%= @pagy.count %></strong><span style="font-weight: normal"> total users</span>
<strong><%= @pagy.count %></strong> total users.
<% end %>
</span>
<% 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? %>
<br>

18
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")

39
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

4
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 = "<a class=\"govuk-link govuk-!-font-weight-bold\" href=\"/users\">Danny Rojas</a>\n#{user.email}"
expected_html = "<a class=\"govuk-link\" href=\"/users\">Danny Rojas</a>\n<span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{user.email}</span>"
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\n<span class='app-!-colour-muted'>Data provider</span>"
expected_html = "DLUHC\n<span class=\"app-!-colour-muted\">Data provider</span>"
expect(org_cell(user)).to match(expected_html)
end
end

2
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("<strong>#{total_organisations_count}</strong><span style=\"font-weight: normal\"> total organisations</span>")
expect(CGI.unescape_html(response.body)).to match("<strong>#{total_organisations_count}</strong> total organisations.")
end
it "has pagination links" do

8
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

Loading…
Cancel
Save