From e1ae1378b9bde922af9b728e88efb1c0e343bba4 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Tue, 7 Aug 2018 13:45:42 +0000 Subject: [PATCH] Improve double dot problem code - 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 --- CHANGELOG.md | 4 ++ .../components/step-by-step-nav.js | 26 +++------- .../components/docs/step_by_step_nav.yml | 32 ++++++------ .../components/step-by-step-nav-spec.js | 50 +++++++++++++------ 4 files changed, 61 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 329f6978de..36eaefaa0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/app/assets/javascripts/govuk_publishing_components/components/step-by-step-nav.js b/app/assets/javascripts/govuk_publishing_components/components/step-by-step-nav.js index 33335cbf72..411054f590 100644 --- a/app/assets/javascripts/govuk_publishing_components/components/step-by-step-nav.js +++ b/app/assets/javascripts/govuk_publishing_components/components/step-by-step-nav.js @@ -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(); } }); } diff --git a/app/views/govuk_publishing_components/components/docs/step_by_step_nav.yml b/app/views/govuk_publishing_components/components/docs/step_by_step_nav.yml index c71f405127..225f520dab 100644 --- a/app/views/govuk_publishing_components/components/docs/step_by_step_nav.yml +++ b/app/views/govuk_publishing_components/components/docs/step_by_step_nav.yml @@ -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' } ] }, @@ -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' } ] } @@ -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' } ] }, @@ -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', @@ -383,7 +379,7 @@ examples: ] }, { - title: "The active step", + title: "Not the active step", contents: [ { type: 'list', diff --git a/spec/javascripts/components/step-by-step-nav-spec.js b/spec/javascripts/components/step-by-step-nav-spec.js index 2aa1410f0b..079decefc4 100644 --- a/spec/javascripts/components/step-by-step-nav-spec.js +++ b/spec/javascripts/components/step-by-step-nav-spec.js @@ -42,14 +42,14 @@ describe('A stepnav module', function () { \
\
    \ -
  1. \ - Link 2\ +
  2. \ + You are currently viewing: Link 2\
    3. \
  3. \ Link 3\
    4. \ -
  4. \ - Link 4\ +
  5. \ + You are currently viewing: Link 4\
    6. \
\
\ @@ -69,20 +69,20 @@ describe('A stepnav module', function () { \
\
    \ -
  1. \ - Link 5\ +
  2. \ + You are currently viewing: Link 5\
    3. \ -
  3. \ - Link 6\ +
  4. \ + You are currently viewing: Link 6\
    5. \
  5. \ Link 7\
    6. \ -
  6. \ - Link 8\ +
  7. \ + You are currently viewing: Link 8\
    8. \ -
  8. \ - Link 9\ +
  9. \ + You are currently viewing: Link 9\
    10. \
\
\ @@ -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'); }); @@ -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 () { @@ -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'); }); @@ -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'); }); @@ -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');