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

Allow HTML pub contents to render cleanly with custom numbers #395

Merged
merged 4 commits into from Jul 3, 2017
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 19 additions & 29 deletions app/assets/stylesheets/views/_html-publication.scss
Expand Up @@ -73,18 +73,28 @@
line-height: 1.3;
}

a {
text-decoration: none;
.contents-text,
.contents-number {
display: table-cell;
}

.contents-number {
min-width: 1.5em;
}

// Child spans are floated which creates a focus box around
// just the number. Set parent link to inline-block for
// correct appearance
display: inline-block;
.contents-text {
$contents-text-padding: 0.3em;
padding-left: $contents-text-padding;

// IE8 and below don't render inline-block correctly
@include ie-lte(8) {
display: block;
.direction-rtl & {
padding-left: 0;
padding-right: $contents-text-padding;
}
}

a {
display: table;
text-decoration: none;

&:hover {
text-decoration: underline;
Expand All @@ -94,26 +104,6 @@
@include media(false, $desktop-breakpoint) {
box-shadow: none;
}

.contents-number {
float: left;
}

.contents-text {
display: block;
margin-left: 1.5em;
}

.direction-rtl & {
.contents-number {
float: right;
}

.contents-text {
margin-left: 0;
margin-right: 1.5em;
}
}
}
}

Expand Down
8 changes: 7 additions & 1 deletion app/helpers/contents_list_helper.rb
@@ -1,7 +1,13 @@
module ContentsListHelper
def wrap_numbers_with_spans(content_item_link)
content_item_text = strip_tags(content_item_link) #just the text of the link
number = /^[0-9]*[.]/.match(content_item_text)

# Must start with a number
# Number must be between 1 and 999 (ie not 2014)
# Must be followed by a space
# May contain a period `1.`
# May be a decimal `1.2`
number = /^\d{1,3}(\.?|\.\d{1,2})(?=\s)/.match(content_item_text)
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably isn't an issue but wanted to check - this regex will match '1.12 Title' but not '1.123 Title' i.e. anything with more than two numbers after a decimal point. Is this intentional? I don't know if any of our content would exceed those two characters or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, that was intentional. I didn't find any examples of X.ABC. I felt like it should be limited. Something like 1.0003 might reasonably be a decimal rather than a heading level. Unlikely to happen in reality.


if number
words = content_item_text.sub(number.to_s, '').strip #remove the number from the text
Expand Down
56 changes: 53 additions & 3 deletions test/helpers/contents_list_helper_test.rb
@@ -1,9 +1,29 @@
require 'test_helper'

class ContentsListHelperTest < ActionView::TestCase
test "it wraps the number and text in span elements" do
input = '<a href="#run-an-effective-welfare-system-that-enables-people-to-achieve-financial-independence-by-providing-assistance-and-guidance-into-employment">1. Run an effective welfare system that enables people to achieve financial independence by providing assistance and guidance into employment</a>'
expected = '<a href="#run-an-effective-welfare-system-that-enables-people-to-achieve-financial-independence-by-providing-assistance-and-guidance-into-employment"><span class="contents-number">1.</span><span class="contents-text">Run an effective welfare system that enables people to achieve financial independence by providing assistance and guidance into employment</span></a>'
test "it wraps a number and text in separate span elements" do
input = '<a href="#first">1. Thing</a>'
expected = '<a href="#first"><span class="contents-number">1.</span><span class="contents-text">Thing</span></a>'
assert_equal expected, wrap_numbers_with_spans(input)

input = '<a href="#vision">10. Vision</a>'
expected = '<a href="#vision"><span class="contents-number">10.</span><span class="contents-text">Vision</span></a>'
assert_equal expected, wrap_numbers_with_spans(input)

input = '<a href="#vision">100. Vision</a>'
expected = '<a href="#vision"><span class="contents-number">100.</span><span class="contents-text">Vision</span></a>'
assert_equal expected, wrap_numbers_with_spans(input)
end

test "it wraps a number and text in span elements if it's a number without a period" do
input = '<a href="#first">1 Thing</a>'
expected = '<a href="#first"><span class="contents-number">1</span><span class="contents-text">Thing</span></a>'
assert_equal expected, wrap_numbers_with_spans(input)
end

test "it wraps a number in the form X.Y" do
input = '<a href="#vision">1.2 Vision</a>'
expected = '<a href="#vision"><span class="contents-number">1.2</span><span class="contents-text">Vision</span></a>'
assert_equal expected, wrap_numbers_with_spans(input)
end

Expand All @@ -13,6 +33,36 @@ class ContentsListHelperTest < ActionView::TestCase
assert_equal expected, wrap_numbers_with_spans(input)
end

test "it does nothing if it's just a number" do
input = '<a href="#first">1</a>'
expected = '<a href="#first">1</a>'
assert_equal expected, wrap_numbers_with_spans(input)
end

test "it does nothing if the number is part of the word" do
input = '<a href="#vision">1Vision</a>'
expected = '<a href="#vision">1Vision</a>'
assert_equal expected, wrap_numbers_with_spans(input)

input = '<a href="#vision">1.Vision</a>'
expected = '<a href="#vision">1.Vision</a>'
assert_equal expected, wrap_numbers_with_spans(input)
end

test "it does nothing if it starts with a number longer than 3 digits" do
input = '<a href="#vision">2014 Vision</a>'
expected = '<a href="#vision">2014 Vision</a>'
assert_equal expected, wrap_numbers_with_spans(input)

input = '<a href="#vision">2014. Vision</a>'
expected = '<a href="#vision">2014. Vision</a>'
assert_equal expected, wrap_numbers_with_spans(input)

input = '<a href="#vision">10001. Vision</a>'
expected = '<a href="#vision">10001. Vision</a>'
assert_equal expected, wrap_numbers_with_spans(input)
end

test "it does nothing if a number is present but not at the start" do
input = '<a href="#run-an-effective-welfare-system">Run an effective welfare system part 1. Social Care</a>'
expected = '<a href="#run-an-effective-welfare-system">Run an effective welfare system part 1. Social Care</a>'
Expand Down