Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update taxonomy sidebar design #1222

Merged
merged 4 commits into from
Dec 15, 2017
Merged
Show file tree
Hide file tree
Changes from 3 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
45 changes: 32 additions & 13 deletions app/assets/stylesheets/govuk-component/_taxonomy-sidebar.scss
Original file line number Diff line number Diff line change
@@ -1,29 +1,48 @@
.govuk-taxonomy-sidebar {
border-top: 10px solid $mainstream-brand;
padding-top: 5px;
border-top: 2px solid $govuk-blue;
padding-top: $gutter-half;
@include core-16;

h2 {
@include bold-24;
margin-top: 0.3em;
margin-bottom: 0.5em;
.govuk-taxonomy-sidebar__heading {
margin-top: 0;
margin-bottom: $gutter-one-third;
@include bold-19;
}

.taxon-description {
margin-bottom: 0.75em;
margin-top: 0;
margin-bottom: $gutter-one-third;
}

a {
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually wouldn't style an "a" directly. Sometimes we are forced to, but we absolutely need to ensure we're not styling all a tags on the page, so
.taxon-description a { blah }
or something may be necessary - ideally though the anchor would have a styling class such as .govuk-taxonomy-sidebar__link

Copy link
Contributor

Choose a reason for hiding this comment

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

These seem to be nested within the selector at the top, but massive 👍 to always avoiding styling base elements if possible.

We should only be doing this if there's no other way as it directly conflicts with our naming conventions that allow us to have confidence with components across so many applications.

https://github.com/alphagov/govuk_publishing_components/blob/master/docs/component_conventions.md

Copy link
Contributor

Choose a reason for hiding this comment

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

Specifically we're using BEM and namespacing

text-decoration: none;
}

nav {
Copy link
Contributor

Choose a reason for hiding this comment

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

again, this needs to be scoped

margin-bottom: $gutter;
}

li {
Copy link
Contributor

Choose a reason for hiding this comment

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

scoping needed

padding: 0;
list-style-type: none;
margin-bottom: $gutter-one-third;
}

ul {
Copy link
Contributor

Choose a reason for hiding this comment

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

scope

// reset the default browser styles
padding: 0;
margin: 0;
list-style: none;
margin-bottom: 1.25em;
}

.govuk-taxonomy-sidebar__collections-heading {
border-top: 1px solid $border-colour;
margin-bottom: $gutter-half;
margin-top: $gutter;
padding-top: $gutter-half;
}

li {
// reset the default browser styles
padding: 0;
margin-bottom: 0.75em;
}
.govuk-taxonomy-sidebar__collections-link {
@include bold-19;
}
}
5 changes: 4 additions & 1 deletion app/views/govuk_component/docs/taxonomy_sidebar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ examples:
link: /government/collections/key-stage-1-teacher-assessment
- title: "Primary assessments: information and resources for 2017"
link: /government/publications/primary-assessments-information-and-resources-for-2017
- title: "Slide pack from Phil Beach at the Skills & Employability Summit"
- title: "Slide pack from Phil Beach at the Skills & Employability Summit"
link: /government/publications/primary-assessments-information-and-resources-for-2017
- title: "Transport to education and training for people aged 16 to 18"
link: /government/publications/primary-assessments-information-and-resources-for-2017
Expand All @@ -64,6 +64,9 @@ examples:
link: /government/collections/key-stage-1-teacher-assessment
- title: "Primary assessments: information and resources for 2017"
link: /government/publications/primary-assessments-information-and-resources-for-2017
collections:
- text: "Statistics: outcome based success measures"
- path: "/government/collections/statistics-outcome-based-success-measures"
no_children:
description: |
The child links are usually pulled from rummager. If rummager doesn’t respond we fallback to a sidebar with a taxon link but without any child content links.
Expand Down
22 changes: 20 additions & 2 deletions app/views/govuk_component/taxonomy_sidebar.raw.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<% if local_assigns[:items] && !items.blank? %>

<aside class='govuk-taxonomy-sidebar' data-module='track-click' role='complementary'>
<h2 class='govuk-taxonomy-sidebar__heading'><%= t("govuk_component.taxonomy_sidebar.related_content") %></h2>
<% items.each_with_index do |item, item_index| %>
<div class='sidebar-taxon' data-track-count="sidebarTaxonSection">
<h2>
<h3 class='govuk-taxonomy-sidebar__heading'>
<%=
link_to_if(
item[:url],
Expand All @@ -21,7 +22,7 @@
},
)
%>
</h2>
</h3>

<p class='taxon-description'>
<%= item[:description] %>
Expand Down Expand Up @@ -56,6 +57,23 @@
<% end %>
</div>
<% end %>

<% if local_assigns[:collections] && !collections.blank? %>
<div>
<h3 class="govuk-taxonomy-sidebar__collections-heading">Collections</h3>
<nav role='navigation'>
<ul>
<% collections.each do |item| %>
<% if item[:text] && item[:path] %>
<li><%= link_to(item[:text],
item[:path],
class: "govuk-taxonomy-sidebar__collections-link") %></li>
<% end %>
<% end %>
</ul>
</nav>
</div>
<% end %>
</aside>

<% end %>
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,5 @@ en:
in: "in"
task_list_related:
part_of: "Part of"
taxonomy_sidebar:
related_content: "Related content"
36 changes: 31 additions & 5 deletions test/govuk_component/taxonomy_sidebar_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def component_name
]
)

taxon_titles = css_select(".sidebar-taxon h2").map { |taxon_title| taxon_title.text.strip }
taxon_titles = css_select(".sidebar-taxon h3").map { |taxon_title| taxon_title.text.strip }
assert_equal ["Item 1 title", "Item 2 title"], taxon_titles
end

Expand Down Expand Up @@ -70,7 +70,7 @@ def component_name
total_sections = 2
total_links_in_section_1 = 3

assert_select 'h2 a', "Item title"
assert_select 'h3 a', "Item title"
assert_select '.govuk-taxonomy-sidebar[data-module="track-click"]', 1
assert_tracking_link("category", "relatedLinkClicked", 6)

Expand All @@ -94,7 +94,7 @@ def component_name
end


test "renders without url on the h2 heading" do
test "renders without url on the h3 heading" do
render_component(
items: [
{
Expand All @@ -114,7 +114,33 @@ def component_name
]
)

assert_select 'h2', "Without an url"
assert_select 'h2 a', false
assert_select 'h3', "Without an url"
assert_select 'h3 a', false
end

test "renders collections links" do
render_component(
items: [
{
title: "Without an url",
description: "An item",
related_content: [
{
title: "Related link 1",
link: "/related-link-1",
}
],
},
],
collections: [
{
text: "Collection 1",
path: "/some-collection"
}
]
)

assert_select "h3.govuk-taxonomy-sidebar__collections-heading", "Collections"
assert_select "a.govuk-taxonomy-sidebar__collections-link", "Collection 1"
end
end