Skip to content

Commit

Permalink
Improve double dot problem code
Browse files Browse the repository at this point in the history 
- shorten
- if no sessionStorage value (i.e. user is arriving at the page cold) highlight the first active link and active step, not the last
- expand documentation to clarify how this behaves
  • Loading branch information
@andysellick
andysellick committed Aug 7, 2018
1 parent 6687c3d commit e1ae137
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 51 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -7,6 +7,10 @@
useful summary for people upgrading their application, not a replication
of the commit log.

## Unreleased

* Improve step by step component double dot problem solving code (PR #473)

## 9.11.0

* Add data attributes and spellcheck support for textarea component (PR #468)
Expand Down
26 changes: 8 additions & 18 deletions app/assets/javascripts/govuk_publishing_components/components/step-by-step-nav.js
View file
Expand Up @@ -232,32 +232,22 @@
return;
}

var lastClicked = loadFromSessionStorage(sessionStoreLink);
var lastClicked = loadFromSessionStorage(sessionStoreLink) || $element.find('.' + activeLinkClass).first().attr('data-position');

if (lastClicked) {
// it's possible for the saved link position value to not match any of the currently duplicate highlighted links
// so check this otherwise it'll take the highlighting off all of them
if (!$element.find('.js-link[data-position="' + lastClicked + '"]').parent().hasClass(activeLinkClass)) {
lastClicked = $element.find('.' + activeLinkClass).first().find('.js-link').attr('data-position');
}
removeActiveStateFromAllButCurrent($activeLinks, lastClicked);
setActiveStepClass();
} else {
var activeLinkInActiveStep = $element.find('.' + activeStepClass).find('.' + activeLinkClass).first();

if (activeLinkInActiveStep.length) {
$activeLinks.removeClass(activeLinkClass);
activeLinkInActiveStep.addClass(activeLinkClass);
} else {
$activeLinks.slice(1).removeClass(activeLinkClass);
}
// it's possible for the saved link position value to not match any of the currently duplicate highlighted links
// so check this otherwise it'll take the highlighting off all of them
if (!$element.find('.js-link[data-position="' + lastClicked + '"]').parent().hasClass(activeLinkClass)) {
lastClicked = $element.find('.' + activeLinkClass).first().find('.js-link').attr('data-position');
}
removeActiveStateFromAllButCurrent($activeLinks, lastClicked);
setActiveStepClass();
}

function removeActiveStateFromAllButCurrent($activeLinks, current) {
$activeLinks.each(function() {
if ($(this).find('.js-link').attr('data-position').toString() !== current.toString()) {
$(this).removeClass(activeLinkClass);
$(this).find('.visuallyhidden').remove();
}
});
}
Expand Down
32 changes: 14 additions & 18 deletions app/views/govuk_publishing_components/components/docs/step_by_step_nav.yml
View file
Expand Up @@ -215,19 +215,16 @@ examples:
contents: [
{
href: '/component-guide/step_by_step_navigation/with_links/preview',
text: 'Obtain a driving licence prior to driving a car',
active: true
text: 'Obtain a driving licence prior to driving a car'
},
{
href: '/component-guide/step_by_step_navigation/with_links/preview',
text: 'Acquire food before attempting to cook',
context: '1p to £20',
active: true
context: '1p to £20'
},
{
href: '/component-guide/step_by_step_navigation/with_links/preview',
text: 'Maintain contact with the ground at all times',
active: true
text: 'Maintain contact with the ground at all times'
}
]
},
Expand All @@ -241,14 +238,12 @@ examples:
contents: [
{
href: '/component-guide/step_by_step_navigation/',
text: 'Leave school at sixteen',
active: true
text: 'Leave school at sixteen'
},
{
href: '/component-guide/step_by_step_navigation/',
text: 'Continue into higher education',
context: '£9,500',
active: true
context: '£9,500'
}
]
}
Expand All @@ -272,8 +267,7 @@ examples:
},
{
href: '/component-guide/step_by_step_navigation/with_links/preview',
text: 'Do the things',
active: true
text: 'Do the things'
}
]
},
Expand Down Expand Up @@ -340,22 +334,24 @@ examples:
]
solve_the_double_dot_problem:
description: |
If a page is in a step by step navigation more than once and a user is viewing that URL, both links to it will be highlighted as the backend has no way to know which link the user is currently viewing (links should only be highlighted when the step by step navigation is in the sidebar).
As shown, options can be passed to the component to highlight one step, expand one step by default, and highlight multiple links. These options should only be used when the component is in the sidebar. The step that is highlighted and expanded will be referred to as the active step (even though they are separate options, it is assumed that they will normally be applied to the same step).
JavaScript is included in the component to solve this. It uses sessionStorage to capture the data-position attribute of non-external links when clicked, and then uses this value to decide which link (and parent step) to highlight and expand when the new page loads. Note that it uses the tracking_id attribute to uniquely identify the current step nav. If tracking_id is not set this may result in other step navs having the wrong link highlighted.
If a link is in a step by step navigation more than once and the user is on that page, the backend doesn't know which link to highlight, so it highlights both of them.
If a user has not clicked a link (i.e. has visited the page without first clicking on a step by step navigation) the first active link in the first active step will be highlighted. If there is no active step, the first active link will be highlighted (but there should always be an active step).
JavaScript is included in the component to solve this. It uses sessionStorage to capture the data-position attribute of non-external links when clicked, and then uses this value to decide which link to highlight after the new page loads. It uses the value of the tracking_id option to uniquely identify the current step nav in the session (if this is not passed to the component this may result in other step navs having the wrong link highlighted).
The current page in the step by step navigation is an anchor link to the top of the page. If there are more than one of these and the user clicks one that is not currently highlighted, that one will be highlighted.
If a user has not clicked a link (i.e. has visited the page without first clicking on a step by step navigation) only the first duplicate link will be highlighted. If that link is not in the active step, the JS makes the highlighted link's parent the active step. If there is no active step, the first active link will be highlighted (but there should always be an active step).
The example below will show all non-external links highlighted if JS is disabled. In the real world no more than two or three links are likely to be highlighted at once.
Changes to the active step and highlighted link are also applied on click, if the user clicks one of the other links that is to the same page (they are amended by the component to be jump links to the top of the page). Open this example using the 'preview' link and try clicking on the 'internal' links to see how it behaves.
data:
highlight_step: 2
show_step: 2
tracking_id: "example-id"
steps: [
{
title: "Not the active step",
title: "The active step",
contents: [
{
type: 'list',
Expand Down Expand Up @@ -383,7 +379,7 @@ examples:
]
},
{
title: "The active step",
title: "Not the active step",
contents: [
{
type: 'list',
Expand Down
50 changes: 35 additions & 15 deletions spec/javascripts/components/step-by-step-nav-spec.js
View file
Expand Up @@ -42,14 +42,14 @@ describe('A stepnav module', function () {
</div>\
<div class="gem-c-step-nav__panel js-panel" id="step-panel-topic-step-two-1">\
<ol class="gem-c-step-nav__list" data-length="2">\
<li class="gem-c-step-nav__list-item gem-c-step-nav__list-item--active js-list-item">\
<a href="/link2" class="gem-c-step-nav__link js-link" data-position="2.1">Link 2</a>\
<li class="gem-c-step-nav__list-item js-will-be-an-active-link js-list-item">\
<a href="/link2" class="gem-c-step-nav__link js-link" data-position="2.1"><span class="visuallyhidden">You are currently viewing: </span>Link 2</a>\
</li>\
<li class="gem-c-step-nav__list-item js-list-item">\
<a href="/link3" class="gem-c-step-nav__link js-link" data-position="2.2">Link 3</a>\
</li>\
<li class="gem-c-step-nav__list-item gem-c-step-nav__list-item--active js-list-item">\
<a href="#content" class="gem-c-step-nav__link js-link" data-position="2.3">Link 4</a>\
<li class="gem-c-step-nav__list-item js-will-be-an-active-link js-list-item">\
<a href="#content" class="gem-c-step-nav__link js-link" data-position="2.3"><span class="visuallyhidden">You are currently viewing: </span>Link 4</a>\
</li>\
</ol>\
</div>\
Expand All @@ -69,20 +69,20 @@ describe('A stepnav module', function () {
</div>\
<div class="gem-c-step-nav__panel js-panel" id="step-panel-topic-step-three-1">\
<ol class="gem-c-step-nav__list" data-length="5">\
<li class="gem-c-step-nav__list-item gem-c-step-nav__list-item--active js-list-item">\
<a href="/link4" class="gem-c-step-nav__link js-link" data-position="3.1">Link 5</a>\
<li class="gem-c-step-nav__list-item js-will-be-an-active-link js-list-item">\
<a href="/link4" class="gem-c-step-nav__link js-link" data-position="3.1"><span class="visuallyhidden">You are currently viewing: </span>Link 5</a>\
</li>\
<li class="gem-c-step-nav__list-item gem-c-step-nav__list-item--active js-list-item">\
<a href="/link5" class="gem-c-step-nav__link js-link" data-position="3.2">Link 6</a>\
<li class="gem-c-step-nav__list-item js-will-be-an-active-link js-list-item">\
<a href="/link5" class="gem-c-step-nav__link js-link" data-position="3.2"><span class="visuallyhidden">You are currently viewing: </span>Link 6</a>\
</li>\
<li class="gem-c-step-nav__list-item js-list-item">\
<a href="http://www.gov.uk" class="gem-c-step-nav__link js-link" data-position="3.3" rel="external">Link 7</a>\
</li>\
<li class="gem-c-step-nav__list-item gem-c-step-nav__list-item--active js-list-item">\
<a href="#content" class="gem-c-step-nav__link js-link" data-position="3.4">Link 8</a>\
<li class="gem-c-step-nav__list-item js-will-be-an-active-link js-list-item">\
<a href="#content" class="gem-c-step-nav__link js-link" data-position="3.4"><span class="visuallyhidden">You are currently viewing: </span>Link 8</a>\
</li>\
<li class="gem-c-step-nav__list-item gem-c-step-nav__list-item--active js-list-item">\
<a href="#content" class="gem-c-step-nav__link js-link" data-position="3.5">Link 9</a>\
<li class="gem-c-step-nav__list-item js-will-be-an-active-link js-list-item">\
<a href="#content" class="gem-c-step-nav__link js-link" data-position="3.5"><span class="visuallyhidden">You are currently viewing: </span>Link 9</a>\
</li>\
</ol>\
</div>\
Expand Down Expand Up @@ -792,10 +792,12 @@ describe('A stepnav module', function () {
beforeEach(function () {
stepnav = new GOVUK.Modules.Gemstepnav();
$element = $(html);
$element.find('.js-will-be-an-active-link').addClass('gem-c-step-nav__list-item--active');
stepnav.start($element);
});

afterEach(function () {
$element.find('.js-will-be-an-active-link').removeClass('gem-c-step-nav__list-item--active');
sessionStorage.removeItem('govuk-step-nav-active-link_unique-id');
});

Expand All @@ -809,10 +811,19 @@ describe('A stepnav module', function () {
expect(sessionStorage.getItem('govuk-step-nav-active-link_unique-id')).toBe(null);
});

it("highlights the first active link in the first active step if no sessionStorage value is set", function () {
it("highlights the first active link and its parent step if no sessionStorage value is set", function () {
expect(sessionStorage.getItem('govuk-step-nav-active-link_unique-id')).toBe(null);
expect($element.find('.js-link[data-position="3.1"]').closest('.js-list-item')).toHaveClass('gem-c-step-nav__list-item--active');
expect($element.find(('.gem-c-step-nav__list-item--active')).length).toBe(1);
var $active = $element.find('.js-link[data-position="2.1"]');
expect($active.closest('.js-list-item')).toHaveClass('gem-c-step-nav__list-item--active');
expect($active.closest('.js-step')).toHaveClass('gem-c-step-nav__step--active');
expect($element.find('.gem-c-step-nav__list-item--active').length).toBe(1);
expect($element.find('.gem-c-step-nav__list-item--active').length).toBe(1);
expect($element.find('.step-is-shown').length).toBe(1);
});

it("ensures only the active link has a hidden span for screen readers to indicate which is the active link", function () {
var $spans = $element.find('.js-link .visuallyhidden');
expect($spans.length).toBe(1);
});

it("highlights a clicked #content link and its parent step, and removes other highlighting", function () {
Expand Down Expand Up @@ -852,11 +863,13 @@ describe('A stepnav module', function () {

stepnav = new GOVUK.Modules.Gemstepnav();
$element = $(html);
$element.find('.js-will-be-an-active-link').addClass('gem-c-step-nav__list-item--active');
sessionStorage.setItem('govuk-step-nav-active-link_unique-id', '3.5');
stepnav.start($element);
});

afterEach(function () {
$element.find('.js-will-be-an-active-link').removeClass('gem-c-step-nav__list-item--active');
sessionStorage.removeItem('govuk-step-nav-active-link_unique-id');
});

Expand All @@ -883,11 +896,13 @@ describe('A stepnav module', function () {

stepnav = new GOVUK.Modules.Gemstepnav();
$element = $(html);
$element.find('.js-will-be-an-active-link').addClass('gem-c-step-nav__list-item--active');
sessionStorage.setItem('govuk-step-nav-active-link_unique-id', 'definitelynotvalid');
stepnav.start($element);
});

afterEach(function () {
$element.find('.js-will-be-an-active-link').removeClass('gem-c-step-nav__list-item--active');
sessionStorage.removeItem('govuk-step-nav-active-link_unique-id');
});

Expand All @@ -900,11 +915,16 @@ describe('A stepnav module', function () {
describe('in a double dot situation where there is no active step', function () {
beforeEach(function () {
$element = $(html);
$element.find('.js-will-be-an-active-link').addClass('gem-c-step-nav__list-item--active');
$element.find('.gem-c-step-nav__step').removeClass('gem-c-step-nav__step--active');
stepnav = new GOVUK.Modules.Gemstepnav();
stepnav.start($element);
});

afterEach(function() {
$element.find('.js-will-be-an-active-link').removeClass('gem-c-step-nav__list-item--active');
});

it("highlights the first active link if no sessionStorage value is set", function () {
expect(sessionStorage.getItem('govuk-step-nav-active-link_unique-id')).toBe(null);
expect($element.find('.js-link[data-position="2.1"]').closest('.js-list-item')).toHaveClass('gem-c-step-nav__list-item--active');
Expand Down

0 comments on commit e1ae137

Please sign in to comment.