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

Related items component #704

Merged
merged 9 commits into from
Jan 13, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
// Components styles
@import "govspeak-print";
@import "metadata-print";
@import "related-items-print";
@import "title-print";
1 change: 1 addition & 0 deletions app/assets/stylesheets/govuk-component/_component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@
@import "option-select";
@import "previous-and-next-navigation";
@import "breadcrumbs";
@import "related-items";
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.govuk-related-items {
ul a {
orphans: 2;
}
}
24 changes: 24 additions & 0 deletions app/assets/stylesheets/govuk-component/_related-items.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
.govuk-related-items {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this give a standard whispace/margin to the bottom of the component? The em values don't seem to map to our usual $gutter variables, if we're porting them across, should we standardise?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we should use standard gutters. I will update.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've noticed our current margins are responsive, but the gutters are fixed pixels – updating to $gutter feels like a regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point, happy to merge with the pixel values.

I think theres a discussion to have about consistency across components, but it's not a blocker for this PR 👍

border-top: 10px solid $mainstream-brand;
padding-top: 5px;

h2 {
@include bold-24;
margin-top: 0.3em;
margin-bottom: 0.5em;
}

ul {
@include core-16;
list-style: none;
margin-bottom: 1.25em;

li {
margin-bottom: 0.75em;

&.related-items-more {
font-weight: bold;
}
}
}
}
46 changes: 46 additions & 0 deletions app/views/govuk_component/docs/related_items.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: "Related items"
description: "Headed sections of related items."
body: |
Accepts an array of sections. Each section can have a title, url, id and a list of related items.

Each item is a hash with a title and url. If the url is external, a rel value can also be provided.
fixtures:
default:
sections:
- title: "Travel Abroad"
url: "/"
id: "related-travel-abroad"
items:
- title: "Link A"
url: /
- title: "Link B"
url: /
- title: "Travel"
id: "related-travel"
items:
- title: "Link 1"
url: /
- title: "Link 2"
url: /
external_links:
sections:
- title: "Elsewhere on the web"
id: "related-elsewhere-on-the-web"
items:
- title: "Wikivorce"
url: "http://www.wikivorce.com"
rel: external
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have other rel values in mind?

If not, it might be simpler to have a boolean is_external field instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I preferred the flexibility of setting rel.

Copy link
Contributor

Choose a reason for hiding this comment

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

To expand, rel values that might be relevant: nofollow, prefetch.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 those make sense, possibly rel="up" too.

I was thinking that in a mustache environment, if we were to do something conditional on a specific value of rel then we'd have issues, but thats theoretical, and it wouldn't be a hard API chance to accommodate that.

real_example:
# from https://www.gov.uk/register-birth
sections:
- title: "Pregnancy and birth"
url: /browse/childcare-parenting/pregnancy-birth
id: "related-pregnancy-and-birth"
items:
- title: "Register a birth"
url: /register-birth
- title: "Elsewhere on GOV.UK"
id: "related-elsewhere-on-govuk"
items:
- title: "Check if you're a British citizen"
url: /check-british-citizen
33 changes: 33 additions & 0 deletions app/views/govuk_component/related_items.raw.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<%
sections ||= []
%>
<aside class="govuk-related-items" role="complementary">
<% sections.each do |section| %>
<h2
<% if section[:id] %>id="<%= section[:id] %>"<% end %>
>
<%= section[:title] %>
</h2>
<nav role="navigation" <% if section[:id] %>aria-labelledby="<%= section[:id] %>"<% end %>>
<ul>
<% section[:items].each do |item| %>
<li>
<a
href="<%= item[:url] %>"
<% if item[:rel] %>rel="<%= item[:rel] %>"<% end %>
>
<%= item[:title] %>
</a>
</li>
<% end %>
<% if section[:url] %>
<li class="related-items-more">
<a href="<%= section[:url] %>">
More<% if section[:title] %> <span class="visuallyhidden">in <%= section[:title] %></span><% end %>
</a>
</li>
<% end %>
</ul>
</nav>
<% end %>
</aside>
2 changes: 1 addition & 1 deletion lib/generators/govuk_component/templates/component.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ body: |
A long form description of the <%= human_name %> component, which may
include markdown formatting.

It should try and descrive the intent of the component, and document
It should try and describe the intent of the component, and document
any parameters passed to the component.
fixtures:
default: {}
115 changes: 115 additions & 0 deletions test/govuk_component/related_items_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
require 'govuk_component_test_helper'

class RelatedItemsTestCase < ComponentTestCase
def component_name
"related_items"
end

test "no error if no parameters passed in" do
assert_nothing_raised do
render_component({})
assert_select ".govuk-related-items"
end
end

test "renders a related items block" do
render_component({
sections: [
{
title: "Section title",
url: "/more-link",
items: [
{
title: "Item title",
url: "/item"
},
{
title: "Other item title",
url: "/other-item"
}
]
}
],
})

assert_select "h2", text: "Section title"
assert_link_with_text_in("ul li:last-child", "/more-link", "More in Section title")
assert_link_with_text_in("ul li:first-child", "/item", "Item title")
assert_link_with_text_in("ul li:first-child + li", "/other-item", "Other item title")
end

test "renders a multiple related items block" do
render_component({
sections: [
{
title: "Section title",
url: "/more-link",
items: [
{
title: "Item title",
url: "/item"
}
]
},
{
title: "Other section title",
url: "/other-more-link",
items: [
{
title: "Other item title",
url: "/other-item"
}
]
}
],
})

assert_select "h2", text: "Section title"
assert_link_with_text_in("ul li:last-child", "/more-link", "More in Section title")
assert_link_with_text_in("ul li:first-child", "/item", "Item title")

assert_select "h2", text: "Other section title"
assert_link_with_text_in("ul li:last-child", "/other-more-link", "More in Other section title")
assert_link_with_text_in("ul li:first-child", "/other-item", "Other item title")
end

test "renders external links with a rel attribute" do
render_component({
sections: [
{
title: "Elsewhere on the web",
url: "/more-link",
items: [
{
title: "Wikivorce",
url: "http://www.wikivorce.com",
rel: "external"
}
]
},
],
})
assert_select "a[rel=external]", text: "Wikivorce"
end

test "includes an id and aria-labelledby when a section id is provided" do
render_component({
sections: [
{
title: "Elsewhere on the web",
url: "/more-link",
id: "related-elsewhere-on-the-web",
items: [
{
title: "Wikivorce",
url: "http://www.wikivorce.com",
rel: "external"
}
]
},
],
})
assert_select "#related-elsewhere-on-the-web", text: "Elsewhere on the web"
assert_select "nav[aria-labelledby=related-elsewhere-on-the-web]"
end
end